From 52816302299854ba1644fce98b5d19db526e6c29 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Tue, 6 Nov 2007 05:26:51 +0000 Subject: [PATCH] login: clear dangerous environment variables if started by non-root --- include/libbb.h | 2 ++ libbb/login.c | 26 ++++++++++++++++++++++++++ loginutils/login.c | 14 ++++++++++++-- loginutils/sulogin.c | 25 ++----------------------- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/include/libbb.h b/include/libbb.h index 77c678cee..f79d80d7f 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -623,6 +623,8 @@ enum { #endif void bb_daemonize_or_rexec(int flags, char **argv); void bb_sanitize_stdio(void); +/* Clear dangerous stuff, set PATH */ +void sanitize_env_for_suid(void); extern const char *opt_complementary; diff --git a/libbb/login.c b/libbb/login.c index 308e1bfed..1af3165b9 100644 --- a/libbb/login.c +++ b/libbb/login.c @@ -99,3 +99,29 @@ void print_login_prompt(void) fputs(LOGIN, stdout); fflush(stdout); } + +/* Clear dangerous stuff, set PATH */ +static const char forbid[] ALIGN1 = + "ENV" "\0" + "BASH_ENV" "\0" + "HOME" "\0" + "IFS" "\0" + "SHELL" "\0" + "LD_LIBRARY_PATH" "\0" + "LD_PRELOAD" "\0" + "LD_TRACE_LOADED_OBJECTS" "\0" + "LD_BIND_NOW" "\0" + "LD_AOUT_LIBRARY_PATH" "\0" + "LD_AOUT_PRELOAD" "\0" + "LD_NOWARN" "\0" + "LD_KEEPDIR" "\0"; + +void sanitize_env_for_suid(void) +{ + const char *p = forbid; + do { + unsetenv(p); + p += strlen(p) + 1; + } while (*p); + putenv((char*)bb_PATH_root_path); +} diff --git a/loginutils/login.c b/loginutils/login.c index bddc0f533..c05edde36 100644 --- a/loginutils/login.c +++ b/loginutils/login.c @@ -201,7 +201,7 @@ static void motd(void) int fd; fd = open(bb_path_motd_file, O_RDONLY); - if (fd) { + if (fd >= 0) { fflush(stdout); bb_copyfd_eof(fd, STDOUT_FILENO); close(fd); @@ -216,6 +216,10 @@ static void alarm_handler(int sig ATTRIBUTE_UNUSED) ndelay_on(1); ndelay_on(2); printf("\r\nLogin timed out after %d seconds\r\n", TIMEOUT); + /* unix API is brain damaged regarding O_NONBLOCK, + * we should undo it, or else we can affect other processes */ + ndelay_off(1); + ndelay_off(2); exit(EXIT_SUCCESS); } @@ -254,6 +258,11 @@ int login_main(int argc, char **argv) * and any extra open fd's are closed. * (The name of the function is misleading. Not daemonizing here.) */ bb_daemonize_or_rexec(DAEMON_ONLY_SANITIZE | DAEMON_CLOSE_EXTRA_FDS, NULL); + /* More of suid paranoia if called by non-root */ + if (!amroot) { + /* Clear dangerous stuff, set PATH */ + sanitize_env_for_suid(); + } opt = getopt32(argv, "f:h:p", &opt_user, &opt_host); if (opt & LOGIN_OPT_f) { @@ -411,7 +420,8 @@ int login_main(int argc, char **argv) fchown(0, pw->pw_uid, pw->pw_gid); fchmod(0, 0600); - if (ENABLE_LOGIN_SCRIPTS) { + /* We trust environment only if we run by root */ + if (ENABLE_LOGIN_SCRIPTS && amroot) { char *t_argv[2]; t_argv[0] = getenv("LOGIN_PRE_SUID_SCRIPT"); diff --git a/loginutils/sulogin.c b/loginutils/sulogin.c index f1545b78f..af457ef1e 100644 --- a/loginutils/sulogin.c +++ b/loginutils/sulogin.c @@ -9,22 +9,6 @@ #include "libbb.h" -static const char forbid[] ALIGN1 = - "ENV" "\0" - "BASH_ENV" "\0" - "HOME" "\0" - "IFS" "\0" - "PATH" "\0" - "SHELL" "\0" - "LD_LIBRARY_PATH" "\0" - "LD_PRELOAD" "\0" - "LD_TRACE_LOADED_OBJECTS" "\0" - "LD_BIND_NOW" "\0" - "LD_AOUT_LIBRARY_PATH" "\0" - "LD_AOUT_PRELOAD" "\0" - "LD_NOWARN" "\0" - "LD_KEEPDIR" "\0"; - //static void catchalarm(int ATTRIBUTE_UNUSED junk) //{ // exit(EXIT_FAILURE); @@ -37,7 +21,6 @@ int sulogin_main(int argc, char **argv) char *cp; int timeout = 0; char *timeout_arg; - const char *p; struct passwd *pwd; const char *shell; #if ENABLE_FEATURE_SHADOWPASSWDS @@ -66,12 +49,8 @@ int sulogin_main(int argc, char **argv) bb_error_msg_and_die("not a tty"); } - /* Clear out anything dangerous from the environment */ - p = forbid; - do { - unsetenv(p); - p += strlen(p) + 1; - } while (*p); + /* Clear dangerous stuff, set PATH */ + sanitize_env_for_suid(); // bb_askpass() already handles this // signal(SIGALRM, catchalarm);