On 2013-01-04, Steven D'Aprano <steve+comp.lang.pyt...@pearwood.info> wrote:
> 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.
I suspected that was the case.
> 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.
I can disallow underscores in names.
> [...]
> * 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.
OK, quotes and dots are out as well.
> * 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.
That's obviously the "right" thing to do. I suppose I should figure
out how to use the ast module.
> 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.
Thanks! It's definitely an improvement.
--
Grant Edwards grant.b.edwards Yow! -- I have seen the
at FUN --
gmail.com
--
http://mail.python.org/mailman/listinfo/python-list
