Skip to content

fix(expr): make the parser iterative to avoid a stack overflow#13333

Open
abendrothj wants to merge 1 commit into
uutils:mainfrom
abendrothj:fix/expr-iterative-parser
Open

fix(expr): make the parser iterative to avoid a stack overflow#13333
abendrothj wants to merge 1 commit into
uutils:mainfrom
abendrothj:fix/expr-iterative-parser

Conversation

@abendrothj

Copy link
Copy Markdown
Contributor

Fixes #13146 (reported via security advisory GHSA-gr9g-xjfw-pqfw).

Problem

The recursive-descent parser has no depth limit, so a deeply nested expression aborts with a stack overflow instead of evaluating:

$ expr $(python3 -c "print('( '*5000)") 1 $(python3 -c "print(') '*5000)")
thread 'main' has overflowed its stack
fatal runtime error: stack overflow, aborting   # SIGABRT

Each open parenthesis costs ~7 stack frames (parse_simple_expression plus the 6-level parse_precedence ladder), and nested keyword operands (length length ... 1) recurse the same way. GNU expr evaluates the same input and exits 0.

Fix

Evaluation was already converted to an explicit stack in 56c3553 for exactly this reason; this PR does the same for the two remaining recursive spots:

  • Parsing: the parse_precedence ladder + parse_simple_expression recursion is replaced with a single explicit-stack state machine using precedence climbing. Same grammar, same precedence/associativity, same error messages — nesting depth is now bounded only by memory (matching GNU).
  • Dropping: AstNode gets an iterative Drop, since destroying a deeply nested Box chain would otherwise still overflow.

Testing

  • New unit test parses, evaluates, and drops 100,000-deep paren and length nestings in-process (test threads have 2 MB stacks, so this is a strict canary).
  • New integration tests test_deeply_nested_expression and test_deeply_nested_length follow the existing test_long_input pattern (reduced depth on Windows for its 8191-char command-line limit).
  • The original repro now prints 1 with exit 0 at depth 5000 and 50000.
  • All 134 expr integration tests pass (including the GNU expr.pl-derived suite); compared output/exit codes against another expr implementation on a battery of precedence/associativity/paren cases with no differences.

The recursive-descent parser had no depth limit, so a deeply nested
expression (e.g. `expr '(' '(' ... 1 ... ')' ')'` with thousands of
parentheses, or a long chain of `length` keywords) aborted with a stack
overflow instead of evaluating. Evaluation was already made iterative in
56c3553; this converts parsing to the same explicit-stack approach
(precedence climbing) and also drops the AST iteratively so destruction
of a deep tree cannot overflow either.

Reported via security advisory GHSA-gr9g-xjfw-pqfw.

Fixes uutils#13146
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tty-eof (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/tail-n0f (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/unexpand/bounded-memory is now passing!
Note: The gnu test tests/env/env-signal-handler was skipped on 'main' but is now failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

expr: deeply nested parentheses cause a stack overflow (recursive parser)

1 participant