Brad Bishop | 64c979e | 2019-11-04 13:55:29 -0500 | [diff] [blame^] | 1 | From f752ae5cee163253730ff7cdf293e34a91aa5520 Mon Sep 17 00:00:00 2001 |
| 2 | From: "Todd C. Miller" <Todd.Miller@sudo.ws> |
| 3 | Date: Thu, 10 Oct 2019 10:04:13 -0600 |
| 4 | Subject: [PATCH] Treat an ID of -1 as invalid since that means "no change". |
| 5 | Fixes CVE-2019-14287. Found by Joe Vennix from Apple Information Security. |
| 6 | |
| 7 | Upstream-Status: Backport [https://github.com/sudo-project/sudo/commit/f752ae5cee163253730ff7cdf293e34a91aa5520] |
| 8 | CVE: CVE-2019-14287 |
| 9 | |
| 10 | Signed-off-by: Changqing Li <changqing.li@windriver.com> |
| 11 | |
| 12 | --- |
| 13 | lib/util/strtoid.c | 100 ++++++++++++++++++++++++++++------------------------- |
| 14 | 1 files changed, 53 insertions(+), 46 deletions(-) |
| 15 | |
| 16 | diff --git a/lib/util/strtoid.c b/lib/util/strtoid.c |
| 17 | index 2dfce75..6b3916b 100644 |
| 18 | --- a/lib/util/strtoid.c |
| 19 | +++ b/lib/util/strtoid.c |
| 20 | @@ -49,6 +49,27 @@ |
| 21 | #include "sudo_util.h" |
| 22 | |
| 23 | /* |
| 24 | + * Make sure that the ID ends with a valid separator char. |
| 25 | + */ |
| 26 | +static bool |
| 27 | +valid_separator(const char *p, const char *ep, const char *sep) |
| 28 | +{ |
| 29 | + bool valid = false; |
| 30 | + debug_decl(valid_separator, SUDO_DEBUG_UTIL) |
| 31 | + |
| 32 | + if (ep != p) { |
| 33 | + /* check for valid separator (including '\0') */ |
| 34 | + if (sep == NULL) |
| 35 | + sep = ""; |
| 36 | + do { |
| 37 | + if (*ep == *sep) |
| 38 | + valid = true; |
| 39 | + } while (*sep++ != '\0'); |
| 40 | + } |
| 41 | + debug_return_bool(valid); |
| 42 | +} |
| 43 | + |
| 44 | +/* |
| 45 | * Parse a uid/gid in string form. |
| 46 | * If sep is non-NULL, it contains valid separator characters (e.g. comma, space) |
| 47 | * If endp is non-NULL it is set to the next char after the ID. |
| 48 | @@ -62,36 +83,33 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr |
| 49 | char *ep; |
| 50 | id_t ret = 0; |
| 51 | long long llval; |
| 52 | - bool valid = false; |
| 53 | debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL) |
| 54 | |
| 55 | /* skip leading space so we can pick up the sign, if any */ |
| 56 | while (isspace((unsigned char)*p)) |
| 57 | p++; |
| 58 | - if (sep == NULL) |
| 59 | - sep = ""; |
| 60 | + |
| 61 | + /* While id_t may be 64-bit signed, uid_t and gid_t are 32-bit unsigned. */ |
| 62 | errno = 0; |
| 63 | llval = strtoll(p, &ep, 10); |
| 64 | - if (ep != p) { |
| 65 | - /* check for valid separator (including '\0') */ |
| 66 | - do { |
| 67 | - if (*ep == *sep) |
| 68 | - valid = true; |
| 69 | - } while (*sep++ != '\0'); |
| 70 | + if ((errno == ERANGE && llval == LLONG_MAX) || llval > (id_t)UINT_MAX) { |
| 71 | + errno = ERANGE; |
| 72 | + if (errstr != NULL) |
| 73 | + *errstr = N_("value too large"); |
| 74 | + goto done; |
| 75 | } |
| 76 | - if (!valid) { |
| 77 | + if ((errno == ERANGE && llval == LLONG_MIN) || llval < INT_MIN) { |
| 78 | + errno = ERANGE; |
| 79 | if (errstr != NULL) |
| 80 | - *errstr = N_("invalid value"); |
| 81 | - errno = EINVAL; |
| 82 | + *errstr = N_("value too small"); |
| 83 | goto done; |
| 84 | } |
| 85 | - if (errno == ERANGE) { |
| 86 | - if (errstr != NULL) { |
| 87 | - if (llval == LLONG_MAX) |
| 88 | - *errstr = N_("value too large"); |
| 89 | - else |
| 90 | - *errstr = N_("value too small"); |
| 91 | - } |
| 92 | + |
| 93 | + /* Disallow id -1, which means "no change". */ |
| 94 | + if (!valid_separator(p, ep, sep) || llval == -1 || llval == (id_t)UINT_MAX) { |
| 95 | + if (errstr != NULL) |
| 96 | + *errstr = N_("invalid value"); |
| 97 | + errno = EINVAL; |
| 98 | goto done; |
| 99 | } |
| 100 | ret = (id_t)llval; |
| 101 | @@ -108,30 +126,15 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr |
| 102 | { |
| 103 | char *ep; |
| 104 | id_t ret = 0; |
| 105 | - bool valid = false; |
| 106 | debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL) |
| 107 | |
| 108 | /* skip leading space so we can pick up the sign, if any */ |
| 109 | while (isspace((unsigned char)*p)) |
| 110 | p++; |
| 111 | - if (sep == NULL) |
| 112 | - sep = ""; |
| 113 | + |
| 114 | errno = 0; |
| 115 | if (*p == '-') { |
| 116 | long lval = strtol(p, &ep, 10); |
| 117 | - if (ep != p) { |
| 118 | - /* check for valid separator (including '\0') */ |
| 119 | - do { |
| 120 | - if (*ep == *sep) |
| 121 | - valid = true; |
| 122 | - } while (*sep++ != '\0'); |
| 123 | - } |
| 124 | - if (!valid) { |
| 125 | - if (errstr != NULL) |
| 126 | - *errstr = N_("invalid value"); |
| 127 | - errno = EINVAL; |
| 128 | - goto done; |
| 129 | - } |
| 130 | if ((errno == ERANGE && lval == LONG_MAX) || lval > INT_MAX) { |
| 131 | errno = ERANGE; |
| 132 | if (errstr != NULL) |
| 133 | @@ -144,28 +147,31 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr |
| 134 | *errstr = N_("value too small"); |
| 135 | goto done; |
| 136 | } |
| 137 | - ret = (id_t)lval; |
| 138 | - } else { |
| 139 | - unsigned long ulval = strtoul(p, &ep, 10); |
| 140 | - if (ep != p) { |
| 141 | - /* check for valid separator (including '\0') */ |
| 142 | - do { |
| 143 | - if (*ep == *sep) |
| 144 | - valid = true; |
| 145 | - } while (*sep++ != '\0'); |
| 146 | - } |
| 147 | - if (!valid) { |
| 148 | + |
| 149 | + /* Disallow id -1, which means "no change". */ |
| 150 | + if (!valid_separator(p, ep, sep) || lval == -1) { |
| 151 | if (errstr != NULL) |
| 152 | *errstr = N_("invalid value"); |
| 153 | errno = EINVAL; |
| 154 | goto done; |
| 155 | } |
| 156 | + ret = (id_t)lval; |
| 157 | + } else { |
| 158 | + unsigned long ulval = strtoul(p, &ep, 10); |
| 159 | if ((errno == ERANGE && ulval == ULONG_MAX) || ulval > UINT_MAX) { |
| 160 | errno = ERANGE; |
| 161 | if (errstr != NULL) |
| 162 | *errstr = N_("value too large"); |
| 163 | goto done; |
| 164 | } |
| 165 | + |
| 166 | + /* Disallow id -1, which means "no change". */ |
| 167 | + if (!valid_separator(p, ep, sep) || ulval == UINT_MAX) { |
| 168 | + if (errstr != NULL) |
| 169 | + *errstr = N_("invalid value"); |
| 170 | + errno = EINVAL; |
| 171 | + goto done; |
| 172 | + } |
| 173 | ret = (id_t)ulval; |
| 174 | } |
| 175 | if (errstr != NULL) |
| 176 | -- |
| 177 | 2.7.4 |
| 178 | |