The open_as_user() function in miscutils/crontab.c uses a child process to drop privileges and check that the file is readable by the specified user. If the child is successful then the (setuid) parent opens the file.
This is racy because the file can be changed on disk (e.g. modified symlink) between the child exiting and the parent re-opening the file. This could allow an non-privileged user to pass a file to crontab that it does not have read access to. In practice this race is probably easy to win since a schedule must occur before the parent re-opens the file. Fix the race by using saving the effective uid/gid, and then setting the effective uid/gid to the specified user. The file is opened as the specified user, and then the old privileges are restored. Signed-off-by: Ryan Mallon <[email protected]> --- diff --git a/miscutils/crontab.c b/miscutils/crontab.c index 4731d8d..b506935 100644 --- a/miscutils/crontab.c +++ b/miscutils/crontab.c @@ -57,24 +57,32 @@ static void edit_file(const struct passwd *pas, const char *file) static int open_as_user(const struct passwd *pas, const char *file) { - pid_t pid; - char c; - - pid = xvfork(); - if (pid) { /* PARENT */ - if (wait4pid(pid) == 0) { - /* exitcode 0: child says it can read */ - return open(file, O_RDONLY); - } - return -1; - } - - /* CHILD */ - /* initgroups, setgid, setuid */ - change_identity(pas); - /* We just try to read one byte. If it works, file is readable - * under this user. We signal that by exiting with 0. */ - _exit(safe_read(xopen(file, O_RDONLY), &c, 1) < 0); + uid_t old_euid; + gid_t old_egid; + int fd, err; + + /* Save and drop privileges */ + old_euid = geteuid(); + old_egid = getegid(); + err = setegid(pas->pw_gid); + if (err) + bb_perror_msg_and_die("failed to drop privileges"); + err = seteuid(pas->pw_uid); + if (err) + bb_perror_msg_and_die("failed to drop privileges"); + + /* Open the file as the specified user */ + fd = xopen(file, O_RDONLY); + + /* Restore privileges */ + err = setegid(old_egid); + if (err) + bb_perror_msg_and_die("failed to restore privileges"); + err = seteuid(old_euid); + if (err) + bb_perror_msg_and_die("failed to restore privileges"); + + return fd; } int crontab_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
