isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rewritten sudoedit_checkdir support that checks all the dirs in the

Browse Source 
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
This commit is contained in:
Todd C. Miller
2016-01-10 18:31:29 -07:00
parent 39db87e62b
commit 68c1073fe5
7 changed files with 187 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
doc/CONTRIBUTORS
View File

@@ -58,6 +58,7 @@ you believe you should be listed, please send a note to sudo@sudo.ws.
Holloway, Nick
Hoover, Adam
Hunter, Michael T.
Hutchings, Ben
Irrgang, Eric
Jackson, Brian
Jackson, John R.

9
doc/UPGRADE
View File

@@ -1,6 +1,15 @@
Notes on upgrading from an older release
========================================
o Upgrading from a version prior to 1.8.16:
The meaning of the sudoedit_checkdir sudoers option has changed
in 1.8.16. Previously, it would only check the parent directory
of the file to be edited. In 1.8.16 and higher all directories
in the path to be edited are checked and sudoedit will refuse
to follow a symbolic link in a directory that is writable by
the invoking user.
o Upgrading from a version prior to 1.8.15:
Prior to version 1.8.15, when env_reset was enabled (the default)

17
doc/sudoers.cat
View File

@@ -1291,12 +1291,15 @@ SSUUDDOOEERRSS OOPPTTIIOONNSS
system call. This flag is _o_f_f by default.
sudoedit_checkdir
If set, ssuuddooeeddiitt will refuse to edit files located in a
directory that is writable by the invoking user unless
it is run by root. On many systems, this option
requires that the parent directory of the file to be
edited be readable by the target user. This flag is
_o_f_f by default.
If set, ssuuddooeeddiitt will check directories in the path to
be edited for writability by the invoking user.
Symbolic links will not be followed in writable
directories and ssuuddooeeddiitt will also refuse to edit a
file located in a writable directory. Theses
restrictions are not enforced when ssuuddooeeddiitt is invoked
as root. On many systems, this option requires that
all directories in the path to be edited be readable by
the target user. This flag is _o_f_f by default.
sudoedit_follow By default, ssuuddooeeddiitt will not follow symbolic links
when opening files. The _s_u_d_o_e_d_i_t___f_o_l_l_o_w option can be
@@ -2495,4 +2498,4 @@ DDIISSCCLLAAIIMMEERR
file distributed with ssuuddoo or https://www.sudo.ws/license.html for
complete details.
Sudo 1.8.16 January 4, 2015 Sudo 1.8.16
Sudo 1.8.16 January 9, 2016 Sudo 1.8.16

16
doc/sudoers.man.in
View File

@@ -21,7 +21,7 @@
.\" Agency (DARPA) and Air Force Research Laboratory, Air Force
.\" Materiel Command, USAF, under agreement number F39502-99-1-0512.
.\"
.TH "SUDOERS" "5" "January 4, 2015" "Sudo @PACKAGE_VERSION@" "File Formats Manual"
.TH "SUDOERS" "5" "January 9, 2016" "Sudo @PACKAGE_VERSION@" "File Formats Manual"
.nh
.if n .ad l
.SH "NAME"
@@ -2747,10 +2747,16 @@ sudoedit_checkdir
.br
If set,
\fBsudoedit\fR
will refuse to edit files located in a directory that is writable
by the invoking user unless it is run by root.
On many systems, this option requires that the parent directory
of the file to be edited be readable by the target user.
will check directories in the path to be edited for writability
by the invoking user.
Symbolic links will not be followed in writable directories and
\fBsudoedit\fR
will also refuse to edit a file located in a writable directory.
Theses restrictions are not enforced when
\fBsudoedit\fR
is invoked as root.
On many systems, this option requires that all directories
in the path to be edited be readable by the target user.
This flag is
\fIoff\fR
by default.

16
doc/sudoers.mdoc.in
View File

@@ -19,7 +19,7 @@
.\" Agency (DARPA) and Air Force Research Laboratory, Air Force
.\" Materiel Command, USAF, under agreement number F39502-99-1-0512.
.\"
.Dd January 4, 2015
.Dd January 9, 2016
.Dt SUDOERS @mansectform@
.Os Sudo @PACKAGE_VERSION@
.Sh NAME
@@ -2581,10 +2581,16 @@ by default.
.It sudoedit_checkdir
If set,
.Nm sudoedit
will refuse to edit files located in a directory that is writable
by the invoking user unless it is run by root.
On many systems, this option requires that the parent directory
of the file to be edited be readable by the target user.
will check directories in the path to be edited for writability
by the invoking user.
Symbolic links will not be followed in writable directories and
.Nm sudoedit
will also refuse to edit a file located in a writable directory.
Theses restrictions are not enforced when
.Nm sudoedit
is invoked as root.
On many systems, this option requires that all directories
in the path to be edited be readable by the target user.
This flag is
.Em off
by default.

2
include/sudo_compat.h
View File

@@ -182,6 +182,8 @@
# ifndef UTIME_NOW
# define UTIME_NOW -2L
# endif
#endif
#if !defined(HAVE_OPENAT) || (!defined(HAVE_FUTIMENS) && !defined(HAVE_UTIMENSAT))
# ifndef AT_FDCWD
# define AT_FDCWD -100
# endif

198
src/sudo_edit.c
View File

@@ -179,13 +179,15 @@ group_matches(gid_t target, gid_t gid, int ngroups, GETGROUPS_T *groups)
}
#ifndef HAVE_OPENAT
/* This does not support AT_FDCWD... */
static int
sudo_openat(int dfd, const char *path, int flags, mode_t mode)
{
int fd, odfd;
debug_decl(sudo_openat, SUDO_DEBUG_EDIT)
if (dfd == AT_FDCWD)
debug_return_int(open(path, flags, mode));
/* Save cwd */
if ((odfd = open(".", O_RDONLY)) == -1)
debug_return_int(-1);
@@ -207,6 +209,64 @@ sudo_openat(int dfd, const char *path, int flags, mode_t mode)
#define openat sudo_openat
#endif /* HAVE_OPENAT */
#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
@@ -245,55 +305,100 @@ dir_is_writable(struct stat *sb, uid_t uid, gid_t gid, int ngroups,
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)
{
char *base, *dir;
int dfd, fd, dflags = DIR_OPEN_FLAGS;
#if defined(__linux__) && defined(O_PATH)
char *opath = path;
#endif
bool is_writable;
struct stat sb;
int dfd, fd;
debug_decl(sudo_edit_open_nonwritable, SUDO_DEBUG_EDIT)
base = strrchr(path, '/');
if (base != NULL) {
*base++ = '\0';
dir = path;
} else {
base = path;
dir = ".";
}
#ifdef O_PATH
if ((dfd = open(dir, O_PATH)) != -1) {
/* Linux kernels < 3.6 can't do fstat on O_PATH fds. */
if (fstat(dfd, &sb) == -1) {
close(dfd);
dfd = open(dir, O_RDONLY);
if (fstat(dfd, &sb) == -1) {
close(dfd);
dfd = -1;
}
}
}
#else
if ((dfd = open(dir, O_RDONLY)) != -1) {
if (fstat(dfd, &sb) == -1) {
close(dfd);
dfd = -1;
}
}
#if defined(__linux__) && defined(O_PATH)
restart:
#endif
if (base != path)
base[-1] = '/'; /* restore path */
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);
if (dir_is_writable(&sb, user_details.uid, user_details.gid,
user_details.ngroups, user_details.groups)) {
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, base, oflags, mode);
fd = openat(dfd, path, oflags, mode);
close(dfd);
debug_return_int(fd);
}
@@ -332,27 +437,10 @@ sudo_edit_open(char *path, int oflags, mode_t mode, int sflags)
if (!ISSET(oflags, O_NONBLOCK))
(void) fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
/*
* Treat [fl]stat() failure like an open() failure.
*/
if (fstat(fd, &sb1) == -1 || lstat(path, &sb2) == -1) {
const int serrno = errno;
if (!ISSET(sflags, CD_SUDOEDIT_FOLLOW) && sudo_edit_is_symlink(fd, path)) {
close(fd);
errno = serrno;
debug_return_int(-1);
}
/*
* Make sure we did not open a link and that what we opened
* matches what is currently on the file system.
*/
if (!ISSET(sflags, CD_SUDOEDIT_FOLLOW)) {
if (S_ISLNK(sb2.st_mode) ||
sb1.st_dev != sb2.st_dev || sb1.st_ino != sb2.st_ino) {
close(fd);
errno = ELOOP;
debug_return_int(-1);
}
fd = -1;
errno = ELOOP;
}
debug_return_int(fd);