Skip to content

Commit 43c0aae

Browse files
Safe eval (#8936)
Addresses [GHSA-h89g-r5pc-wxfm](GHSA-h89g-r5pc-wxfm). ### Description This introduces a `safe_eval` function to evaluate known safe expressions which do not contain member access, calls, indexing, or other expressions which could be used for code injection. Use of `eval` is replaced where appropriate. ### Types of changes <!--- Put an `x` in all the boxes that apply, and remove the not applicable items --> - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [ ] Breaking change (fix or new feature that would cause existing functionality to change). - [x] New tests added to cover the changes. - [ ] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [ ] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [ ] In-line docstrings updated. - [ ] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: Eric Kerfoot <17726042+ericspod@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 6d87117 commit 43c0aae

7 files changed

Lines changed: 192 additions & 6 deletions

File tree

docs/source/utils.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,8 @@ Ordering
8080
--------
8181
.. automodule:: monai.utils.ordering
8282
:members:
83+
84+
Safe Evaluation
85+
---------------
86+
.. automodule:: monai.utils.safeeval
87+
:members:

monai/bundle/scripts.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from textwrap import dedent
2626
from typing import Any
2727

28+
import numpy as np
2829
import torch
2930
from torch.cuda import is_available
3031

@@ -51,6 +52,7 @@
5152
min_version,
5253
optional_import,
5354
pprint_edges,
55+
safe_eval,
5456
)
5557

5658
validate, _ = optional_import("jsonschema", name="validate")
@@ -158,10 +160,12 @@ def _get_fake_spatial_shape(shape: Sequence[str | int], p: int = 1, n: int = 1,
158160
if i == "*":
159161
ret.append(any)
160162
else:
161-
for c in _get_var_names(i):
162-
if c not in ["p", "n"]:
163-
raise ValueError(f"only support variables 'p' and 'n' so far, but got: {c}.")
164-
ret.append(eval(i, {"p": p, "n": n}))
163+
bad_names = set(c for c in _get_var_names(i) if c not in {"p", "n"})
164+
if bad_names:
165+
raise ValueError(f"Only variables `p` and `n` currently supported. Invalid names: {bad_names}")
166+
167+
# evaluate using Numpy types to prevent slow Python DoS attacks
168+
ret.append(int(safe_eval(i, {"p": np.int32(p), "n": np.int32(n)}, rewrite_np=True)))
165169
else:
166170
raise ValueError(f"spatial shape items must be int or string, but got: {type(i)} {i}.")
167171
return tuple(ret)

monai/utils/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@
137137
torch_profiler_time_cpu_gpu,
138138
torch_profiler_time_end_to_end,
139139
)
140+
from .safeeval import SAFE_TYPES, safe_eval
140141
from .state_cacher import StateCacher
141142
from .tf32 import detect_default_tf32, has_ampere_or_later
142143
from .type_conversion import (

monai/utils/ordering.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ def _order_template(self, template: np.ndarray) -> np.ndarray:
148148
else:
149149
rows, columns, depths = (template.shape[0], template.shape[1], template.shape[2])
150150

151-
sequence = eval(f"self.{self.ordering_type}_idx")(rows, columns, depths)
151+
sequence = getattr(self, f"{self.ordering_type}_idx")(rows, columns, depths)
152152

153153
ordering = np.array([template[tuple(e)] for e in sequence])
154154

monai/utils/safeeval.py

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# Copyright (c) MONAI Consortium
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
12+
from __future__ import annotations
13+
14+
import ast
15+
from collections.abc import Mapping, Sequence
16+
from typing import Any
17+
18+
import numpy as np
19+
20+
__all__ = ["SAFE_TYPES", "safe_eval"]
21+
22+
# default set of safe AST node types
23+
SAFE_TYPES: Sequence[type] = (
24+
ast.Expression,
25+
ast.Name,
26+
ast.Load,
27+
ast.Constant,
28+
ast.BinOp,
29+
ast.UnaryOp,
30+
ast.Add,
31+
ast.Sub,
32+
ast.Mult,
33+
ast.Div,
34+
ast.FloorDiv,
35+
ast.Pow,
36+
ast.Mod,
37+
ast.USub,
38+
ast.UAdd,
39+
)
40+
41+
42+
class _RewriteConstNp(ast.NodeTransformer):
43+
"""Replaces int and float constants in the tree with those wrapped in Numpy types."""
44+
45+
def __init__(self, int_type_str: str, float_type_str: str):
46+
self.int_type_str = int_type_str
47+
self.float_type_str = float_type_str
48+
49+
def visit_Constant(self, node):
50+
if isinstance(node.value, (int, float)):
51+
type_str = self.int_type_str if isinstance(node.value, int) else self.float_type_str
52+
return ast.parse(f"{type_str}({node.value})")
53+
54+
return node
55+
56+
57+
def safe_eval(
58+
expr: str,
59+
globals_vars: Mapping[str, Any] | None = None,
60+
locals_vars: Mapping[str, object] | None = None,
61+
allowed_types: Sequence[type] = SAFE_TYPES,
62+
rewrite_np: bool = False,
63+
int_type_str: str = "np.int32",
64+
float_type_str: str = "np.float32",
65+
) -> Any:
66+
"""
67+
Evaluate the Python expression `expr` using `eval`, but only if it is a safe expression in that its parsed AST
68+
contains nodes whose types are given in `allowed_types`. This ensures unsafe node types are excluded, if these
69+
are present in the AST a ValueError is raised. The default set of such types in `SAFE_TYPES` ensures only
70+
expressions with constants and names can be evaluated, so excludes attribute access, indexing, and calls. Code
71+
injection is infeasible through such expressions, so this is a safe and secure way of evaluating simple expressions.
72+
73+
If `rewrite_np` is True, int and float constants in the given expression will be wrapped with Numpy types as given
74+
by `int_type_str` and `float_type_str`. These are expected to be constructor names prefixed with `np.` as Numpy
75+
will be present in the expression global variables under that name. The values can be changed to other types if
76+
needed, such as "int64". One advantage of doing this is to avoid denial-of-service attacks by attempting to evaluate
77+
an expressoini which is incredibly slow under native Python but fast (though potentially erroneous) under Numpy.
78+
79+
Args:
80+
expr: expression to evaluate, this will be stripped before parsing to avoid indentation complaints
81+
globals_vars: global variable mapping, this will be treated as read-only for this function, unlike `eval`
82+
locals_vars: local variable mapping
83+
allowed_types: sequence of allowed AST types which can be found in `expr` when parsed
84+
rewrite_np: if True, wrap int or float literals in Numpy types
85+
int_type_str: int Numpy wrapping type string
86+
float_type_str: float Numpy wrapping type string
87+
88+
Raises:
89+
ValueError: raised when any node in the AST parsed from `expr` has a type not in `allowed_types`
90+
91+
Returns:
92+
The evaluated expression value, using `eval` with `globals_vars` and `locals_vars`
93+
"""
94+
parsed = ast.parse(expr.strip(), mode="eval")
95+
96+
# collect nodes in the AST which aren't permitted and unparse them for inclusion in the exception message
97+
disallowed = [ast.unparse(n) for n in ast.walk(parsed) if not isinstance(n, tuple(allowed_types))]
98+
99+
if disallowed:
100+
raise ValueError(f"Unsafe expression `{expr}` not evaluated, contains disallowed components: {disallowed}")
101+
102+
if rewrite_np:
103+
parsed = _RewriteConstNp(int_type_str, float_type_str).visit(parsed)
104+
locals_vars = {"np": np, **(locals_vars or {})}
105+
106+
return eval(expr, dict(globals_vars) if globals_vars else None, locals_vars)

tests/utils/test_alias.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@
2323

2424

2525
class TestModuleAlias(unittest.TestCase):
26-
"""check that 'import monai.xx.file_name' returns a module"""
26+
"""
27+
Check that 'import monai.xx.file_name' returns a module. Note that this test will fail if a module has the same name
28+
as a member of that module (or any other) which is imported in a `__init__.py` file.
29+
"""
2730

2831
def test_files(self):
2932
src_dir = os.path.dirname(TESTS_PATH)

tests/utils/test_safe_eval.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Copyright (c) MONAI Consortium
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
12+
from __future__ import annotations
13+
14+
import ast
15+
import unittest
16+
17+
from parameterized import parameterized
18+
19+
from monai.utils import safe_eval
20+
21+
GOOD_EXPRS = [
22+
("1+2", None, None, 3),
23+
(" 1 + 2 ", None, None, 3),
24+
("1+2+x", {"x": 4}, None, 7),
25+
("1+2+x", None, {"x": 4}, 7),
26+
("1*2+x", {"x": 4}, None, 6),
27+
("(1+2)*3", None, None, 9),
28+
("foo+bar", {"foo": 1030}, {"bar": 204}, 1234),
29+
]
30+
31+
BAD_EXPRS = [("foo()",), ("foo.bar",), ("foo[123]",), ("(1,2)",), ("[3,4]",), ("int.__class__.__init__.__globals__",)]
32+
33+
34+
class TestSafeEval(unittest.TestCase):
35+
@parameterized.expand(GOOD_EXPRS)
36+
def test_good_exprs(self, expr, globals_vars, locals_vars, expected):
37+
"""Test valid expressions with globals/locals evaluate to correct values."""
38+
result = safe_eval(expr, globals_vars, locals_vars)
39+
self.assertEqual(result, expected)
40+
41+
@parameterized.expand(GOOD_EXPRS)
42+
def test_good_exprs_np(self, expr, globals_vars, locals_vars, expected):
43+
"""Test valid expressions with globals/locals evaluate to correct values with Numpy wrapping."""
44+
result = safe_eval(expr, globals_vars, locals_vars, rewrite_np=True)
45+
self.assertEqual(result, expected)
46+
47+
@parameterized.expand(BAD_EXPRS)
48+
def test_bad_exprs(self, expr):
49+
"""Test bad expressions correctly raise ValueError."""
50+
with self.assertRaises(ValueError):
51+
safe_eval(expr)
52+
53+
with self.assertRaises(ValueError):
54+
safe_eval(expr, rewrite_np=True)
55+
56+
def test_allowed_types(self):
57+
"""Test restricting the allowed list of types."""
58+
allowed = [ast.Expression, ast.Constant, ast.BinOp, ast.Add]
59+
result = safe_eval("1+2", allowed_types=allowed)
60+
self.assertEqual(result, 3)
61+
62+
with self.assertRaises(ValueError):
63+
safe_eval("1*2", allowed_types=allowed)
64+
65+
66+
if __name__ == "__main__":
67+
unittest.main()

0 commit comments

Comments
 (0)