Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.