CODEWITHSUNDEEP
pythonsecurity
d33tah
d33tah

Reputation: 11561

Is there a string `s` such that eval(repr(s)) leads to arbitrary code execution?

I found similar code somewhere:

USER_CONTROLLED = 'a'
open("settings.py", "w").write("USER_CONTROLLED = %s" % eval(repr(a)))

And in another file:

import settings
x = settings.USER_CONTROLLED * [0]

Is this a security vulnerability?

Upvotes: 2

Views: 428

Answers (1)

poke
poke

Reputation: 387557

In contrast to what you were told on IRC, there definitely is an x that makes eval(repr(x)) dangerous, so saying it just like that without any restrictions is wrong too.

Imagine a custom object that implements __repr__ differently. The documentation says on __repr__ that it “should look like a valid Python expression that could be used to recreate an object with the same value”. But there is simply nothing that can possibly enforce this guideline.

So instead, we could create a class that has a custom __repr__ that returns a string which when evaluated runs arbitrary code. For example:

class MyObj:
    def __repr__ (self):
        return "__import__('urllib.request').request.urlopen('http://example.com').read()"

Calling repr() on an object of that type shows that it returns a string that can surely be evaluated:

>>> repr(MyObj())
"__import__('urllib.request').request.urlopen('http://example.com').read()"

Here, that would just involve making a request to example.com. But as you can see, we can import arbitrary modules here and run code with them. And that code can have any kind of side effects. So it’s definitely dangerous.

If we however limit that x to known types of which we know what calling repr() on them will do, then we can indeed say when it’s impossible to run arbitrary code with it. For example, if that x is a string, then the implementation of unicode_repr makes sure that everything is properly escaped and that evaluating the repr() of that object will always return a proper string (which even equals x) without any side effects.

So we should check for the type before evaluating it:

if type(a) is not str:
    raise Exception('Only strings are allowed!')

something = eval(repr(a))

Note that we do not use isinstance here to do an inheritance-aware type check. Because I could absolutely make MyObj above inherit from str:

>>> x = MyObj()
>>> isinstance(x, str)
True
>>> type(x)
<class '__main__.MyObj'>

So you should really test against concrete types here.

Note that for strings, there is actually no reason to call eval(repr(x)) because as mentioned above, this will result in x itself. So you could just assign x directly.

Coming to your actual use case however, you do have a very big security problem here. You want to create a variable assignment and store that code in a Python file to be later run by an actual Python interpreter. So you should absolutely make sure that the right side of the assignment is not arbitrary code but actually the repr of a string:

>>> a = 'runMaliciousCode()'
>>> "USER_CONTROLLED = %s" % eval(repr(a))
'USER_CONTROLLED = runMaliciousCode()'
>>> "USER_CONTROLLED = %s" % repr(a)
"USER_CONTROLLED = 'runMaliciousCode()'"

As you can see, evaluating the repr() will put the actual content on the right side of the assignment (since it’s equivalent to "…" % a). But that can then lead to malicious code running when you import that file. So you should really just insert the repr of the string there, and completely forget about using eval altogether.

Upvotes: 2

Related Questions