zopefoundation / RestrictedPython

A restricted execution environment for Python to run untrusted code.
http://restrictedpython.readthedocs.io/
Other
466 stars 38 forks source link

Allow augmented assignments on __setitem__ ? #128

Open BluBb-mADe opened 6 years ago

BluBb-mADe commented 6 years ago
d = {"test": 0}
d["test"]+=1
>>> Augmented assignment of object items and slices is not allowed.

Why is this a security risk and is it possible to safely allow augmented assignments on mapped c++ objects which implement __getitem__ and __setitem__? Furthermore is it even possible to allow this without writing a custom transformer?

icemac commented 6 years ago

Currently augmented assignment of object items is not allowed because it is currently not checked whether the user is has read and write access to the item.

@stephan-hof You implemented these checks in 1f26049af765c3e2e53b833f010bb755b34db3b8. What was the rationale behind disallowing certain types of augmented assignment?

stephan-hof commented 6 years ago

This restriction is there for a long time. It has been introduced here: https://github.com/zopefoundation/RestrictedPython/commit/db27fa738962de54747ef56634a1eeb1065b2bce

I guess the reason is that restriction python has currently not the possibility to check if the 'write back' into object is allowed. For normal assignments restricted python does

foo[a] = c    
becomes
_write_(foo)[a] = c

However this 'write' check cannot be done with augmented assignment, because the write back happens inside the __iadd__ code of the object.

One possibility to still support augmented assignments could be to transform it into:

foo[a] += 1
becomes
_write_(foo)[a] = _getitem_(foo, a) + 1

Which means the __iadd__ of foo is not called.

Apart from this I have currently no idea how a proper secured __iadd__ could look like. Probably a change in AccessControl is required as well.