[git commit] sulogin: remove suid paranoia code, explain why it's not necessary

Denys Vlasenko vda.linux at googlemail.com
Wed Mar 30 15:27:32 UTC 2016


commit: https://git.busybox.net/busybox/commit/?id=32c08acba3d938ec2fa4f9d2ff8160bbe05a20cb
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

function                                             old     new   delta
sulogin_main                                         325     270     -55

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 loginutils/sulogin.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/loginutils/sulogin.c b/loginutils/sulogin.c
index d2ac1f6..33f078a 100644
--- a/loginutils/sulogin.c
+++ b/loginutils/sulogin.c
@@ -32,6 +32,14 @@ int sulogin_main(int argc UNUSED_PARAM, char **argv)
 	struct passwd *pwd;
 	const char *shell;
 
+	/* Note: sulogin is not a suid app. It is meant to be run by init
+	 * for single user / emergency mode. init starts it as root.
+	 * Normal users (potentially malisious ones) can only run it under
+	 * their UID, therefore no paranoia here is warranted:
+	 * $LD_LIBRARY_PATH in env, TTY = /dev/sda
+	 * are no more dangerous here than in e.g. cp applet.
+	 */
+
 	logmode = LOGMODE_BOTH;
 	openlog(applet_name, 0, LOG_AUTH);
 
@@ -47,18 +55,9 @@ int sulogin_main(int argc UNUSED_PARAM, char **argv)
 		dup(0);
 	}
 
-	/* Malicious use like "sulogin /dev/sda"? */
-	if (!isatty(0) || !isatty(1) || !isatty(2)) {
-		logmode = LOGMODE_SYSLOG;
-		bb_error_msg_and_die("not a tty");
-	}
-
-	/* Clear dangerous stuff, set PATH */
-	sanitize_env_if_suid();
-
 	pwd = getpwuid(0);
 	if (!pwd) {
-		goto auth_error;
+		bb_error_msg_and_die("no password entry for root");
 	}
 
 	while (1) {
@@ -92,7 +91,4 @@ int sulogin_main(int argc UNUSED_PARAM, char **argv)
 
 	/* Exec login shell with no additional parameters. Never returns. */
 	run_shell(shell, 1, NULL, NULL);
-
- auth_error:
-	bb_error_msg_and_die("no password entry for root");
 }


More information about the busybox-cvs mailing list