Is this a safe use of python eval()?

Question

If an attacker can control the value of attacker_controlled_nasty_variable, is this segment of code vulnerable?

dic={"one":1,
      "nasty":attacker_controlled_nasty_variable,
     }
store=str(dict)
...
dic=eval(store)

Since an attacker can edit your Python source, the question is rather silly, isn't it? — S.Lott, Mar 18 '11 at 11:10
Who said that the attacker is running the code on his local machine? — Tim Pietzcker, Mar 18 '11 at 11:11
@Tim Pietzcker: How did the attacker set the nasty variable if not running this on their local machine? — S.Lott, Mar 18 '11 at 11:15
@S.Lott SQL injection, config file, a form on a webpage - plenty of ways... — theheadofabroom, Mar 18 '11 at 11:40
@S.Lott its a web application and this is a method of serializing a dictionary. — rook, Mar 18 '11 at 12:13
@BiggAl: SQL injection is specious -- it doesn't happen unless you write really bad code. Config file is managed by web admin, who has access to source. Form on a web page must be validated and can't be used like this. I'm not seeing a way for this happen except when an admin **also** has access to the source. — S.Lott, Mar 18 '11 at 14:03
@Rook: If you're serializing a dictionary, why not use JSON instead of `repr` and `eval`? — S.Lott, Mar 18 '11 at 14:03
@S.Lott Yet many people do write really bad code. Depending on the application, you may wish to allow users to have their own config files which they upload and are run against their account, however if you do not sanitise your inputs you get vulnerabilities. Most importantly, not all projects as=re developed in whole by one man (few are) so make your code as good as it can be and it can cover for some of your colleges' inadequacies. — theheadofabroom, Mar 18 '11 at 14:08
@BiggAl: "you may wish to allow users to have their own config files which they upload and are run against their account" It's still trivial to avoid executing user-provided Python code. "cover for some of your colleges' inadequacies". Sorry. If you can't trust your colleagues and your web admin, you can't trust **anyone**. If you can't trust anyone, your only choice is to deliver software to them burned into ROMs plugged into Epoxy-sealed boards mounted in tamper-resistent racks that you physically watch 24x7. The alternative is to trust others. — S.Lott, Mar 18 '11 at 14:16
@S.Lott I din't necessarily mean you can't trust your colleagues, but when they come to use a piece of code which you wrote when you're not around, they're going to interpret it differently to how you meant to write it - most of the time it's close enough, other times it'll cause a bug. Your job is to stop these bugs from causing vulnerabilities. When everyone codes that way you end up with highly secure code. Your web admin is probably very clever but he didn't write the damn code, it's just his job to run it. — theheadofabroom, Mar 18 '11 at 14:21
@BiggAl: My point is this. The source is available. That's the #1 avenue of attack. Fooling around with trying to avoid `eval` is silly when anyone can make any mistake. "they're going to interpret it differently to how you meant to write it" means that ordinary QA and ordinary unit testing are far, far more important than worrying about `eval`. — S.Lott, Mar 18 '11 at 14:28
@S.Lott I still think it's better to play it safe. You're right that QA and unit test are important, but software quality comes from development, QA only measure it. — theheadofabroom, Mar 18 '11 at 14:39
@BiggAl: I would think QA would include all aspects of quality. Including testing and security. "Play it safe" goes without saying. Wasting time trying to finesse `eval` in Python is just playing. Not playing it safe. — S.Lott, Mar 18 '11 at 14:42
@S.Lott Yeah json is a great solution. bencode is a little smaller and asn.1 is by far the smallest. A `zlib.compress(str(dict),9)` works well, and i like the docs for `ast.literal_eval()`. — rook, Mar 18 '11 at 19:44
@S.Lott Also, `eval()` is different in python than any other language i have used. In this case even if i where to use `eval()` it would be a bad practice but i don't think its exploitable. The `__repr__` hack would fail because the class definition would decode as a string. — rook, Mar 18 '11 at 19:52

score 11 · Accepted Answer · answered Mar 18 '11 at 11:11

11

Use ast.literal_eval() instead of eval().

answered Mar 18 '11 at 11:11

payne

13,833
5
42
49

Ignacio Vazquez-Abrams · Answer 2 · 2011-03-18T11:15:06.123

6

Yes. It could be replaced with an object that has a __repr__() method that either has a payload itself, or returns a string that could be unsafe when passed to eval().

Proof of Concept:

class MyClass(object):
  def __repr__(self):
    return 'os.system("format c:")'

bad = [MyClass()]
print str(bad)

edited Mar 18 '11 at 11:15

answered Mar 18 '11 at 11:09

Ignacio Vazquez-Abrams

776,304
153
1,341
1,358

1

Can you give an example? I'm trying to visualize how the attack gets through the str() operation in his example. – payne Mar 18 '11 at 11:13
4

`class bad_repr(object): def __repr__(self): return 'evil'`(he cannot do that if his variable can only be a string though) With that class the stringified dict will look like `{'nasty': evil, 'one': 1}` - so `evil` is not quoted and could be pretty much any python code. – ThiefMaster Mar 18 '11 at 11:15
The pivotal point in this thread seems to be if the would-be attacker can set the object, or only a string value. – payne Mar 18 '11 at 11:54
@payne, that is an important point, because in the above code the nasty input would be a string and thus not-exploitable. – rook Mar 18 '11 at 19:52

score 3 · Answer 3 · answered Mar 18 '11 at 11:10

It is safe as long as you can be sure that attacker_controlled_nasty_variable is never an object where the attacker can control __repr__ (or __str__) as he could otherwise inject python code.

However, better use repr(dic) instead of str(dic) since only repr is expected to return valid python code.

Additionally - as mentioned by @payne - use the safer ast.literal_eval() instead of eval().

score 1 · Answer 4 · answered Mar 18 '11 at 11:17

Let's try it:

>>> attacker_controlled_nasty_variable="`cat /etc/passwd`"
>>> dic={"one":1,
...     "nasty":attacker_controlled_nasty_variable,
... }
>>> store = repr(dic)
>>> store
"{'nasty': '`cat /etc/passwd`', 'one': 1}"
>>> dic=eval(store)
>>> dic
{'nasty': '`cat /etc/passwd`', 'one': 1}

>>> attacker_controlled_nasty_variable="'hello',}"
>>> dic={"one":1,
...     "nasty":attacker_controlled_nasty_variable,
... }
>>> repr(dic)
'{\'nasty\': "\'hello\',}", \'one\': 1}'
>>> eval(repr(dic))
{'nasty': "'hello',}", 'one': 1}

You may want to try more cases, but empirically it looks like __repr__ is quoting the contents correctly.

What about: class Bad: def __repr__(self): os.system("do bad stuff) \ attacker_controlled_nasty_variable = Bad()? — James, Mar 18 '11 at 11:20

Is this a safe use of python eval()?

4 Answers4

Linked