Xavier (Open ERP) has proposed merging
lp:~openerp-dev/openobject-server/6.1-harden-safe-eval-no-dunder-xmo into
lp:openobject-server/6.1.
Requested reviews:
OpenERP Core Team (openerp)
For more details, see:
https://code.launchpad.net/~openerp-dev/openobject-server/6.1-harden-safe-eval-no-dunder-xmo/+merge/109083
The current safe_eval leaves the door open to some attack patterns such as
http://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html[0] which
segfault the interpreter (and may be able to do more).
This patch fixes the hole by blanket-forbidding all access to "dunder"
attributes (attributes both pre- and post-fixed by __). This prevents direct
access to such attributes as __class__, __bases__, __subclasses__,
__getattribute__, __name__ and many others.
It also makes bytecode validation recursive, since openerp allows creating
lambdas and lambda bytecode instructions (and names) are stored in separate
code objects not directly in the global "root" one.
[0] that code does not work as-is due to a hardcoded string check on
"__subclasses__", but that reference can be replaced with
"__getattribute__('__sub' + 'classes__')" and it will work
--
https://code.launchpad.net/~openerp-dev/openobject-server/6.1-harden-safe-eval-no-dunder-xmo/+merge/109083
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openobject-server/6.1-harden-safe-eval-no-dunder-xmo.
=== modified file 'openerp/tools/safe_eval.py'
--- openerp/tools/safe_eval.py 2012-02-08 17:28:25 +0000
+++ openerp/tools/safe_eval.py 2012-06-07 08:28:18 +0000
@@ -82,16 +82,65 @@
[100, 100, 23, 100, 100, 102, 103, 83]
"""
i = 0
- opcodes = []
byte_codes = codeobj.co_code
while i < len(byte_codes):
code = ord(byte_codes[i])
- opcodes.append(code)
+ yield code
+
if code >= HAVE_ARGUMENT:
i += 3
else:
i += 1
- return opcodes
+
+def assert_no_dunder_name(code_obj, expr):
+ """ assert_no_dunder_name(code_obj, expr) -> None
+
+ Asserts that the code object does not refer to any "dunder name"
+ (__$name__), so that safe_eval prevents access to any internal-ish Python
+ attribute or method (both are loaded via LOAD_ATTR which uses a name, not a
+ const or a var).
+
+ Checks that no such name exists in the provided code object (co_names).
+
+ :param code_obj: code object to name-validate
+ :type code_obj: CodeType
+ :param str expr: expression corresponding to the code object, for debugging
+ purposes
+ :raises NameError: in case a forbidden name (prefixed with two underscores)
+ is found in ``code_obj``
+
+ .. note:: actually forbids every name prefixed with 2 underscores
+ """
+ for name in code_obj.co_names:
+ if name.startswith('__'):
+ raise NameError('Access to forbidden name %r (%r)' % (name, expr))
+
+def assert_valid_codeobj(allowed_codes, code_obj, expr):
+ """ Asserts that the provided code object validates against the bytecode
+ and name constraints.
+
+ Recursively validates the code objects stored in its co_consts in case
+ lambdas are being created/used (lambdas generate their own separated code
+ objects and don't live in the root one)
+
+ :param allowed_codes: list of permissible bytecode instructions
+ :type allowed_codes: set(int)
+ :param code_obj: code object to name-validate
+ :type code_obj: CodeType
+ :param str expr: expression corresponding to the code object, for debugging
+ purposes
+ :raises ValueError: in case of forbidden bytecode in ``code_obj``
+ :raises NameError: in case a forbidden name (prefixed with two underscores)
+ is found in ``code_obj``
+ """
+ assert_no_dunder_name(code_obj, expr)
+ for opcode in _get_opcodes(code_obj):
+ if opcode not in allowed_codes:
+ raise ValueError(
+ "opcode %s not allowed (%r)" % (opname[opcode], expr))
+ for const in code_obj.co_consts:
+ if isinstance(const, CodeType):
+ assert_valid_codeobj(allowed_codes, const, 'lambda')
def test_expr(expr, allowed_codes, mode="eval"):
"""test_expr(expression, allowed_codes[, mode]) -> code_object
@@ -106,15 +155,14 @@
# eval() does not like leading/trailing whitespace
expr = expr.strip()
code_obj = compile(expr, "", mode)
- except (SyntaxError, TypeError):
+ except (SyntaxError, TypeError, ValueError):
_logger.debug('Invalid eval expression', exc_info=True)
raise
except Exception:
_logger.debug('Disallowed or invalid eval expression', exc_info=True)
raise ValueError("%s is not a valid expression" % expr)
- for code in _get_opcodes(code_obj):
- if code not in allowed_codes:
- raise ValueError("opcode %s not allowed (%r)" % (opname[code], expr))
+
+ assert_valid_codeobj(allowed_codes, code_obj, expr)
return code_obj
@@ -183,19 +231,13 @@
This can be used to e.g. evaluate
an OpenERP domain expression from an untrusted source.
- Throws TypeError, SyntaxError or ValueError (not allowed) accordingly.
-
- >>> safe_eval("__import__('sys').modules")
- Traceback (most recent call last):
- ...
- ValueError: opcode LOAD_NAME not allowed
-
+ :throws TypeError: If the expression provided is a code object
+ :throws SyntaxError: If the expression provided is not valid Python
+ :throws NameError: If the expression provided accesses forbidden names
+ :throws ValueError: If the expression provided uses forbidden bytecode
"""
if isinstance(expr, CodeType):
- raise ValueError("safe_eval does not allow direct evaluation of code objects.")
-
- if '__subclasses__' in expr:
- raise ValueError('expression not allowed (__subclasses__)')
+ raise TypeError("safe_eval does not allow direct evaluation of code objects.")
if globals_dict is None:
globals_dict = {}
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
