diff --git a/monai/utils/safeeval.py b/monai/utils/safeeval.py index e8f0bcb1ae..e79ae7683e 100644 --- a/monai/utils/safeeval.py +++ b/monai/utils/safeeval.py @@ -71,6 +71,13 @@ def safe_eval( expressions with constants and names can be evaluated, so excludes attribute access, indexing, and calls. Code injection is infeasible through such expressions, so this is a safe and secure way of evaluating simple expressions. + Names are resolved only against `globals_vars` and `locals_vars`, so an expression naming anything the caller did + not supply raises a `NameError` rather than silently resolving to a builtin or to a module imported here. Two + names resolve without being supplied: `__builtins__`, which is bound to an empty mapping so that nothing can be + reached through it, and `np`, which `rewrite_np` adds because the rewritten constants need it. This matters when + `allowed_types` is widened beyond `SAFE_TYPES`, since node types such as `ast.Attribute` and `ast.Call` would + otherwise make the builtins and this module's imports reachable. + If `rewrite_np` is True, int and float constants in the given expression will be wrapped with Numpy types as given by `int_type_str` and `float_type_str`. These are expected to be constructor names prefixed with `np.` as Numpy will be present in the expression global variables under that name. The values can be changed to other types if @@ -79,7 +86,8 @@ def safe_eval( Args: expr: expression to evaluate, this will be stripped before parsing to avoid indentation complaints - globals_vars: global variable mapping, this will be treated as read-only for this function, unlike `eval` + globals_vars: global variable mapping, this will be treated as read-only for this function, unlike `eval`. + An empty `__builtins__` is added to the copy unless the mapping provides its own value for that key locals_vars: local variable mapping allowed_types: sequence of allowed AST types which can be found in `expr` when parsed rewrite_np: if True, wrap int or float literals in Numpy types @@ -88,6 +96,8 @@ def safe_eval( Raises: ValueError: raised when any node in the AST parsed from `expr` has a type not in `allowed_types` + NameError: raised when `expr` names a variable not present in `globals_vars` or `locals_vars`, other than + `__builtins__` and, when `rewrite_np` is True, `np` Returns: The evaluated expression value, using `eval` with `globals_vars` and `locals_vars` @@ -105,4 +115,9 @@ def safe_eval( ast.fix_missing_locations(parsed) locals_vars = {**(locals_vars or {}), "np": np} - return eval(compile(parsed, "", "eval"), dict(globals_vars) if globals_vars else None, locals_vars) + # an explicit globals mapping stops `eval` from falling back to this module's globals, and an empty + # `__builtins__` keeps the builtins out of scope, so only names the caller supplied can be resolved + eval_globals: dict[str, Any] = dict(globals_vars) if globals_vars else {} + eval_globals.setdefault("__builtins__", {}) + + return eval(compile(parsed, "", "eval"), eval_globals, locals_vars) diff --git a/tests/utils/test_safe_eval.py b/tests/utils/test_safe_eval.py index d578ced9ac..669613e5b1 100644 --- a/tests/utils/test_safe_eval.py +++ b/tests/utils/test_safe_eval.py @@ -17,7 +17,7 @@ import numpy as np from parameterized import parameterized -from monai.utils import safe_eval +from monai.utils import SAFE_TYPES, safe_eval GOOD_EXPRS = [ ("1+2", None, None, 3), @@ -86,6 +86,48 @@ def test_rewrite_np_preserves_bool(self): result = safe_eval("False", rewrite_np=True) self.assertIs(result, False) + def test_builtins_not_in_scope(self): + """Test that builtins cannot be reached as bare names.""" + for expr in ("int", "type", "object", "abs"): + with self.subTest(expr=expr), self.assertRaises(NameError): + safe_eval(expr) + + # `__builtins__` itself still names the mapping `eval` looks names up in, but it is now empty + self.assertEqual(safe_eval("__builtins__"), {}) + + def test_module_globals_not_in_scope(self): + """Test that the names imported by the safeeval module itself cannot be reached.""" + for expr in ("np", "ast", "safe_eval", "SAFE_TYPES"): + with self.subTest(expr=expr), self.assertRaises(NameError): + safe_eval(expr) + + # `rewrite_np` supplies `np` itself, since the constants it rewrites are calls into it + self.assertIs(safe_eval("np", rewrite_np=True), np) + + def test_unknown_name_raises(self): + """Test that a name the caller did not supply raises NameError rather than resolving elsewhere.""" + with self.assertRaises(NameError): + safe_eval("x+1", {"y": 2}) + + self.assertEqual(safe_eval("x+1", {"x": 2}), 3) + + def test_widened_allowed_types_cannot_escape(self): + """Test that attribute access and calls stay harmless once `allowed_types` is widened.""" + allowed = (*SAFE_TYPES, ast.Attribute, ast.Call, ast.Subscript) + + for expr in ("int.__class__.__init__.__globals__", "np.ndarray", "safe_eval('1')"): + with self.subTest(expr=expr), self.assertRaises(NameError): + safe_eval(expr, allowed_types=allowed) + + # the builtins mapping is reachable by name but holds nothing to escape with + with self.assertRaises(KeyError): + safe_eval("__builtins__['__import__']", allowed_types=allowed) + + def test_globals_can_opt_out_of_empty_builtins(self): + """Test that a caller supplying its own `__builtins__` keeps control of the namespace.""" + result = safe_eval("abs", {"__builtins__": {"abs": abs}}) + self.assertIs(result, abs) + def test_rewrite_np_inf_constant(self): """Test that rewrite_np handles overflowing infinity literals.""" result = safe_eval("1e309", rewrite_np=True)