sudo: CVE-2015-5602 the patch is based on: https://www.sudo.ws/repos/sudo/rev/9636fd256325 https://www.sudo.ws/repos/sudo/rev/c2e36a80a279 Rewritten sudoedit_checkdir support that checks all the dirs in the path and refuses to follow symlinks in writable directories. This is a better fix for CVE-2015-5602. Adapted from a diff by Ben Hutchings. Bug #707 Signed-off-by: Li Wang --- plugins/sudoers/policy.c | 5 src/sudo.c | 10 + src/sudo.h | 3 src/sudo_edit.c | 289 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 296 insertions(+), 11 deletions(-) --- a/src/sudo_edit.c +++ b/src/sudo_edit.c @@ -79,6 +79,267 @@ switch_user(uid_t euid, gid_t egid, int debug_return; } +static bool +group_matches(gid_t target, gid_t gid, int ngroups, GETGROUPS_T *groups) +{ + int i; + debug_decl(group_matches, SUDO_DEBUG_EDIT) + + if (target == gid) { + sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, + "user gid %u matches directory gid %u", (unsigned int)gid, + (unsigned int)target); + debug_return_bool(true); + } + for (i = 0; i < ngroups; i++) { + if (target == groups[i]) { + sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, + "user gid %u matches directory gid %u", (unsigned int)gid, + (unsigned int)target); + debug_return_bool(true); + } + } + debug_return_bool(false); +} + +#ifdef O_NOFOLLOW +static int +sudo_edit_openat_nofollow(int dfd, char *path, int oflags, mode_t mode) +{ + debug_decl(sudo_edit_open_nofollow, SUDO_DEBUG_EDIT) + + debug_return_int(openat(dfd, path, oflags|O_NOFOLLOW, mode)); +} +#else +/* + * Returns true if fd and path don't match or path is a symlink. + * Used on older systems without O_NOFOLLOW. + */ +static bool +sudo_edit_is_symlink(int fd, char *path) +{ + struct stat sb1, sb2; + debug_decl(sudo_edit_is_symlink, SUDO_DEBUG_EDIT) + + /* + * Treat [fl]stat() failure like there was a symlink. + */ + if (fstat(fd, &sb1) == -1 || lstat(path, &sb2) == -1) + debug_return_bool(true); + + /* + * Make sure we did not open a link and that what we opened + * matches what is currently on the file system. + */ + if (S_ISLNK(sb2.st_mode) || + sb1.st_dev != sb2.st_dev || sb1.st_ino != sb2.st_ino) { + debug_return_bool(true); + } + + debug_return_bool(false); +} + +static int +sudo_edit_openat_nofollow(char *path, int oflags, mode_t mode) +{ + struct stat sb1, sb2; + int fd; + debug_decl(sudo_edit_openat_nofollow, SUDO_DEBUG_EDIT) + + fd = openat(dfd, path, oflags, mode); + if (fd == -1) + debug_return_int(-1); + + if (sudo_edit_is_symlink(fd, path)) { + close(fd); + fd = -1; + errno = ELOOP; + } + + debug_return_int(fd); +} +#endif /* O_NOFOLLOW */ + +/* + * Returns true if the directory described by sb is writable + * by the user. We treat directories with the sticky bit as + * unwritable unless they are owned by the user. + */ +static bool +dir_is_writable(struct stat *sb, uid_t uid, gid_t gid, int ngroups, + GETGROUPS_T *groups) +{ + debug_decl(dir_is_writable, SUDO_DEBUG_EDIT) + + /* If the user owns the dir we always consider it writable. */ + if (sb->st_uid == uid) { + sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, + "user uid %u matches directory uid %u", (unsigned int)uid, + (unsigned int)sb->st_uid); + debug_return_bool(true); + } + + /* Other writable? */ + if (ISSET(sb->st_mode, S_IWOTH)) { + sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, + "directory is writable by other"); + debug_return_bool(true); + } + + /* Group writable? */ + if (ISSET(sb->st_mode, S_IWGRP)) { + if (group_matches(sb->st_gid, gid, ngroups, groups)) { + sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, + "directory is writable by one of the user's groups"); + debug_return_bool(true); + } + } + + debug_return_bool(false); +} + +/* + * Directory open flags for use with openat(2) and fstat(2). + * Use O_PATH and O_DIRECTORY where possible. + */ +#if defined(O_PATH) && defined(O_DIRECTORY) +# define DIR_OPEN_FLAGS (O_PATH|O_DIRECTORY) +#elif defined(O_PATH) && !defined(O_DIRECTORY) +# define DIR_OPEN_FLAGS O_PATH +#elif !defined(O_PATH) && defined(O_DIRECTORY) +# define DIR_OPEN_FLAGS (O_RDONLY|O_DIRECTORY) +#else +# define DIR_OPEN_FLAGS (O_RDONLY|O_NONBLOCK) +#endif + +static int +sudo_edit_open_nonwritable(char *path, int oflags, mode_t mode) +{ + int dfd, fd, dflags = DIR_OPEN_FLAGS; +#if defined(__linux__) && defined(O_PATH) + char *opath = path; +#endif + bool is_writable; + struct stat sb; + debug_decl(sudo_edit_open_nonwritable, SUDO_DEBUG_EDIT) + +#if defined(__linux__) && defined(O_PATH) +restart: +#endif + if (path[0] == '/') { + dfd = open("/", dflags); + path++; + } else { + dfd = open(".", dflags); + if (path[0] == '.' && path[1] == '/') + path += 2; + } + if (dfd == -1) + debug_return_int(-1); + + for (;;) { + char *slash; + int subdfd; + + /* + * Look up one component at a time, avoiding symbolic links in + * writable directories. + */ + if (fstat(dfd, &sb) == -1) { + close(dfd); +#if defined(__linux__) && defined(O_PATH) + /* Linux prior to 3.6 can't fstat an O_PATH fd */ + if (ISSET(dflags, O_PATH)) { + CLR(dflags, O_PATH); + path = opath; + goto restart; + } +#endif + debug_return_int(-1); + } +#ifndef O_DIRECTORY + if (!S_ISDIR(sb.st_mode)) { + close(dfd); + errno = ENOTDIR; + debug_return_int(-1); + } +#endif + is_writable = dir_is_writable(&sb, user_details.uid, user_details.gid, + user_details.ngroups, user_details.groups); + + while (path[0] == '/') + path++; + slash = strchr(path, '/'); + if (slash == NULL) + break; + *slash = '\0'; + if (is_writable) + subdfd = sudo_edit_openat_nofollow(dfd, path, dflags, 0); + else + subdfd = openat(dfd, path, dflags, 0); + *slash = '/'; /* restore path */ + close(dfd); + if (subdfd == -1) + debug_return_int(-1); + path = slash + 1; + dfd = subdfd; + } + + if (is_writable) { + close(dfd); + errno = EISDIR; + debug_return_int(-1); + } + + fd = openat(dfd, path, oflags, mode); + close(dfd); + debug_return_int(fd); +} + +#ifdef O_NOFOLLOW +static int +sudo_edit_open(char *path, int oflags, mode_t mode, int sflags) +{ + int fd; + debug_decl(sudo_edit_open, SUDO_DEBUG_EDIT) + + if (!ISSET(sflags, CD_SUDOEDIT_FOLLOW)) + oflags |= O_NOFOLLOW; + if (ISSET(sflags, CD_SUDOEDIT_CHECKDIR) && user_details.uid != 0) + fd = sudo_edit_open_nonwritable(path, oflags|O_NONBLOCK, mode); + else + fd = open(path, oflags|O_NONBLOCK, mode); + if (fd != -1 && !ISSET(oflags, O_NONBLOCK)) + (void) fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK); + debug_return_int(fd); +} +#else +static int +sudo_edit_open(char *path, int oflags, mode_t mode, int sflags) +{ + struct stat sb1, sb2; + int fd; + debug_decl(sudo_edit_open, SUDO_DEBUG_EDIT) + + if (ISSET(sflags, CD_SUDOEDIT_CHECKDIR) && user_details.uid != 0) + fd = sudo_edit_open_nonwritable(path, oflags|O_NONBLOCK, mode); + else + fd = open(path, oflags|O_NONBLOCK, mode); + if (fd == -1) + debug_return_int(-1); + if (!ISSET(oflags, O_NONBLOCK)) + (void) fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK); + + if (!ISSET(sflags, CD_SUDOEDIT_FOLLOW) && sudo_edit_is_symlink(fd, path)) { + close(fd); + fd = -1; + errno = ELOOP; + } + + debug_return_int(fd); +} +#endif /* O_NOFOLLOW */ + /* * Wrapper to allow users to edit privileged files with their own uid. */ @@ -97,8 +358,8 @@ sudo_edit(struct command_details *comman struct tempfile { char *tfile; char *ofile; - struct timeval omtim; off_t osize; + struct timeval omtim; } *tf = NULL; debug_decl(sudo_edit, SUDO_DEBUG_EDIT) @@ -153,7 +414,8 @@ sudo_edit(struct command_details *comman rc = -1; switch_user(command_details->euid, command_details->egid, command_details->ngroups, command_details->groups); - if ((ofd = open(files[i], O_RDONLY, 0644)) != -1 || errno == ENOENT) { + ofd = sudo_edit_open(files[i], O_RDONLY, 0644, command_details->flags); + if (ofd != -1 || errno == ENOENT) { if (ofd == -1) { zero_bytes(&sb, sizeof(sb)); /* new file */ rc = 0; @@ -163,11 +425,17 @@ sudo_edit(struct command_details *comman } switch_user(ROOT_UID, user_details.egid, user_details.ngroups, user_details.groups); - if (rc || (ofd != -1 && !S_ISREG(sb.st_mode))) { - if (rc) - warning("%s", files[i]); + if (ofd != -1 && !S_ISREG(sb.st_mode)) { + warningx(_("%s: not a regular file"), files[i]); + close(ofd); + continue; + } + if (rc == -1) { + /* open() or fstat() error. */ + if (ofd == -1 && errno == ELOOP) + warningx(_("%s: is a symbolic link"), files[i]); else - warningx(_("%s: not a regular file"), files[i]); + warning("%s", files[i]); if (ofd != -1) close(ofd); continue; @@ -258,9 +526,9 @@ sudo_edit(struct command_details *comman rc = -1; if (seteuid(user_details.uid) != 0) fatal("seteuid(%d)", (int)user_details.uid); - if ((tfd = open(tf[i].tfile, O_RDONLY, 0644)) != -1) { + tfd = sudo_edit_open(tf[i].tfile, O_RDONLY, 0644, 0); + if (tfd != -1) rc = fstat(tfd, &sb); - } if (seteuid(ROOT_UID) != 0) fatal("seteuid(ROOT_UID)"); if (rc || !S_ISREG(sb.st_mode)) { @@ -289,8 +557,9 @@ sudo_edit(struct command_details *comman } switch_user(command_details->euid, command_details->egid, command_details->ngroups, command_details->groups); - ofd = open(tf[i].ofile, O_WRONLY|O_TRUNC|O_CREAT, 0644); - switch_user(ROOT_UID, user_details.egid, + ofd = sudo_edit_open(tf[i].ofile, O_WRONLY|O_TRUNC|O_CREAT, 0644, + command_details->flags); + switch_user(ROOT_UID, user_details.egid, user_details.ngroups, user_details.groups); if (ofd == -1) { warning(_("unable to write to %s"), tf[i].ofile); --- a/plugins/sudoers/policy.c +++ b/plugins/sudoers/policy.c @@ -383,8 +383,11 @@ sudoers_policy_exec_setup(char *argv[], easprintf(&command_info[info_len++], "maxseq=%u", def_maxseq); } } - if (ISSET(sudo_mode, MODE_EDIT)) + if (ISSET(sudo_mode, MODE_EDIT)) { command_info[info_len++] = estrdup("sudoedit=true"); + command_info[info_len++] = estrdup("sudoedit_checkdir=true"); + command_info[info_len++] = estrdup("sudoedit_follow=true"); + } if (ISSET(sudo_mode, MODE_LOGIN_SHELL)) { /* Set cwd to run user's homedir. */ command_info[info_len++] = fmt_string("cwd", runas_pw->pw_dir); --- a/src/sudo.c +++ b/src/sudo.c @@ -727,6 +727,16 @@ command_info_to_details(char * const inf SET(details->flags, CD_SUDOEDIT); break; } + if (strncmp("sudoedit_checkdir=", info[i], sizeof("sudoedit_checkdir=") - 1) == 0) { + if (atobool(info[i] + sizeof("sudoedit_checkdir=") - 1) == true) + SET(details->flags, CD_SUDOEDIT_CHECKDIR); + break; + } + if (strncmp("sudoedit_follow=", info[i], sizeof("sudoedit_follow=") - 1) == 0) { + if (atobool(info[i] + sizeof("sudoedit_follow=") - 1) == true) + SET(details->flags, CD_SUDOEDIT_FOLLOW); + break; + } break; case 't': if (strncmp("timeout=", info[i], sizeof("timeout=") - 1) == 0) { --- a/src/sudo.h +++ b/src/sudo.h @@ -129,6 +129,9 @@ struct user_details { #define CD_USE_PTY 0x1000 #define CD_SET_UTMP 0x2000 #define CD_EXEC_BG 0x4000 +#define CD_SUDOEDIT_COPY 0x08000 +#define CD_SUDOEDIT_FOLLOW 0x10000 +#define CD_SUDOEDIT_CHECKDIR 0x20000 struct command_details { uid_t uid;