From: Ryan Mallon <rmallon@gmail.com>
Date: Tue, 8 Oct 2013 12:53:29 +0000 (+0200)
Subject: wall,crontab: use xopen_as_uid_gid()
X-Git-Tag: 1_22_0~79
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=1d30b3f1f66a0cd179f47082245079ef357b6a66;p=oweals%2Fbusybox.git

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>
---

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])