Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:15
sudo.22978
sudo-CVE-2019-14287.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File sudo-CVE-2019-14287.patch of Package sudo.22978
Treat an ID of -1 as invalid since that means "no change". Fixes CVE-2019-14287. Found by Joe Vennix from Apple Information Security. diff --git a/lib/util/regress/atofoo/atofoo_test.c b/lib/util/regress/atofoo/atofoo_test.c index 1ad78eb..071123f 100644 --- a/lib/util/regress/atofoo/atofoo_test.c +++ b/lib/util/regress/atofoo/atofoo_test.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014 Todd C. Miller <Todd.Miller@sudo.ws> + * Copyright (c) 2014-2019 Todd C. Miller <Todd.Miller@sudo.ws> * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -24,6 +24,7 @@ #else # include "compat/stdbool.h" #endif +#include <errno.h> #include "sudo_compat.h" #include "sudo_util.h" @@ -78,10 +79,15 @@ static struct strtoid_data { id_t id; const char *sep; const char *ep; + int errnum; } strtoid_data[] = { - { "0,1", 0, ",", "," }, - { "10", 10, NULL, NULL }, - { "-2", -2, NULL, NULL }, + { "0,1", 0, ",", ",", 0 }, + { "10", 10, NULL, NULL, 0 }, + { "-1", 0, NULL, NULL, EINVAL }, + { "4294967295", 0, NULL, NULL, EINVAL }, + { "4294967296", 0, NULL, NULL, ERANGE }, + { "-2147483649", 0, NULL, NULL, ERANGE }, + { "-2", -2, NULL, NULL, 0 }, #if SIZEOF_ID_T != SIZEOF_LONG_LONG { "-2", (id_t)4294967294U, NULL, NULL }, #endif @@ -102,11 +108,23 @@ test_strtoid(int *ntests) (*ntests)++; errstr = "some error"; value = sudo_strtoid(d->idstr, d->sep, &ep, &errstr); - if (errstr != NULL) { - if (d->id != (id_t)-1) { - sudo_warnx_nodebug("FAIL: %s: %s", d->idstr, errstr); + if (d->errnum != 0) { + if (errstr == NULL) { + sudo_warnx_nodebug("FAIL: %s: missing errstr for errno %d", + d->idstr, d->errnum); + errors++; + } else if (value != 0) { + sudo_warnx_nodebug("FAIL: %s should return 0 on error", + d->idstr); + errors++; + } else if (errno != d->errnum) { + sudo_warnx_nodebug("FAIL: %s: errno mismatch, %d != %d", + d->idstr, errno, d->errnum); errors++; } + } else if (errstr != NULL) { + sudo_warnx_nodebug("FAIL: %s: %s", d->idstr, errstr); + errors++; } else if (value != d->id) { sudo_warnx_nodebug("FAIL: %s != %u", d->idstr, (unsigned int)d->id); errors++; diff --git a/lib/util/strtoid.c b/lib/util/strtoid.c index 2339a88..6797074 100644 --- a/lib/util/strtoid.c +++ b/lib/util/strtoid.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2016 Todd C. Miller <Todd.Miller@sudo.ws> + * Copyright (c) 2013-2019 Todd C. Miller <Todd.Miller@sudo.ws> * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -46,6 +46,27 @@ #include "sudo_debug.h" #include "sudo_util.h" +/* + * Make sure that the ID ends with a valid separator char. + */ +static bool +valid_separator(const char *p, const char *ep, const char *sep) +{ + bool valid = false; + debug_decl(valid_separator, SUDO_DEBUG_UTIL) + + if (ep != p) { + /* check for valid separator (including '\0') */ + if (sep == NULL) + sep = ""; + do { + if (*ep == *sep) + valid = true; + } while (*sep++ != '\0'); + } + debug_return_bool(valid); +} + /* * Parse a uid/gid in string form. * If sep is non-NULL, it contains valid separator characters (e.g. comma, space) @@ -60,36 +81,33 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr char *ep; id_t ret = 0; long long llval; - bool valid = false; debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL) /* skip leading space so we can pick up the sign, if any */ while (isspace((unsigned char)*p)) p++; - if (sep == NULL) - sep = ""; + + /* While id_t may be 64-bit signed, uid_t and gid_t are 32-bit unsigned. */ errno = 0; llval = strtoll(p, &ep, 10); - if (ep != p) { - /* check for valid separator (including '\0') */ - do { - if (*ep == *sep) - valid = true; - } while (*sep++ != '\0'); + if ((errno == ERANGE && llval == LLONG_MAX) || llval > (id_t)UINT_MAX) { + errno = ERANGE; + if (errstr != NULL) + *errstr = N_("value too large"); + goto done; } - if (!valid) { + if ((errno == ERANGE && llval == LLONG_MIN) || llval < INT_MIN) { + errno = ERANGE; if (errstr != NULL) - *errstr = N_("invalid value"); - errno = EINVAL; + *errstr = N_("value too small"); goto done; } - if (errno == ERANGE) { - if (errstr != NULL) { - if (llval == LLONG_MAX) - *errstr = N_("value too large"); - else - *errstr = N_("value too small"); - } + + /* Disallow id -1, which means "no change". */ + if (!valid_separator(p, ep, sep) || llval == -1 || llval == (id_t)UINT_MAX) { + if (errstr != NULL) + *errstr = N_("invalid value"); + errno = EINVAL; goto done; } ret = (id_t)llval; @@ -106,30 +124,15 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr { char *ep; id_t ret = 0; - bool valid = false; debug_decl(sudo_strtoid, SUDO_DEBUG_UTIL) /* skip leading space so we can pick up the sign, if any */ while (isspace((unsigned char)*p)) p++; - if (sep == NULL) - sep = ""; + errno = 0; if (*p == '-') { long lval = strtol(p, &ep, 10); - if (ep != p) { - /* check for valid separator (including '\0') */ - do { - if (*ep == *sep) - valid = true; - } while (*sep++ != '\0'); - } - if (!valid) { - if (errstr != NULL) - *errstr = N_("invalid value"); - errno = EINVAL; - goto done; - } if ((errno == ERANGE && lval == LONG_MAX) || lval > INT_MAX) { errno = ERANGE; if (errstr != NULL) @@ -142,28 +145,31 @@ sudo_strtoid_v1(const char *p, const char *sep, char **endp, const char **errstr *errstr = N_("value too small"); goto done; } - ret = (id_t)lval; - } else { - unsigned long ulval = strtoul(p, &ep, 10); - if (ep != p) { - /* check for valid separator (including '\0') */ - do { - if (*ep == *sep) - valid = true; - } while (*sep++ != '\0'); - } - if (!valid) { + + /* Disallow id -1, which means "no change". */ + if (!valid_separator(p, ep, sep) || lval == -1) { if (errstr != NULL) *errstr = N_("invalid value"); errno = EINVAL; goto done; } + ret = (id_t)lval; + } else { + unsigned long ulval = strtoul(p, &ep, 10); if ((errno == ERANGE && ulval == ULONG_MAX) || ulval > UINT_MAX) { errno = ERANGE; if (errstr != NULL) *errstr = N_("value too large"); goto done; } + + /* Disallow id -1, which means "no change". */ + if (!valid_separator(p, ep, sep) || ulval == UINT_MAX) { + if (errstr != NULL) + *errstr = N_("invalid value"); + errno = EINVAL; + goto done; + } ret = (id_t)ulval; } if (errstr != NULL) diff --git a/plugins/sudoers/regress/testsudoers/test5.out.ok b/plugins/sudoers/regress/testsudoers/test5.out.ok index 5e319c9..cecf700 100644 --- a/plugins/sudoers/regress/testsudoers/test5.out.ok +++ b/plugins/sudoers/regress/testsudoers/test5.out.ok @@ -4,7 +4,7 @@ Parse error in sudoers near line 1. Entries for user root: Command unmatched -testsudoers: test5.inc should be owned by gid 4294967295 +testsudoers: test5.inc should be owned by gid 4294967294 Parse error in sudoers near line 1. Entries for user root: diff --git a/plugins/sudoers/regress/testsudoers/test5.sh b/plugins/sudoers/regress/testsudoers/test5.sh index 9e690a6..94d585c 100755 --- a/plugins/sudoers/regress/testsudoers/test5.sh +++ b/plugins/sudoers/regress/testsudoers/test5.sh @@ -24,7 +24,7 @@ EOF # Test group writable chmod 664 $TESTFILE -./testsudoers -U $MYUID -G -1 root id <<EOF +./testsudoers -U $MYUID -G -2 root id <<EOF #include $TESTFILE EOF
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
