Security: remote code execution

[michael-lazar]

Hi,

I am currently auditing many open-source reddit clients and have
found an important vulnerability in rtv involving massive remote code
execution.

The problem

In rtv/terminal.py:471 (rev. 20b59d5), in open_browser(...) the
command variable is built unsafely. An url such as:

http://test.com/?q=');print('hacked');print('

would allow python code injection which would lately be happily
executed by python through the Popen call.

Attack scenario

An attacker could craft a malicious url executing code (most likely
downloading a trojan) and post it under an attractive reddit title.
Anyone trying to open it in the webbrowser through rtv would get
hacked.

Also note that in my example a tab is still opened to the url

http://test.com/?q=

so one could very well provide a real url in order to mask his
attack for those that do not use rtv.

The impact

The impact is critical: hundreds of computers can be impacted at
once. While it needs user interaction it is very easy to hide the
attack from one.

Recommended fix

The best would be not to evaluate python code at all. Every code
injection problem comes from bugs at the interface between two
languages or layers.

The quickest solution though would be to escape quotes and
backslashes within the url. Escaping each and every character would
break lots of urls but at the very least quotes and backslashes
shouldn't be part of it.

I don't recommend the quick solution though: executing external code
through proper means is better.

Course of action

In my humble opinion your best course of action is to patch it as
soon as possible then issue a clear warning to already existing
users, maybe by getting a CVE number (Common Vulnerability and
Exposures, a global reference for all security vulnerabilities in
software of some importance).

I hope this finds you well,

Sincerely,

Cédric Picard

[michael-lazar]

Cedric first sent this via email but have given me permission to post it publicly.

This is the offending line. If the url contains a quote, it can break out of the string and execute arbitrary python commands.

command = "import webbrowser; webbrowser.open_new_tab('%s')" % url

The quickest solution as stated above is to sanitize all urls from reddit before opening them. I wonder if urllib.parse.quote would be sufficient. I'm honestly surprised that the reddit api doesn't already do this for us.

[michael-lazar]

Possible related security hole with mailcap https://bugs.python.org/issue24778 that I noticed a while ago but never looked into.

[cym13]

A quick and dirty solution (albeit effective) would be something along the lines of:

    if self.display:
        command = """
                  import webbrowser, urllib.parse as p
                  webbrowser.open_new_tab(p.unquote('%s'))" % p.quote(url)
                  """
        args = [sys.executable, '-c', command]

It avoids any misquote and injection

[michael-lazar]

Is there any valid reason for the url to have quotes? i.e. why not just

    if self.display:
        command = """
                  import webbrowser
                  webbrowser.open_new_tab('%s')
                  """ % p.quote(url)
        args = [sys.executable, '-c', command]

[cym13]

Not unquoting it would break many things, take a youtube url for example:

https://www.youtube.com/watch?v=dQw4w9WgXcQ

Quoted it becomes https%3A//www.youtube.com/watch%3Fv%3DdQw4w9WgXcQ which isn't valid for youtube. So it needs to be unquoted.

[michael-lazar]

I just published a new release and put a warning in the README letting people know to upgrade.