fix(utils): confine safe_eval name resolution to caller-supplied variables

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, "<safe_eval>", "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, "<safe_eval>", "eval"), eval_globals, locals_vars)

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)