From f43ed077e56cf252f457929df475d7a0ae991bc1 Mon Sep 17 00:00:00 2001 From: Fernando Macedo Date: Tue, 13 Oct 2026 18:51:31 -0300 Subject: [PATCH] fix: cap integer products, concatenation, nested collections and % formatting in the restricted evaluator The restricted evaluator capped ** and sequence replication with *, but a small untrusted document could still build a huge value: a product of two integers, a sequence concatenation with +, a nested collection that repeats a reference (cheap to build, but compared, hashed or printed item by item), or % string formatting sized by its field width. Cap the integer product, measure every sequence and collection the evaluator builds by the size of its nested items counted per reference, and reject % on str and bytes operands. Signed-off-by: Fernando Macedo --- docs/io/security.md | 15 +++-- docs/releases/3.2.2.md | 24 ++++++- statemachine/spec_parser.py | 103 +++++++++++++++++++++++++------ tests/io/test_security.py | 26 +++++--- tests/test_spec_parser_values.py | 50 +++++++++++++-- 5 files changed, 182 insertions(+), 36 deletions(-) diff --git a/docs/io/security.md b/docs/io/security.md index 8984b5bd..7c89a675 100644 --- a/docs/io/security.md +++ b/docs/io/security.md @@ -115,12 +115,15 @@ action failure. ## Arithmetic is magnitude-bounded -The restricted evaluator allows arithmetic, but `**` and `*` are capped so that a tiny -expression cannot exhaust CPU or memory: an exponentiation whose result would be enormous -(`9 ** 9 ** 9`, a ~370-million-digit number) and a sequence replication that would allocate a -huge object (`[0] * 20000000`) are both rejected before they run. Ordinary scalar arithmetic -(`x * 2`, `x ** 2`) is unaffected. Under `trusted=True` the full Python evaluator is used, so -these caps do not apply. +The restricted evaluator allows arithmetic, but caps the operators that can build a huge object +from a tiny expression, so it cannot exhaust CPU or memory. An exponentiation or an integer +product whose result would be enormous (`9 ** 9 ** 9`, a ~370-million-digit number), a +sequence replication or concatenation that would allocate a huge object (`[0] * 20000000`), a +list, tuple, set or dict whose nested items add up to a huge value (`[[0] * 1000] * 2000`, +cheap to build but slow to compare, hash or print), and `%` string formatting, whose field +width sizes the result (`'%20000000d' % 0`), are rejected before they run. Ordinary arithmetic +and collections (`x * 2`, `x ** 2`, `x + 1`, `x % 2`, `[a, b]`) are unaffected. Under +`trusted=True` the full Python evaluator is used, so these caps do not apply. ## External resource references (`src`) diff --git a/docs/releases/3.2.2.md b/docs/releases/3.2.2.md index 2ba5d087..35c51795 100644 --- a/docs/releases/3.2.2.md +++ b/docs/releases/3.2.2.md @@ -2,12 +2,31 @@ *Not released yet* -python-statemachine 3.2.2 is a bugfix release that also adds Python 3.15 support. Most of its +python-statemachine 3.2.2 is a bugfix release that also adds Python 3.15 support and closes +denial-of-service gaps in the restricted expression evaluator. Most of its fixes close gaps between what the documentation promised and what the engine did: events declared inside a `State.Compound` body, `delay` and `internal` declared on an `Event`, internal events and the order of the event queues, callbacks spread over nested state bodies, and the errors Graphviz reports when it cannot render a diagram. +## Security fixes in 3.2.2 + +### Unbounded arithmetic in the restricted evaluator (GHSA-9r2q-5523-wc47, GHSA-rfcg-j6wv-c8cr, GHSA-wq35-qxpw-w5jc) + +The magnitude caps added in 3.2.1 (GHSA-r8gj-366q-cgvj) covered `**` and sequence replication +with `*`, but other operators could still build a huge object from a small untrusted document +loaded with the default `trusted=False`: a product of two integers, a sequence concatenation +with `+`, nested collections that repeat a reference (`[[0] * 1000] * 2000` is cheap to build +but walks two million items when compared, hashed or printed), and `%` string formatting, whose +field width sizes the result. The restricted evaluator now caps the integer product, and caps +every sequence or collection it builds by the size of its nested items counted per +reference, and it rejects `%` on `str` and `bytes` operands. Numeric `%` and ordinary arithmetic are +unaffected, and `trusted=True` still uses full Python without these caps. See +[](../io/security.md). + +Reported by [@manus-pi](https://github.com/manus-pi) (GHSA-9r2q-5523-wc47, +GHSA-rfcg-j6wv-c8cr) and [@manus-use](https://github.com/manus-use) (GHSA-wq35-qxpw-w5jc). + ## Python compatibility in 3.2.2 Added Python 3.15 on the test matrix. python-statemachine 3.2.2 supports Python 3.10, 3.11, 3.12, @@ -63,6 +82,9 @@ fail after upgrading: {ref}`release-3.2.2-name-collisions`. - A state reached through its parent (`sm.door.shut`) now reports its real `is_active` and `parent`. See {ref}`release-3.2.2-nested-proxies`. +- `%` string formatting (`'%d items' % n`) in an expression of a document loaded with + `trusted=False` is now rejected and routed to `error.execution` like any failing expression. + Use `trusted=True` for documents that need it. - The diagram paths that run `dot` (the `svg` format, `_repr_svg_()`, the command line and the Sphinx directive) raise `statemachine.contrib.diagram.GraphvizError` where they raised pydot's `AssertionError` when `dot` failed. A missing `dot` raises `GraphvizNotFoundError`, which is diff --git a/statemachine/spec_parser.py b/statemachine/spec_parser.py index ee526faf..133be1c6 100644 --- a/statemachine/spec_parser.py +++ b/statemachine/spec_parser.py @@ -204,7 +204,9 @@ def decorated(*args, **kwargs): def build_collection(factory, item_exprs: "list[Callable]") -> Callable: def decorated(*args, **kwargs): - return factory(item(*args, **kwargs) for item in item_exprs) + items = [item(*args, **kwargs) for item in item_exprs] + _check_value_size(items, "collection") + return factory(items) decorated.__name__ = f"{factory.__name__}(...)" return decorated @@ -212,10 +214,12 @@ def decorated(*args, **kwargs): def build_dict(key_exprs: "list[Callable]", value_exprs: "list[Callable]") -> Callable: def decorated(*args, **kwargs): - return { - key(*args, **kwargs): value(*args, **kwargs) + items = [ + (key(*args, **kwargs), value(*args, **kwargs)) for key, value in zip(key_exprs, value_exprs, strict=True) - } + ] + _check_value_size(items, "dict") + return dict(items) decorated.__name__ = "dict(...)" return decorated @@ -383,11 +387,51 @@ def parse_expr(expr: str, variable_hook: Callable) -> Callable: ast.NotEq: build_custom_operator(operator.ne), } -# Result-size caps for the two operators that can blow up cheaply. Ordinary scalar -# arithmetic (``x * 2``, ``x ** 2``) is well under these; the caps only reject the -# denial-of-service forms (``9**9**9`` bignum, ``[0]*20000000`` sequence replication). +# Result-size caps for the values an expression can build cheaply. Ordinary arithmetic and +# collections are well under these; the caps only reject the denial-of-service forms (giant +# bignums, giant or deeply shared sequences and collections). _MAX_POW_RESULT_BITS = 4096 -_MAX_SEQUENCE_LEN = 1_000_000 +_MAX_VALUE_SIZE = 1_000_000 + +_TEXT_TYPES = (str, bytes, bytearray) +_SEQUENCE_TYPES = (*_TEXT_TYPES, list, tuple) +_CONTAINER_TYPES = (list, tuple, set, frozenset) + + +def _value_size(value) -> int: + """Size of ``value`` counting a shared reference every time it appears. + + ``[[0] * 1000] * 1000`` holds a thousand references to one list, so it is cheap to build, + but comparing, hashing or printing it walks a million elements. That walked size is what + is measured here, in elements, characters or bytes, memoized by identity so the + measurement itself stays cheap. + """ + memo: "dict[int, int]" = {} + + def size(item) -> int: + key = id(item) + if key in memo: + return memo[key] + memo[key] = 1 + if isinstance(item, _TEXT_TYPES): + total = len(item) + elif isinstance(item, int): + total = item.bit_length() // 8 + elif isinstance(item, dict): + total = sum(size(k) + size(v) for k, v in item.items()) + elif isinstance(item, _CONTAINER_TYPES): + total = sum(size(element) for element in item) + else: + total = 1 + memo[key] = max(total, 1) + return memo[key] + + return size(value) + + +def _check_value_size(value, what: str): + if _value_size(value) > _MAX_VALUE_SIZE: + raise ValueError(f"{what} is too large for the restricted evaluator") def _guarded_pow(base, exp): @@ -405,29 +449,52 @@ def _guarded_pow(base, exp): def _guarded_mul(a, b): - """``*`` with a sequence-replication cap (GHSA-r8gj-366q-cgvj). + """``*`` with a result-size cap (GHSA-r8gj-366q-cgvj). ``seq * n`` (list/str/bytes/tuple times an int) can allocate an enormous object from a - 12-character expression (``[0]*20000000``). The resulting length is checked *before* - allocating. Scalar numeric multiplication is unaffected. + 12-character expression (``[0]*20000000``), and repeatedly squaring an ``int`` doubles its + size every time. The result size is checked *before* allocating. Other operands (floats) + are passed straight through. """ + if isinstance(a, int) and isinstance(b, int): + if a.bit_length() + b.bit_length() > _MAX_POW_RESULT_BITS: + raise ValueError("'*' result is too large for the restricted evaluator") for seq, n in ((a, b), (b, a)): - if isinstance(seq, (str, bytes, bytearray, list, tuple)) and isinstance(n, int): - if n > 0 and n * len(seq) > _MAX_SEQUENCE_LEN: + if isinstance(seq, _SEQUENCE_TYPES) and isinstance(n, int): + if n > 0 and n * _value_size(seq) > _MAX_VALUE_SIZE: raise ValueError("'*' repetition is too large for the restricted evaluator") return operator.mul(a, b) -# ``**`` and ``*`` stay available for ordinary arithmetic but are wrapped so a tiny untrusted -# expression cannot exhaust CPU/memory (GHSA-r8gj-366q-cgvj). ``trusted=True`` uses the full -# Python evaluator instead, without these caps. +def _guarded_add(a, b): + """``+`` with a sequence-length cap: repeated ``x + x`` doubles a sequence every time.""" + sizes = [_value_size(x) for x in (a, b) if isinstance(x, _SEQUENCE_TYPES)] + if len(sizes) == 2 and sum(sizes) > _MAX_VALUE_SIZE: + raise ValueError("'+' result is too large for the restricted evaluator") + return operator.add(a, b) + + +def _guarded_mod(a, b): + """``%`` restricted to numbers. + + printf-style formatting (``'%2000000000d' % 0``) allocates whatever field width the + expression asks for, so it is rejected on ``str``/``bytes`` operands. + """ + if isinstance(a, _TEXT_TYPES): + raise ValueError("'%' string formatting is not allowed by the restricted evaluator") + return operator.mod(a, b) + + +# ``**``, ``*``, ``+`` and ``%`` stay available for ordinary arithmetic but are wrapped so a +# tiny untrusted expression cannot exhaust CPU/memory (GHSA-r8gj-366q-cgvj). ``trusted=True`` +# uses the full Python evaluator instead, without these caps. binary_operators = { - ast.Add: operator.add, + ast.Add: _guarded_add, ast.Sub: operator.sub, ast.Mult: _guarded_mul, ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, - ast.Mod: operator.mod, + ast.Mod: _guarded_mod, ast.Pow: _guarded_pow, } diff --git a/tests/io/test_security.py b/tests/io/test_security.py index dc33d16e..ce23be68 100644 --- a/tests/io/test_security.py +++ b/tests/io/test_security.py @@ -600,10 +600,22 @@ def _dos_expr_scxml(expr: str) -> str: @pytest.mark.parametrize("fmt", FORMATS) class TestArithmeticDoS: - """``**`` and ``*`` are magnitude-capped in the restricted evaluator: the denial-of-service - forms are rejected, but ordinary scalar arithmetic keeps working (GHSA-r8gj-366q-cgvj).""" - - @pytest.mark.parametrize("expr", ["9**9**9", "2**100000", "[0]*20000000", "'a'*20000000"]) + """``**``, ``*``, ``+`` and ``%`` are magnitude-capped in the restricted evaluator: the + denial-of-service forms are rejected, but ordinary scalar arithmetic keeps working + (GHSA-r8gj-366q-cgvj).""" + + @pytest.mark.parametrize( + "expr", + [ + "9**9**9", + "2**100000", + "[0]*20000000", + "'a'*20000000", + "3**2048 * 3**2048", + "[0]*999999 + [0]*999999", + "'%20000000d' % 0", + ], + ) def test_dos_expr_rejected(self, fmt, expr): # The cap raises while evaluating the expression, surfacing as error.execution. sm = _run_exec( @@ -615,12 +627,12 @@ def test_dos_expr_rejected(self, fmt, expr): def test_scalar_arithmetic_preserved(self, fmt): sm = _run_exec( - _dos_expr_scxml("x * 2 + 3 ** 2"), - [{"assign": {"location": "x", "expr": "x * 2 + 3 ** 2"}}], + _dos_expr_scxml("x * 2 + 3 ** 2 + 10 % 3"), + [{"assign": {"location": "x", "expr": "x * 2 + 3 ** 2 + 10 % 3"}}], fmt, ) assert "failed" not in _config(sm) - assert sm.model.x == 9 # 0 * 2 + 3 ** 2 + assert sm.model.x == 10 # 0 * 2 + 3 ** 2 + 10 % 3 @pytest.mark.parametrize("fmt", FORMATS) diff --git a/tests/test_spec_parser_values.py b/tests/test_spec_parser_values.py index 82ce8e6b..ac669c25 100644 --- a/tests/test_spec_parser_values.py +++ b/tests/test_spec_parser_values.py @@ -86,12 +86,35 @@ def test_nested_expression(self): class TestArithmeticMagnitudeCaps: - """``**`` and ``*`` stay usable but cannot blow up: the denial-of-service forms raise, - while ordinary and edge-case arithmetic pass through (GHSA-r8gj-366q-cgvj).""" + """``**``, ``*``, ``+`` and ``%`` stay usable but cannot blow up: the denial-of-service + forms raise, while ordinary and edge-case arithmetic pass through (GHSA-r8gj-366q-cgvj).""" - @pytest.mark.parametrize("expr", ["9**9**9", "2**100000", "[0]*20000000", "'a'*20000000"]) + @pytest.mark.parametrize( + "expr", + [ + "9**9**9", + "2**100000", + "[0]*20000000", + "'a'*20000000", + "3**2048 * 3**2048", + "[0]*999999 + [0]*999999", + "'a'*999999 + 'a'*999999", + "[[0]*1000]*1001", + "[[0]*1000]*500 + [[0]*1000]*501", + "[('a'*999999,)]*2", + "[3**2048]*3000", + "{((0,)*1000,)*1000, ((1,)*1000,)*1000}", + "{1: [[0]*1000]*600, 2: [[0]*1000]*600}", + "[p, p]", + ], + ) def test_dos_forms_rejected(self, expr): with pytest.raises(ValueError, match="too large"): + parse_expr(expr, hook_from({"p": [[0] * 1000] * 600}))() + + @pytest.mark.parametrize("expr", ["'%20000000d' % 0", "b'%d' % 1"]) + def test_string_formatting_rejected(self, expr): + with pytest.raises(ValueError, match="string formatting is not allowed"): parse_expr(expr, hook_from({}))() @pytest.mark.parametrize( @@ -106,10 +129,29 @@ def test_dos_forms_rejected(self, expr): ("[0] * 0", []), # non-positive count: cap check skipped ("'ab' * 2", "abab"), # small str replication ("3 * 4", 12), # scalar multiply: neither operand a sequence + ("2.5 * 2", 5.0), # float operand: int cap check skipped + ("3**2048 * 3", 3**2049), # int product under the cap + ("[1] + [2]", [1, 2]), # small sequence concatenation + ("'a' + 'b'", "ab"), # small str concatenation + ("1 + 2", 3), # scalar add: neither operand a sequence + ("10 % 3", 1), # numeric modulo + ("[[0] * 2] * 2", [[0, 0], [0, 0]]), # small nested replication + ("[[], 1.5, None]", [[], 1.5, None]), # empty and scalar items + ("{1: (2, 'a'), 3: {4: 5}}", {1: (2, "a"), 3: {4: 5}}), # nested dict + ("{(1, 2)}", {(1, 2)}), # set literal + ("[d, d]", [{1: 2}, {1: 2}]), # shared reference counted per appearance ], ) def test_bounded_and_edge_cases_pass(self, expr, expected): - assert parse_expr(expr, hook_from({}))() == expected + assert parse_expr(expr, hook_from({"d": {1: 2}}))() == expected + + def test_sets_are_measured_by_their_items(self): + assert parse_expr("[s, s]", hook_from({"s": frozenset({1, 2})}))() == [ + frozenset({1, 2}), + frozenset({1, 2}), + ] + with pytest.raises(ValueError, match="too large"): + parse_expr("[s, s]", hook_from({"s": set(range(600_000))}))() class TestRejectedExpressions: