Skip to content

Commit 1a0f791

Browse files
lrgirdwokv2019i
authored andcommitted
crash-decode: do not eval() linker script memory expressions
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>
1 parent 8caf175 commit 1a0f791

1 file changed

Lines changed: 72 additions & 2 deletions

File tree

scripts/sof-crash-decode.py

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,76 @@
3535
import os
3636
import json
3737
import shlex
38+
import ast
39+
import operator
40+
41+
42+
# Largest shift we accept: linker addresses/sizes fit well under 2**64, so a
43+
# bigger shift is nonsensical and only serves to build a huge integer.
44+
_MAX_SHIFT = 64
45+
# Reject any literal or intermediate result beyond 64 bits, and cap the source
46+
# length, so a crafted expression can't build a huge integer or stall the parse.
47+
_MAX_VALUE = 1 << 64
48+
_MAX_EXPR_LEN = 256
49+
50+
51+
def _trunc_div(a, b):
52+
# truncate toward zero (C semantics); Python's // floors instead
53+
q = abs(a) // abs(b)
54+
return -q if (a < 0) != (b < 0) else q
55+
56+
57+
def _lshift(a, b):
58+
# reject absurd shift counts to avoid building a massive integer
59+
if b < 0 or b > _MAX_SHIFT:
60+
raise ValueError("shift count out of range")
61+
return a << b
62+
63+
64+
# Operators allowed when evaluating arithmetic from an untrusted linker script.
65+
_SAFE_OPS = {
66+
ast.Add: operator.add, ast.Sub: operator.sub,
67+
ast.Mult: operator.mul, ast.Div: _trunc_div,
68+
ast.Mod: operator.mod, ast.LShift: _lshift,
69+
ast.RShift: operator.rshift, ast.BitOr: operator.or_,
70+
ast.BitAnd: operator.and_, ast.BitXor: operator.xor,
71+
ast.USub: operator.neg, ast.UAdd: operator.pos,
72+
}
73+
74+
75+
def safe_eval_int(expr):
76+
"""Evaluate an integer arithmetic expression without executing code.
77+
78+
A crash bundle's linker.cmd is attacker-controllable, so its MEMORY
79+
expressions must never be passed to eval(). Only integer literals and
80+
basic arithmetic operators are accepted, and values are bounded to 64
81+
bits. Anything unsupported or out of range raises an exception
82+
(ValueError, or ZeroDivisionError on '/ 0'); the caller treats any such
83+
failure as 'skip this entry'.
84+
"""
85+
def _check(value):
86+
if abs(value) >= _MAX_VALUE:
87+
raise ValueError("value out of range")
88+
return value
89+
90+
def _eval(node):
91+
if isinstance(node, ast.Expression):
92+
return _eval(node.body)
93+
if isinstance(node, ast.Constant):
94+
# reject bool (a subclass of int) and everything non-integer
95+
if type(node.value) is int:
96+
return _check(node.value)
97+
raise ValueError("non-integer constant")
98+
if isinstance(node, ast.BinOp) and type(node.op) in _SAFE_OPS:
99+
return _check(_SAFE_OPS[type(node.op)](_eval(node.left),
100+
_eval(node.right)))
101+
if isinstance(node, ast.UnaryOp) and type(node.op) in _SAFE_OPS:
102+
return _check(_SAFE_OPS[type(node.op)](_eval(node.operand)))
103+
raise ValueError("unsupported expression")
104+
105+
if len(expr) > _MAX_EXPR_LEN:
106+
raise ValueError("expression too long")
107+
return _eval(ast.parse(expr, mode='eval'))
38108

39109
XTENSA_EXCCAUSE = {
40110
0: "No Error (or IllegalInstruction)",
@@ -151,8 +221,8 @@ def parse_linker_cmd(filepath):
151221
org_expr = m_org.group(1).strip()
152222
len_expr = m_len.group(1).strip()
153223
try:
154-
org_val = eval(org_expr)
155-
len_val = eval(len_expr)
224+
org_val = safe_eval_int(org_expr)
225+
len_val = safe_eval_int(len_expr)
156226
# Ignore debug regions
157227
if not (name.startswith('.debug') or name.startswith('.stab')):
158228
regions.append({'name': name, 'start': org_val, 'end': org_val + len_val})

0 commit comments

Comments
 (0)