Re: CVE request: python-gnupg before 0.3.5 shell injection

[Florian Weimer <fweimer () redhat com>]

On 02/04/2014 11:04 AM, Henri Salo wrote:
> On Tue, Feb 04, 2014 at 10:35:46AM +0100, Hanno Böck wrote:
> > python-gnupg 0.3.5 lists in the changelog:
> > "Added improved shell quoting to guard against shell injection."
> >
> > Sounds like a severe security issue, but further info is lacking.
>
> Diff attached. New function shell_quote() seems to represent major changes to
> shell input quoting against unsafe input.
>
> +# We use the test below because it works for Jython as well as CPython
> +if os.path.__name__ == 'ntpath':
> +    # On Windows, we don't need shell quoting, other than worrying about
> +    # paths with spaces in them.
> +    def shell_quote(s):
> +        return '"%s"' % s
> +else:
> +    # Section copied from sarge
> +
> +    # This regex determines which shell input needs quoting
> +    # because it may be unsafe
> +    UNSAFE = re.compile(r'[^\w%+,./:=@-]')
> +
> +    def shell_quote(s):
> +        """
> +        Quote text so that it is safe for Posix command shells.
> +
> +        For example, "*.py" would be converted to "'*.py'". If the text is
> +        considered safe it is returned unquoted.
> +
> +        :param s: The value to quote
> +        :type s: str (or unicode on 2.x)
> +        :return: A safe version of the input, from the point of view of Posix
> +                 command shells
> +        :rtype: The passed-in type
> +        """
> +        if not isinstance(s, string_types):
> +            raise TypeError('Expected string type, got %s' % type(s))
> +        if not s:
> +            result = "''"
> +        elif len(s) >= 2 and (s[0], s[-1]) == ("'", "'"):
> +            result = '"%s"' % s.replace('"', r'\"')
> +        elif not UNSAFE.search(s):
> +            result = s
> +        else:
> +            result = "'%s'" % s.replace("'", "'\"'\"'")
> +        return result
> +
> +    # end of sarge code

This fix appears to be incomplete:

>>> print shell_quote("'$(touch /tmp/I_was_here'")
"'$(touch /tmp/I_was_here'"


[fweimer@oldenburg ~]$ echo "'$(touch /tmp/I_was_here)'"
''
[fweimer@oldenburg ~]$ ls -l /tmp/I_was_here
-rw-rw-r--. 1 fweimer fweimer 0 Feb  4 11:12 /tmp/I_was_here

The proper way (at least if your shell runs in a UTF-8 or ISO-8859 
locale) to escape shell arguments is to wrap them in '', after replacing 
embedded ' characters with the four character sequence '\''.  However, 
using the subprocess module with shell=False (the default) is strongly 
preferred.

In both cases, you need to make sure that you prevent option injection 
through positional arguments.  With a GNU getopt-derived command line 
parser, option processing can be terminated with a -- argument. 
(Warning: GnuPG does not strictly follow GNU command line processing 
conventions.)

Is anyone in touch with the python-gpg folks and can rely this 
information?  Thanks.

--
Florian Weimer / Red Hat Product Security Team