diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst index c659071..ee49c2a 100644 --- a/RELEASE_NOTES.rst +++ b/RELEASE_NOTES.rst @@ -6,6 +6,14 @@ Changes from 2.14.2 to 2.14.3 ----------------------------- * **Under development.** +* Replaced expression-string blacklist filtering with AST validation before + evaluation, and disabled Python builtins while sanitization is enabled. + This closes sanitizer bypasses in both cached and ``disable_cache=True`` + evaluation. Expressions using unsupported Python syntax are now rejected + before evaluation. Parser errors raise ``SyntaxError``; expressions rejected + by the sanitizer raise ``ValueError``; and unknown functions raise + ``TypeError``. Sanitization can still be explicitly disabled with + ``sanitize=False`` or ``NUMEXPR_SANITIZE=0``. Changes from 2.14.1 to 2.14.2 ----------------------------- diff --git a/numexpr/necompiler.py b/numexpr/necompiler.py index 98d11e4..797e647 100644 --- a/numexpr/necompiler.py +++ b/numexpr/necompiler.py @@ -10,8 +10,8 @@ import __future__ +import ast as pyast import os -import re import sys import threading import weakref @@ -278,37 +278,117 @@ def __str__(self): return 'Immediate(%d)' % (self.node.value,) -_flow_pat = r'[\;\[\:]' -_dunder_pat = r'(^|[^\w])__[\w]+__($|[^\w])' -_attr_pat = r'\.\b(?!(real|imag|(\d*[eE]?[+-]?\d+)|(\d*[eE]?[+-]?\d+j)|(\d*j))\b)' -_blacklist_re = re.compile(f'{_flow_pat}|{_dunder_pat}|{_attr_pat}') +_allowed_ast_node_types = frozenset(( + pyast.Expression, + pyast.Constant, + pyast.Name, + pyast.Load, + pyast.BinOp, + pyast.BoolOp, + pyast.UnaryOp, + pyast.Compare, + pyast.IfExp, + pyast.Call, + pyast.keyword, + pyast.Attribute, + pyast.Add, + pyast.Sub, + pyast.Mult, + pyast.Div, + pyast.FloorDiv, + pyast.Pow, + pyast.Mod, + pyast.LShift, + pyast.RShift, + pyast.BitAnd, + pyast.BitOr, + pyast.BitXor, + pyast.And, + pyast.Or, + pyast.UAdd, + pyast.USub, + pyast.Invert, + pyast.Not, + pyast.Eq, + pyast.NotEq, + pyast.Lt, + pyast.LtE, + pyast.Gt, + pyast.GtE, + pyast.In, + pyast.NotIn, + pyast.Is, + pyast.IsNot, +)) + +def _forbidden_expression(expression): + raise ValueError(f'Expression {expression} has forbidden syntax.') + + +def _is_dunder_name(name): + # Not a security boundary: a bare Name can only ever become a VariableNode, + # since every name in `co_names` is bound before `eval` and builtins are + # empty. Escapes need attribute access or a call, which are gated below. + # This check exists to preserve the pre-2.14.3 `_dunder_pat` regex + # `__[\w]+__`, which required at least one character between the underscore + # pairs -- hence the length bound, which admits '____' but not '_____'. + return ( + len(name) > 4 + and name.startswith('__') + and name.endswith('__') + ) + + +def _sanitize_expression(expression): + parsed = pyast.parse(expression, filename='', mode='eval') + + for node in pyast.walk(parsed): + node_type = type(node) + if node_type not in _allowed_ast_node_types: + _forbidden_expression(expression) + if node_type is pyast.Name and _is_dunder_name(node.id): + _forbidden_expression(expression) + if node_type is pyast.Attribute and node.attr not in ('real', 'imag'): + _forbidden_expression(expression) + if node_type is pyast.Call: + if not isinstance(node.func, pyast.Name): + _forbidden_expression(expression) + # Redundant with the function-table check below -- no dunder is in + # `expressions.functions` -- but it reports `__import__(...)` as a + # sanitizer rejection rather than as an unknown function. + if _is_dunder_name(node.func.id): + _forbidden_expression(expression) + if node.func.id not in expressions.functions: + raise TypeError(f'unknown function: {node.func.id}') + return parsed + + +def _resolve_sanitize(sanitize): + if sanitize is not None: + return sanitize + if 'NUMEXPR_SANITIZE' in os.environ: + return bool(int(os.environ['NUMEXPR_SANITIZE'])) + return True + + +def _compile_expression(expression, context, sanitize): + sanitize = _resolve_sanitize(sanitize) + source = _sanitize_expression(expression) if sanitize else expression + if context.get('truediv', False): + flags = __future__.division.compiler_flag + else: + flags = 0 + return compile(source, '', 'eval', flags) -def stringToExpression(s, types, context, sanitize: bool=True): - """Given a string, convert it to a tree of ExpressionNode's. - """ - # sanitize the string for obvious attack vectors that NumExpr cannot - # parse into its homebrew AST. This is to protect the call to `eval` below. - # We forbid `;`, `:`. `[` and `__`, and attribute access via '.'. - # We cannot ban `.real` or `.imag` however... - # We also cannot ban `.\d*j`, where `\d*` is some digits (or none), e.g. 1.5j, 1.j - if sanitize: - no_whitespace = re.sub(r'\s+', '', s) - skip_quotes = re.sub(r'(\'[^\']*\')', '', no_whitespace) - if _blacklist_re.search(skip_quotes) is not None: - raise ValueError(f'Expression {s} has forbidden control characters.') +def _expression_from_compiled(compiled, types, context, sanitize): + sanitize = _resolve_sanitize(sanitize) old_ctx = expressions._context.get_current_context() try: expressions._context.set_new_context(context) - # first compile to a code object to determine the names - if context.get('truediv', False): - flags = __future__.division.compiler_flag - else: - flags = 0 - c = compile(s, '', 'eval', flags) # make VariableNode's for the names names = {} - for name in c.co_names: + for name in compiled.co_names: if name == "None": names[name] = None elif name == "True": @@ -321,7 +401,10 @@ def stringToExpression(s, types, context, sanitize: bool=True): names.update(expressions.functions) # now build the expression - ex = eval(c, names) + if sanitize: + ex = eval(compiled, {'__builtins__': {}}, names) + else: + ex = eval(compiled, names) if expressions.isConstant(ex): ex = expressions.ConstantNode(ex, expressions.getKind(ex)) @@ -332,6 +415,14 @@ def stringToExpression(s, types, context, sanitize: bool=True): return ex +def stringToExpression(s, types, context, sanitize: bool=True): + """Given a string, convert it to a tree of ExpressionNode's. + """ + sanitize = _resolve_sanitize(sanitize) + compiled = _compile_expression(s, context, sanitize) + return _expression_from_compiled(compiled, types, context, sanitize) + + def isReduction(ast): prefixes = (b'sum_', b'prod_', b'min_', b'max_') return any(ast.value.startswith(p) for p in prefixes) @@ -582,15 +673,18 @@ def getContext(kwargs, _frame_depth=1): return context -def precompile(ex, signature=(), context={}, sanitize: bool=True): +def _precompile(ex, signature, context, sanitize, compiled=None): """ Compile the expression to an intermediate form. """ + sanitize = _resolve_sanitize(sanitize) types = dict(signature) input_order = [name for (name, type_) in signature] if isinstance(ex, str): - ex = stringToExpression(ex, types, context, sanitize) + if compiled is None: + compiled = _compile_expression(ex, context, sanitize) + ex = _expression_from_compiled(compiled, types, context, sanitize) # the AST is like the expression, but the node objects don't have # any odd interpretations @@ -636,7 +730,21 @@ def precompile(ex, signature=(), context={}, sanitize: bool=True): return threeAddrProgram, signature, tempsig, constants, input_names -def NumExpr(ex, signature=(), sanitize: bool=True, **kwargs): +def precompile(ex, signature=(), context={}, sanitize: bool=True): + return _precompile(ex, signature, context, sanitize) + + +def _numexpr(ex, signature, context, sanitize, compiled=None): + threeAddrProgram, inputsig, tempsig, constants, input_names = _precompile( + ex, signature, context, sanitize=sanitize, compiled=compiled + ) + program = compileThreeAddrForm(threeAddrProgram) + return interpreter.NumExpr(inputsig.encode('ascii'), + tempsig.encode('ascii'), + program, constants, input_names) + + +def NumExpr(ex, signature=(), sanitize: bool=True, **kwargs): """ Compile an expression built using E. variables to a function. @@ -653,11 +761,8 @@ def NumExpr(ex, signature=(), sanitize: bool=True, **kwargs): # translated to either True or False). _frame_depth = 1 context = getContext(kwargs, _frame_depth=_frame_depth) - threeAddrProgram, inputsig, tempsig, constants, input_names = precompile(ex, signature, context, sanitize=sanitize) - program = compileThreeAddrForm(threeAddrProgram) - return interpreter.NumExpr(inputsig.encode('ascii'), - tempsig.encode('ascii'), - program, constants, input_names) + sanitize = _resolve_sanitize(sanitize) + return _numexpr(ex, signature, context, sanitize) def disassemble(nex): @@ -734,8 +839,11 @@ def getType(a): raise ValueError("unknown type %s" % a.dtype.name) -def getExprNames(text, context, sanitize: bool=True): - ex = stringToExpression(text, {}, context, sanitize) +def _getExprNames(text, context, sanitize, compiled=None): + sanitize = _resolve_sanitize(sanitize) + if compiled is None: + compiled = _compile_expression(text, context, sanitize) + ex = _expression_from_compiled(compiled, {}, context, sanitize) ast = expressionToAST(ex) input_order = getInputOrder(ast, None) #try to figure out if vml operations are used by expression @@ -752,6 +860,10 @@ def getExprNames(text, context, sanitize: bool=True): return [a.value for a in input_order], ex_uses_vml +def getExprNames(text, context, sanitize: bool=True): + return _getExprNames(text, context, sanitize) + + def getArguments(names, local_dict=None, global_dict=None, _frame_depth: int=2): """ Get the arguments based on the names. @@ -873,8 +985,8 @@ def validate(ex: str, sanitize: Optional[bool] Both `validate` and by extension `evaluate` call `eval(ex)`, which is potentially dangerous on unsanitized inputs. As such, NumExpr by default - performs simple sanitization, banning the character ':;[', the - dunder '__[\w+]__', and attribute access to all but '.real' and '.imag'. + permits only AST nodes used by its expression language, functions from + its function table, and the attributes '.real' and '.imag'. Using `None` defaults to `True` unless the environment variable `NUMEXPR_SANITIZE=0` is set, in which case the default is `False`. @@ -902,17 +1014,21 @@ def validate(ex: str, if not isinstance(ex, str): raise ValueError("must specify expression as a string") - if sanitize is None: - if 'NUMEXPR_SANITIZE' in os.environ: - sanitize = bool(int(os.environ['NUMEXPR_SANITIZE'])) - else: - sanitize = True + sanitize = _resolve_sanitize(sanitize) # Get the names for this expression context = getContext(kwargs) expr_key = (ex, tuple(sorted(context.items()))) + # Keep unsanitized results separate without changing the default + # cache-key shape used by the hot path. + if not sanitize: + expr_key += (sanitize,) + compiled = None if expr_key not in _names_cache.c: - _names_cache.c[expr_key] = getExprNames(ex, context, sanitize=sanitize) + compiled = _compile_expression(ex, context, sanitize) + _names_cache.c[expr_key] = _getExprNames( + ex, context, sanitize=sanitize, compiled=compiled + ) names, ex_uses_vml = _names_cache.c[expr_key] arguments = getArguments(names, local_dict, global_dict, _frame_depth=_frame_depth) @@ -925,7 +1041,11 @@ def validate(ex: str, try: compiled_ex = _numexpr_cache.c[numexpr_key] except KeyError: - compiled_ex = _numexpr_cache.c[numexpr_key] = NumExpr(ex, signature, sanitize=sanitize, **context) + if compiled is None: + compiled = _compile_expression(ex, context, sanitize) + compiled_ex = _numexpr_cache.c[numexpr_key] = _numexpr( + ex, signature, context, sanitize, compiled=compiled + ) kwargs = _cache_last_kwargs(out, order, casting, ex_uses_vml) _numexpr_last.l.set(ex=compiled_ex, argnames=names, kwargs=kwargs) except Exception as e: @@ -985,11 +1105,11 @@ def evaluate(ex: str, like float64 to float32, are allowed. * 'unsafe' means any data conversions may be done. - sanitize: bool + sanitize: Optional[bool] `validate` (and by extension `evaluate`) call `eval(ex)`, which is potentially dangerous on non-sanitized inputs. As such, NumExpr by default - performs simple sanitization, banning the characters ':;[', the - dunder '__[\w+]__', and attribute access to all but '.real' and '.imag'. + permits only AST nodes used by its expression language, functions from + its function table, and the attributes '.real' and '.imag'. Using `None` defaults to `True` unless the environment variable `NUMEXPR_SANITIZE=0` is set, in which case the default is `False`. @@ -1021,15 +1141,21 @@ def evaluate(ex: str, # here, but we have difficulties with the `sys.getframe(2)` call in # `getArguments` - # If dissable_cache set to be True, we evaluate the expression here + # If disable_cache is True, we evaluate the expression here. # Otherwise we validate and then re_evaluate if disable_cache: + sanitize = _resolve_sanitize(sanitize) context = getContext(kwargs) - names, ex_uses_vml = getExprNames(ex, context, sanitize=sanitize) + compiled = _compile_expression(ex, context, sanitize) + names, ex_uses_vml = _getExprNames( + ex, context, sanitize=sanitize, compiled=compiled + ) arguments = getArguments(names, local_dict, global_dict, _frame_depth=_frame_depth - 1) signature = [(name, getType(arg)) for (name, arg) in zip(names, arguments)] - compiled_ex = NumExpr(ex, signature, sanitize=sanitize, **context) + compiled_ex = _numexpr( + ex, signature, context, sanitize, compiled=compiled + ) kwargs = {'out': out, 'order': order, 'casting': casting, 'ex_uses_vml': ex_uses_vml} return compiled_ex(*arguments, **kwargs) diff --git a/numexpr/tests/test_numexpr.py b/numexpr/tests/test_numexpr.py index 74ed65e..5fdeebb 100644 --- a/numexpr/tests/test_numexpr.py +++ b/numexpr/tests/test_numexpr.py @@ -455,6 +455,8 @@ def test_validate_missing_var(self): def test_validate_syntax(self): retval = validate("2+") assert(isinstance(retval, SyntaxError)) + retval = validate("a = 1") + assert(isinstance(retval, SyntaxError)) def test_validate_dict(self): a1 = array([1., 2., 3.]) @@ -673,10 +675,10 @@ def test_sanitize(self): else: self.fail() - # Forbid semicolon + # Statements are invalid in an expression. try: evaluate('import os;') - except ValueError: + except SyntaxError: pass else: self.fail() @@ -725,6 +727,69 @@ def test_sanitize(self): x = np.array(['a', 'b'], dtype=bytes) evaluate("x == 'b:'") + # Reject syntax hidden between single quotes that are themselves + # contained in double-quoted literals. + with self.assertRaises(ValueError): + evaluate('"\'" + ().__class__ + "\'"') + + # The no-cache path must resolve sanitize=None to the safe default. + with self.assertRaises(ValueError): + evaluate('().__class__', disable_cache=True) + + # An unsanitized cache entry must not bypass later sanitization. + a = arange(3) + evaluate('(a,)[0]', sanitize=False) + with self.assertRaises(ValueError): + evaluate('(a,)[0]', sanitize=True) + + # Preserve names containing non-dunder double underscores. + for name in ('feature__scaled', 'col__value', '__x', 'x__'): + result = evaluate( + f'{name} + 1', local_dict={name: a} + ) + assert_array_equal(result, a + 1) + + # Preserve expression forms accepted by the previous sanitizer. + b = a + 1 + assert_array_equal( + evaluate('a if True else b', local_dict={'a': a, 'b': b}), + a, + ) + assert_equal( + evaluate('a is b', local_dict={'a': a, 'b': b}), + False, + ) + assert_equal( + evaluate('a is not b', local_dict={'a': a, 'b': b}), + True, + ) + with self.assertRaises(TypeError): + evaluate('a in b', local_dict={'a': a, 'b': b}) + with self.assertRaises(TypeError): + evaluate('a not in b', local_dict={'a': a, 'b': b}) + + with self.assertRaisesRegex(TypeError, 'unknown function: foo'): + evaluate('foo(a)', local_dict={'a': a}) + + # sanitize=None must resolve to the secure default for NumExpr. + with self.assertRaises(ValueError): + NumExpr('(a,)[0]', [('a', double)], sanitize=None) + + context = {'optimization': 'aggressive', 'truediv': False} + for call in ( + lambda: numexpr.necompiler.stringToExpression( + '(a,)[0]', {'a': double}, context, sanitize=None + ), + lambda: numexpr.necompiler.precompile( + '(a,)[0]', [('a', double)], context, sanitize=None + ), + lambda: numexpr.necompiler.getExprNames( + '(a,)[0]', context, sanitize=None + ), + ): + with self.assertRaises(ValueError): + call() + @pytest.mark.thread_unsafe def test_no_sanitize(self): try: # Errors on compile() after eval() @@ -742,6 +807,10 @@ def test_no_sanitize(self): else: self.fail() + a = arange(3, dtype=double) + expr = NumExpr('(a,)[0]', [('a', double)], sanitize=None) + assert_array_equal(expr(a), a) + def test_disassemble(self): assert_equal(disassemble(NumExpr( "where(m, a, -1)", [('m', bool), ('a', float)])),