From 7b25b1c5b2794a499c8ae99db75830a6d564561e Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 20 Aug 2016 15:58:34 +0200 Subject: [PATCH] hush: do not leak script fds into NOEXEC children We set all opened script fds to CLOEXEC, thus making then go away after fork+exec. Unfortunately, CLOFORK does not exist. NOEXEC children will still see those fds open. For one, "ls" applet is NOEXEC. Therefore running "ls -l /proc/self/fd" in a script from standalone shell shows this: lrwx------ 1 root root 64 Aug 20 15:17 0 -> /dev/pts/3 lrwx------ 1 root root 64 Aug 20 15:17 1 -> /dev/pts/3 lrwx------ 1 root root 64 Aug 20 15:17 2 -> /dev/pts/3 lr-x------ 1 root root 64 Aug 20 15:17 3 -> /path/to/top/level/script lr-x------ 1 root root 64 Aug 20 15:17 4 -> /path/to/sourced/SCRIPT1 ... with as many open fds as there are ". SCRIPTn" nest levels. Fix it by closing these fds after fork (only for NOEXEC children). Signed-off-by: Denys Vlasenko --- shell/hush.c | 78 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index 9b45b312f..6b0d85039 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -711,6 +711,13 @@ enum { }; +/* Can be extended to handle a FIXME in setup_redirects about not saving script fds */ +struct FILE_list { + struct FILE_list *next; + FILE *fp; +}; + + /* "Globals" within this file */ /* Sorted roughly by size (smaller offsets == smaller code) */ struct globals { @@ -801,6 +808,9 @@ struct globals { unsigned count_SIGCHLD; unsigned handled_SIGCHLD; smallint we_have_children; +#endif +#if ENABLE_FEATURE_SH_STANDALONE + struct FILE_list *FILE_list; #endif /* Which signals have non-DFL handler (even with no traps set)? * Set at the start to: @@ -1252,6 +1262,54 @@ static void free_strings(char **strings) } +/* Manipulating the list of open FILEs */ +static FILE *remember_FILE(FILE *fp) +{ + if (fp) { +#if ENABLE_FEATURE_SH_STANDALONE + struct FILE_list *n = xmalloc(sizeof(*n)); + n->fp = fp; + n->next = G.FILE_list; + G.FILE_list = n; +#endif + close_on_exec_on(fileno(fp)); + } + return fp; +} +#if ENABLE_FEATURE_SH_STANDALONE +static void close_all_FILE_list(void) +{ + struct FILE_list *fl = G.FILE_list; + while (fl) { + /* fclose would also free FILE object. + * It is disastrous if we share memory with a vforked parent. + * I'm not sure we never come here after vfork. + * Therefore just close fd, nothing more. + */ + /*fclose(fl->fp); - unsafe */ + close(fileno(fl->fp)); + fl = fl->next; + } +} +static void fclose_and_forget(FILE *fp) +{ + struct FILE_list **pp = &G.FILE_list; + while (*pp) { + struct FILE_list *cur = *pp; + if (cur->fp == fp) { + *pp = cur->next; + free(cur); + break; + } + pp = &cur->next; + } + fclose(fp); +} +#else +# define fclose_and_forget(fp) fclose(fp); +#endif + + /* Helpers for setting new $n and restoring them back */ typedef struct save_arg_t { @@ -5968,8 +6026,7 @@ static FILE *generate_stream_from_string(const char *s, pid_t *pid_p) free(to_free); # endif close(channel[1]); - close_on_exec_on(channel[0]); - return xfdopen_for_read(channel[0]); + return remember_FILE(xfdopen_for_read(channel[0])); } /* Return code is exit status of the process that is run. */ @@ -5998,7 +6055,7 @@ static int process_command_subs(o_string *dest, const char *s) } debug_printf("done reading from `cmd` pipe, closing it\n"); - fclose(fp); + fclose_and_forget(fp); /* We need to extract exitcode. Test case * "true; echo `sleep 1; false` $?" * should print 1 */ @@ -6584,6 +6641,8 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save, if (a >= 0) { # if BB_MMU /* see above why on NOMMU it is not allowed */ if (APPLET_IS_NOEXEC(a)) { + /* Do not leak open fds from opened script files etc */ + close_all_FILE_list(); debug_printf_exec("running applet '%s'\n", argv[0]); run_applet_no_and_exit(a, argv); } @@ -8086,10 +8145,10 @@ int hush_main(int argc, char **argv) debug_printf("sourcing /etc/profile\n"); input = fopen_for_read("/etc/profile"); if (input != NULL) { - close_on_exec_on(fileno(input)); + remember_FILE(input); install_special_sighandlers(); parse_and_run_file(input); - fclose(input); + fclose_and_forget(input); } /* bash: after sourcing /etc/profile, * tries to source (in the given order): @@ -8111,11 +8170,11 @@ int hush_main(int argc, char **argv) G.global_argv++; debug_printf("running script '%s'\n", G.global_argv[0]); input = xfopen_for_read(G.global_argv[0]); - close_on_exec_on(fileno(input)); + remember_FILE(input); install_special_sighandlers(); parse_and_run_file(input); #if ENABLE_FEATURE_CLEAN_UP - fclose(input); + fclose_and_forget(input); #endif goto final_return; } @@ -8983,7 +9042,7 @@ static int FAST_FUNC builtin_source(char **argv) if (arg_path) filename = arg_path; } - input = fopen_or_warn(filename, "r"); + input = remember_FILE(fopen_or_warn(filename, "r")); free(arg_path); if (!input) { /* bb_perror_msg("%s", *argv); - done by fopen_or_warn */ @@ -8992,7 +9051,6 @@ static int FAST_FUNC builtin_source(char **argv) */ return EXIT_FAILURE; } - close_on_exec_on(fileno(input)); #if ENABLE_HUSH_FUNCTIONS sv_flg = G.flag_return_in_progress; @@ -9003,7 +9061,7 @@ static int FAST_FUNC builtin_source(char **argv) save_and_replace_G_args(&sv, argv); parse_and_run_file(input); - fclose(input); + fclose_and_forget(input); if (argv[1]) restore_G_args(&sv, argv);