diff options
| author | Ryan Mallon <rmallon@gmail.com> | 2013-10-08 14:53:29 +0200 |
|---|---|---|
| committer | Denys Vlasenko <vda.linux@googlemail.com> | 2013-10-08 14:53:29 +0200 |
| commit | 1d30b3f1f66a0cd179f47082245079ef357b6a66 (patch) | |
| tree | 2a5eaf34ebb770e2d4d499338e6a4c82a22d3086 | |
| parent | 5906a5c26c392b9687d14951a6da3a5195b576be (diff) | |
| download | busybox-1d30b3f1f66a0cd179f47082245079ef357b6a66.tar.gz | |
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 <rmallon@gmail.com>
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
| -rw-r--r-- | miscutils/crontab.c | 27 | ||||
| -rw-r--r-- | 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]) |