|
Message-ID: <20140929013322.GA27172@openwall.com> Date: Mon, 29 Sep 2014 05:33:22 +0400 From: Solar Designer <solar@...nwall.com> To: oss-security@...ts.openwall.com Cc: Chester Ramey <chet.ramey@...e.edu> Subject: Re: binary-patching bash On Mon, Sep 29, 2014 at 04:44:05AM +0400, Solar Designer wrote: > The primary risk I see here is that some build of bash might include > custom patches where this check had been changed to use something other > than (an equivalent of) strncmp(). I am not aware of any such cases. Another concern is that (previously custom-patched) code following the check might jump over the constant string (or a portion of it) that it thinks has just been matched. I am not aware of any such cases, and this is not true in at least bash 1.14.7 and bash 4.3. In 1.14.7, we have: if (!privileged_mode && STREQN ("() {", string, 4)) { SHELL_VAR *f; char *eval_string; eval_string = xmalloc (3 + string_length + strlen (name)); sprintf (eval_string, "%s %s", name, string); parse_and_execute (eval_string, name, 0); In 4.3, we have: 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); if (posixly_correct == 0 || legal_identifier (name)) parse_and_execute (temp_string, name, SEVAL_NONINT|SEVAL_NOHIST); As we can see, "string" is used with no offset, despite of its 4 chars having already been checked. That's good. I hope it's the same for all other revisions of the code, which is natural because "() {" is part of the needed input to the parser anyway, but some risk is there. Alexander
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.