scripts: do not eval() linker script expressions in crash-decode#10928
scripts: do not eval() linker script expressions in crash-decode#10928lgirdwood wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR removes the use of Python eval() when parsing attacker-controlled arithmetic from linker.cmd in the crash-decode helper, replacing it with a restricted AST-based integer expression evaluator to prevent arbitrary code execution.
Changes:
- Added
safe_eval_int()that parses expressions withast.parse(..., mode="eval")and evaluates only a whitelisted set of arithmetic operators. - Replaced
eval(org_expr)/eval(len_expr)withsafe_eval_int(...)in linker script parsing.
| if isinstance(node, ast.Constant): | ||
| if isinstance(node.value, int): | ||
| return node.value | ||
| raise ValueError("non-integer constant") |
There was a problem hiding this comment.
Fixed — the constant check is now type(node.value) is int, so booleans (a subclass of int) are rejected as non-integers.
| _SAFE_OPS = { | ||
| ast.Add: operator.add, ast.Sub: operator.sub, | ||
| ast.Mult: operator.mul, ast.Div: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.LShift: operator.lshift, | ||
| ast.RShift: operator.rshift, ast.BitOr: operator.or_, | ||
| ast.BitAnd: operator.and_, ast.BitXor: operator.xor, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos, | ||
| } |
There was a problem hiding this comment.
Changed — ast.Div now maps to a truncate-toward-zero helper (C/linker semantics) instead of Python's floor //. e.g. -7 / 2 evaluates to -3, not -4.
| return _SAFE_OPS[type(node.op)](_eval(node.operand)) | ||
| raise ValueError("unsupported expression") | ||
|
|
||
| return _eval(ast.parse(expr, mode='eval')) |
There was a problem hiding this comment.
The caller already handles this: parse_linker_cmd() wraps both safe_eval_int() calls in try: ... except Exception: pass, so a SyntaxError from ast.parse(), a ZeroDivisionError, or a RecursionError just causes that MEMORY region to be skipped rather than aborting the tool. So the simple "invalid expression -> ignore" failure mode is already in place.
| if isinstance(node, ast.BinOp) and type(node.op) in _SAFE_OPS: | ||
| return _SAFE_OPS[type(node.op)](_eval(node.left), _eval(node.right)) | ||
| if isinstance(node, ast.UnaryOp) and type(node.op) in _SAFE_OPS: | ||
| return _SAFE_OPS[type(node.op)](_eval(node.operand)) |
There was a problem hiding this comment.
Added a guard — left shifts are now capped at 64 bits (_MAX_SHIFT), which rejects things like 1 << 100000000 before a huge integer is built. Linker addresses/sizes are well under 2**64, so this doesn't restrict valid input. Deep nesting / large literals are bounded in practice by the small regex-extracted org/len expressions, and any RecursionError/MemoryError is caught by the caller (see below).
The crash-decode helper evaluated arithmetic from a user-supplied linker script with eval(), so a crafted crash bundle could run arbitrary Python on the analyst's machine. Parse the expressions with a restricted evaluator that only accepts integer literals and basic arithmetic operators. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
98f2ca6 to
bcbf2f0
Compare
The crash-decode helper evaluated arithmetic read from a user-supplied
linker script (
linker.cmd) witheval(), so a maliciously craftedlinker.cmd in a shared crash bundle could execute arbitrary Python on the
analyst's machine. Parse the MEMORY expressions with a small restricted
evaluator that only accepts integer literals and basic arithmetic
operators, rejecting anything else.