From 1d30b3f1f66a0cd179f47082245079ef357b6a66 Mon Sep 17 00:00:00 2001 From: Ryan Mallon Date: Tue, 8 Oct 2013 14:53:29 +0200 Subject: [PATCH] wall,crontab: use xopen_as_uid_gid() This fixes a narrow security race in crontab. function old new delta xopen_as_uid_gid - 80 +80 seteuid - 64 +64 setegid - 64 +64 setreuid - 37 +37 xseteuid - 22 +22 xsetegid - 22 +22 crontab_main 590 577 -13 setfsuid 33 - -33 setfsgid 33 - -33 wall_main 138 102 -36 open_as_user 109 - -109 text data bss dec hex filename 893539 497 7568 901604 dc1e4 busybox_old 893618 497 7568 901683 dc233 busybox_unstripped Signed-off-by: Ryan Mallon Signed-off-by: Denys Vlasenko --- miscutils/crontab.c | 27 +-------------------------- miscutils/wall.c | 6 +----- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/miscutils/crontab.c b/miscutils/crontab.c index 4731d8da6..aad242fd8 100644 --- a/miscutils/crontab.c +++ b/miscutils/crontab.c @@ -55,28 +55,6 @@ static void edit_file(const struct passwd *pas, const char *file) bb_perror_msg_and_die("can't execute '%s'", ptr); } -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); -} - int crontab_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int crontab_main(int argc UNUSED_PARAM, char **argv) { @@ -137,10 +115,7 @@ int crontab_main(int argc UNUSED_PARAM, char **argv) if (!argv[0]) bb_show_usage(); if (NOT_LONE_DASH(argv[0])) { - src_fd = open_as_user(pas, argv[0]); - if (src_fd < 0) - bb_error_msg_and_die("user %s cannot read %s", - pas->pw_name, argv[0]); + src_fd = xopen_as_uid_gid(argv[0], O_RDONLY, pas->pw_uid, pas->pw_gid); } } diff --git a/miscutils/wall.c b/miscutils/wall.c index c74f4f27b..bb709ee39 100644 --- a/miscutils/wall.c +++ b/miscutils/wall.c @@ -41,11 +41,7 @@ int wall_main(int argc UNUSED_PARAM, char **argv) /* The applet is setuid. * Access to the file must be under user's uid/gid. */ - setfsuid(getuid()); - setfsgid(getgid()); - fd = xopen(argv[1], O_RDONLY); - setfsuid(geteuid()); - setfsgid(getegid()); + fd = xopen_as_uid_gid(argv[1], O_RDONLY, getuid(), getgid()); } msg = xmalloc_read(fd, NULL); if (ENABLE_FEATURE_CLEAN_UP && argv[1])