diff options
author | Denys Vlasenko | 2016-08-20 15:58:34 +0200 |
---|---|---|
committer | Denys Vlasenko | 2016-08-20 15:58:34 +0200 |
commit | 7b25b1c5b2794a499c8ae99db75830a6d564561e (patch) | |
tree | c136ae68fd879d80277eebac0ef7686b749181df | |
parent | 869994cf4f9647fdfb519a1945f8582e71d3df3d (diff) | |
download | busybox-7b25b1c5b2794a499c8ae99db75830a6d564561e.zip busybox-7b25b1c5b2794a499c8ae99db75830a6d564561e.tar.gz |
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 <vda.linux@googlemail.com>
-rw-r--r-- | shell/hush.c | 78 |
1 files changed, 68 insertions, 10 deletions
diff --git a/shell/hush.c b/shell/hush.c index 9b45b31..6b0d850 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 { @@ -802,6 +809,9 @@ struct globals { 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: * (SIGQUIT + maybe SPECIAL_INTERACTIVE_SIGS + maybe SPECIAL_JOBSTOP_SIGS) @@ -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); |