|
Message-ID: <1411691031.31097.1.camel@16bits.net> Date: Fri, 26 Sep 2014 02:23:51 +0200 From: Ángel González <angel@...its.net> To: oss-security@...ts.openwall.com Subject: Re: Non-upstream patches for bash Forwarding to the oss-security thread the patch I sent to bug-bash 1 hour ago. The trick here is to delay parsing of functions coming from the environment until they are actually needed. Thus extra code (CVE-2014-6271) or even a parsing vulnerability like CVE-2014-7169 won't be triggered unless you attempt to run the exported function (or you use a builtin such as declare or type that must print the code, things like type -t are safe to use). It can be applied standalone (and remain compatible with older bash versions), or it could be combined with some of the other patches. It also makes bash more efficient by not parsing unused functions :) Although it passes the testsuite, it has only been lightly tested, don't install in your nuclear plant yet. 😉 Best regards --------- Original message -------- Subject: Re: Issues with exported functions Date: Fri, 26 Sep 2014 00:58:43 +0200 Lazily import functions coming from the environment diff --git a/variables.h b/variables.h index 1a783b9..f496b9d 100644 --- a/variables.h +++ b/variables.h @@ -125,6 +125,7 @@ typedef struct _vlist { #define att_imported 0x0008000 /* came from environment */ #define att_special 0x0010000 /* requires special handling */ #define att_nofree 0x0020000 /* do not free value on unset */ +#define att_unparsed 0x0040000 /* it was not parsed after loading from environ */ #define attmask_int 0x00ff000 @@ -153,6 +154,7 @@ typedef struct _vlist { #define imported_p(var) ((((var)->attributes) & (att_imported))) #define specialvar_p(var) ((((var)->attributes) & (att_special))) #define nofree_p(var) ((((var)->attributes) & (att_nofree))) +#define unparsed_p(var) ((((var)->attributes) & (att_unparsed))) #define tempvar_p(var) ((((var)->attributes) & (att_tempvar))) diff --git a/variables.c b/variables.c index 92a5a10..a0e42ff 100644 --- a/variables.c +++ b/variables.c @@ -351,34 +351,13 @@ initialize_shell_variables (env, privmode) the environment in privileged mode. */ if (privmode == 0 && read_but_dont_execute == 0 && STREQN ("() {", string, 4)) { - string_length = strlen (string); - temp_string = (char *)xmalloc (3 + string_length + char_index); - strcpy (temp_string, name); - temp_string[char_index] = ' '; - strcpy (temp_string + char_index + 1, string); - - /* Don't import function names that are invalid identifiers from the - environment, though we still allow them to be defined as shell - variables. */ - if (legal_identifier (name)) - parse_and_execute (temp_string, name, SEVAL_NONINT|SEVAL_NOHIST|SEVAL_FUNCDEF|SEVAL_ONECMD); - - if (temp_var = find_function (name)) - { - VSETATTR (temp_var, (att_exported|att_imported)); - array_needs_making = 1; - } - else - { - if (temp_var = bind_variable (name, string, 0)) + if (temp_var = bind_function (name, 0)) { - VSETATTR (temp_var, (att_exported | att_imported | att_invisible)); + VSETATTR (temp_var, (att_exported | att_imported | att_unparsed)); + SET_EXPORTSTR(temp_var, savestring (string)); array_needs_making = 1; } - last_command_exit_value = 1; - report_error (_("error importing function definition for `%s'"), name); - } } #if defined (ARRAY_VARS) # if ARRAY_EXPORT @@ -1049,8 +1028,11 @@ print_var_function (var) if (function_p (var) && var_isset (var)) { - x = named_function_string ((char *)NULL, function_cell(var), FUNC_MULTILINE|FUNC_EXTERNAL); - printf ("%s", x); + if (!maybe_parse_unparsed_func (var)) + { + x = named_function_string ((char *)NULL, function_cell(var), FUNC_MULTILINE|FUNC_EXTERNAL); + printf ("%s", x); + } } } @@ -3929,7 +3911,16 @@ make_env_array_from_var_list (vars) if (var->exportstr) value = var->exportstr; else if (function_p (var)) - value = named_function_string ((char *)NULL, function_cell (var), 0); + { + if (unparsed_p (var)) + { + value = var->exportstr; + } + else + { + value = named_function_string ((char *)NULL, function_cell (var), 0); + } + } #if defined (ARRAY_VARS) else if (array_p (var)) # if ARRAY_EXPORT diff --git a/execute_cmd.h b/execute_cmd.h index 67ae93a..4f4e3ee 100644 --- a/execute_cmd.h +++ b/execute_cmd.h @@ -27,6 +27,7 @@ extern struct fd_bitmap *new_fd_bitmap __P((int)); extern void dispose_fd_bitmap __P((struct fd_bitmap *)); extern void close_fd_bitmap __P((struct fd_bitmap *)); extern int executing_line_number __P((void)); +extern int maybe_parse_unparsed_func __P((SHELL_VAR*)); extern int execute_command __P((COMMAND *)); extern int execute_command_internal __P((COMMAND *, int, int, int, struct fd_bitmap *)); extern int shell_execve __P((char *, char **, char **)); diff --git a/execute_cmd.c b/execute_cmd.c index 9cebaef..df060d3 100644 --- a/execute_cmd.c +++ b/execute_cmd.c @@ -4368,6 +4368,46 @@ execute_builtin (builtin, words, flags, subshell) return (result); } +int +maybe_parse_unparsed_func (var) + SHELL_VAR *var; +{ + int result, string_length, char_index; + char *temp_string; + + if (!unparsed_p(var)) + return 0; + + string_length = strlen (var->exportstr); + char_index = strlen (var->name); + temp_string = (char *)xmalloc (3 + string_length + char_index); + + strcpy (temp_string, var->name); + temp_string[char_index] = ' '; + strcpy (temp_string + char_index + 1, var->exportstr); + + /* Don't import function names that are invalid identifiers from the + environment, though we still allow them to be defined as shell + variables. */ + result = 1; + if (legal_identifier (var->name) && + !(result = parse_and_execute (temp_string, var->name, SEVAL_NONINT|SEVAL_NOHIST|SEVAL_FUNCDEF|SEVAL_ONECMD))) + { + var = find_function (var->name); + VSETATTR (var, (att_exported|att_imported)); + VUNSETATTR(var, att_unparsed); + } + else + { + if (!legal_identifier (var->name)) + free (temp_string); + last_command_exit_value = 1; + report_error (_("error importing function definition for `%s'"), var->name); + return result; + } + return 0; +} + static int execute_function (var, words, flags, fds_to_close, async, subshell) SHELL_VAR *var; @@ -4403,6 +4443,10 @@ execute_function (var, words, flags, fds_to_close, async, subshell) GET_ARRAY_FROM_VAR ("BASH_LINENO", bash_lineno_v, bash_lineno_a); #endif + if (result = maybe_parse_unparsed_func (var)) + return result; + + tc = (COMMAND *)copy_command (function_cell (var)); if (tc && (flags & CMD_IGNORE_RETURN)) tc->flags |= CMD_IGNORE_RETURN; @@ -4502,7 +4546,7 @@ execute_function (var, words, flags, fds_to_close, async, subshell) push_args (words->next); /* Number of the line on which the function body starts. */ - line_number = function_line_number = tc->line; + line_number = function_line_number = tc ? tc->line : 0; #if defined (JOB_CONTROL) if (subshell) diff --git a/builtins/type.def b/builtins/type.def index bd9ecfc..25c1943 100644 --- a/builtins/type.def +++ b/builtins/type.def @@ -274,7 +274,8 @@ describe_command (command, dflags) printf (_("%s is a function\n"), command); /* We're blowing away THE_PRINTED_COMMAND here... */ - + if (result = maybe_parse_unparsed_func (func)) + return result; result = named_function_string (command, function_cell (func), FUNC_MULTILINE|FUNC_EXTERNAL); printf ("%s\n", result); } diff --git a/builtins/setattr.def b/builtins/setattr.def index 3be3189..f25558e 100644 --- a/builtins/setattr.def +++ b/builtins/setattr.def @@ -348,7 +348,7 @@ show_var_attributes (var, pattr, nodefs) int pattr, nodefs; { char flags[16], *x; - int i; + int i, result; i = 0; @@ -411,6 +411,8 @@ show_var_attributes (var, pattr, nodefs) reused as input to recreate the current state. */ if (function_p (var) && nodefs == 0 && (pattr == 0 || posixly_correct == 0)) { + if (result = maybe_parse_unparsed_func (var)) + return result; printf ("%s\n", named_function_string (var->name, function_cell (var), FUNC_MULTILINE|FUNC_EXTERNAL)); nodefs++; if (pattr == 0 && i == 1 && flags[0] == 'f')
Powered by blists - more mailing lists
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.