From 87232bba2c01eb2ec62a7944bf5fd31268269573 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Sun, 28 Jun 2026 22:41:02 +0300 Subject: [PATCH] fix(expr-eval): bound eval_to_string against super-linear DoS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recursion-depth guard fixes the parser stack overflow, but user input actually flows through eval_to_string (command line, viewport, dynamic input — synchronous on the UI thread). Its embedded-expression fallback re-tokenises every (start, end) window, so cost is ≈O(n³): a few-KB paste of non-evaluable text or deeply nested parens froze the app for seconds to minutes even though eval_number fast-returns None in microseconds (measured: "x"×3200 → 16 s, 4 KB of parens → 51 s). Cap the fallback scan at MAX_EVAL_SCAN_LEN (256) — whole-string numbers and expressions of any length still evaluate via the fast paths; only the mixed free-text scan is bounded, returning long input unchanged. Clarify that MAX_RECURSION counts parser frames (256 ≈ 41 nested parens, not 256 levels). Add regression tests: the 41/42 nesting boundary, unary-chain and function-arg-nesting rejection, and an eval_to_string over-cap promptness test (50k junk + 100k nested parens return instantly). Co-Authored-By: Claude Opus 4.8 --- src/app/expr_eval.rs | 63 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/src/app/expr_eval.rs b/src/app/expr_eval.rs index 9e24b959d..24f5b18f4 100644 --- a/src/app/expr_eval.rs +++ b/src/app/expr_eval.rs @@ -90,10 +90,19 @@ struct Parser { pos: usize, } -/// Maximum recursion depth for the recursive-descent parser. Deeply nested -/// expressions (e.g. thousands of parentheses) otherwise overflow the stack. +/// Maximum recursion depth (parser stack frames) for the recursive-descent +/// parser. Deeply nested expressions otherwise overflow the stack. This counts +/// *frames*, not nesting levels: each parenthesis level descends ~6 methods +/// (expr→sum→product→power→unary→atom), so 256 frames ≈ 41 nested parens — well +/// past anything a human types, while still capping the stack at a safe depth. const MAX_RECURSION: usize = 256; +/// Maximum input length for `eval_to_string`'s embedded-expression fallback +/// scan. The scan is super-linear in the input length, so this bounds it to a +/// worst case of a few milliseconds. Command-line / dynamic-input fields are +/// never legitimately this long; longer text is returned unchanged. +const MAX_EVAL_SCAN_LEN: usize = 256; + impl Parser { fn new(tokens: Vec) -> Self { Self { tokens, pos: 0 } @@ -403,6 +412,17 @@ pub fn eval_to_string(input: &str) -> String { return format!("{}", v); } + // Length cap on the embedded-expression fallback below. That greedy + // longest-match rescan re-tokenises every (start, end) window, so its cost + // is super-linear (≈O(n³)): without a cap a few-KB paste of non-evaluable + // text freezes this synchronous, UI-thread caller for seconds. Whole-string + // numbers and expressions of any length already returned via the fast paths + // above; only mixed free-text scanning is bounded here. Over the cap we hand + // the text back unchanged rather than hunting for embedded sub-expressions. + if trimmed.len() > MAX_EVAL_SCAN_LEN { + return trimmed.to_string(); + } + // Fallback: scan character-by-character, finding maximal expression substrings let chars: Vec = trimmed.chars().collect(); let len = chars.len(); @@ -632,4 +652,43 @@ mod tests { let expr = "(".repeat(30) + "1" + &")".repeat(30); assert_eq!(eval_number(&expr), Some(1.0), "moderately nested expression should evaluate"); } + + #[test] + fn nesting_limit_boundary_is_pinned() { + // ~6 parser frames per paren level → 41 accepted, 42 rejected. Pinned so + // a refactor that changes the method-chain length can't silently shift + // the real depth limit without a failing test. + let ok = "(".repeat(41) + "1" + &")".repeat(41); + let over = "(".repeat(42) + "1" + &")".repeat(42); + assert_eq!(eval_number(&ok), Some(1.0), "41 levels must still evaluate"); + assert_eq!(eval_number(&over), None, "42 levels must be rejected"); + } + + #[test] + fn deep_unary_chain_is_rejected_not_overflow() { + // The unary recursion (parse_unary → parse_unary) is also depth-guarded: + // a long sign run must return None, not overflow the stack. + let expr = "-".repeat(100_000) + "1"; + assert_eq!(eval_number(&expr), None); + } + + #[test] + fn deep_function_arg_nesting_is_rejected_not_overflow() { + // parse_atom → parse_args → parse_expr is the third recursion entry. + let expr = "abs(".repeat(100_000) + "1" + &")".repeat(100_000); + assert_eq!(eval_number(&expr), None); + } + + #[test] + fn eval_to_string_over_cap_returns_promptly_unchanged() { + // Above MAX_EVAL_SCAN_LEN the super-linear fallback scan must be skipped + // entirely (DoS guard): a long non-evaluable string returns unchanged + // without freezing. Without the cap this input takes seconds. + let junk = "x".repeat(50_000); + assert_eq!(eval_to_string(&junk), junk); + // Nested parens (the class the depth guard targets) routed through the + // production entry point must also return promptly, not hang. + let nested = "(".repeat(50_000) + "1" + &")".repeat(50_000); + assert_eq!(eval_to_string(&nested), nested); + } }