Error instead of crashing on deeply nested TypeScript types in the transpiler#31361
Conversation
Deeply nested types ("[[[[...", constraints of TSX generic arrow
functions, destructuring patterns inside skipped function types) recurse
through skip_type_script_type_with_opts / skip_type_script_binding with
no stack check, so a large enough input kills the process with SIGSEGV.
Add the same is_safe_to_recurse() guard used by parse_expr_common and
parse_binding so these inputs report "Maximum call stack size exceeded"
instead of overflowing the stack.
|
Warning Review limit reached
Your plan currently allows 5 reviews/hour. Refill in 46 minutes and 48 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 7:05 PM PT - May 24th, 2026
❌ @robobun, your commit 2a39f71 has some failures in 🧪 To try this PR locally: bunx bun-pr 31361That installs a local version of the PR into your bun-31361 --bun |
There was a problem hiding this comment.
LGTM — applies the existing stack_check.is_safe_to_recurse() guard to the two TS type-skipping recursion points that were missing it, with a regression test mirroring the adjacent deep-expression test.
Extended reasoning...
Overview
Two 3-line guards added to skip_type_script_binding and skip_type_script_type_with_opts in src/js_parser/parse/parse_skip_typescript.rs, plus a new subprocess-based regression test in test/bundler/transpiler/transpiler.test.js. The guards return err!("StackOverflow") when self.stack_check.is_safe_to_recurse() reports the stack is nearly exhausted, converting a SIGSEGV on pathologically deep TypeScript types into the existing "Maximum call stack size exceeded" diagnostic.
Security risks
None. This is purely defensive: it removes a denial-of-service vector (process crash on adversarial input) by bounding recursion. No new inputs are accepted, no new code paths are reachable for well-formed inputs, and the error variant is already handled at parse_entry.rs:455/848.
Level of scrutiny
Low. The change is mechanical: the identical if !self.stack_check.is_safe_to_recurse() { return Err(err!("StackOverflow")); } idiom already appears in parse_expr_common (parse/mod.rs:75), parse_binding (parse/mod.rs:974), parse_property (parse_property.rs:256), parse_stmt (parse_stmt.rs:1902), parse_type_script_namespace_stmt (parse_typescript.rs:221), and several visit/DCE passes. This PR simply extends that pattern to two recursive type-skipping functions that were overlooked. Even when the error is swallowed by a lexer_backtracker_* wrapper, the alternative path will hit the same guard at the same stack depth, so the crash is still prevented.
Other factors
- The new test is a near-verbatim copy of the adjacent
deeply nested expressions error instead of crashing the processtest (samerepeathelper, same subprocess spawn, same/Maximum call stack size exceeded|StackOverflow/check, same[exitCode, signalCode]assertion, same 60s timeout), so it fits established test conventions. - PR description reports the full transpiler test suite passes (430 pass, 0 fail) and that the test fails on the unfixed build with SIGSEGV.
- No outstanding reviewer comments; no prior reviews from me; no design decisions involved.
There was a problem hiding this comment.
LGTM — applies the same stack_check.is_safe_to_recurse() guard already used throughout the parser to the two TS type-skipping recursion entry points, with a regression test mirroring the existing deeply-nested-expressions test.
Extended reasoning...
Overview
This PR adds a stack-depth guard to two mutually/self-recursive functions in src/js_parser/parse/parse_skip_typescript.rs (skip_type_script_binding and skip_type_script_type_with_opts) so that pathologically nested TypeScript types produce a "Maximum call stack size exceeded" error instead of a SIGSEGV. It also adds a subprocess-based regression test in test/bundler/transpiler/transpiler.test.js.
Security risks
None. The change only adds an early return Err(err!("StackOverflow")) when the existing StackCheck reports the stack is nearly exhausted. It introduces no new inputs, allocations, or control-flow paths beyond an error return that the parse entry point (parse_entry.rs:455/848) already handles by logging a range error. If anything this is a robustness/DoS hardening fix (crash → recoverable error).
Level of scrutiny
Low-to-medium. While the file is part of the core JS/TS parser, the change is purely additive and mechanical: the exact 3-line if !self.stack_check.is_safe_to_recurse() { return Err(err!("StackOverflow")); } pattern already appears verbatim in parse_binding (mod.rs:974), parse_expr_common (mod.rs:75), parse_property.rs:256, parse_typescript.rs:221, and parse_stmt.rs:1902. The guard is a no-op until stack headroom is exhausted, so normal inputs are unaffected. The StackOverflow error returned inside backtracking contexts (lexer_backtracker_bool) is treated as a backtrack, which is consistent with how the sibling guarded functions already behave and still terminates because the alternate path re-hits the same guard at the same depth.
Other factors
- The new test is structurally identical to the immediately preceding
deeply nested expressions error instead of crashing the processtest (samerepeathelper, same subprocess spawn, same/Maximum call stack size exceeded|StackOverflow/error filter, same[exitCode, signalCode]assertion), so it follows established conventions. - No CODEOWNERS entry covers these paths.
- The bug-hunting system found no issues.
- The robobun comment shows CI failures on the first commit, but a
ci: retriggercommit was pushed; given the change cannot affect anything until the stack is nearly full, any failures are almost certainly unrelated/flaky.
|
CI status for this PR: the diff itself is green.
Needs a maintainer to either retry those two Buildkite jobs or merge as-is. |
* oven/main (20 new commits): webcore: free Blob's owned content type on drop (oven-sh#31358) Support cross-compiling macOS binaries from Linux (oven-sh#31303) test: forward keep-alive requests in proxy.test.ts's mock proxy (oven-sh#31352) Port Bun.stringWidth to C++ with explicit SIMD (oven-sh#31351) Fix quadratic hang reporting duplicate-binding parse errors in the transpiler (oven-sh#31341) shell: don't abort when a glob's directory prefix doesn't exist (oven-sh#31367) Error instead of crashing on deeply nested statements in the transpiler (oven-sh#31333) Fix JSX transform panic when a bare `key` prop precedes `key` with a value (oven-sh#31350) Cap ANSI markdown indentation so deeply nested lists render in linear time (oven-sh#31366) css: bound selector-list expansion when compiling nesting for older targets (oven-sh#31277) node:http2: reassemble HEADERS+CONTINUATION before HPACK decoding (oven-sh#31323) Fix `await using` expression printing `using` as `await` (oven-sh#31324) Parenthesize `async` when it starts a for-of loop initializer (oven-sh#31326) Print Infinity and negative numeric property keys as computed properties (oven-sh#31328) css: keep required grouping parens in @container conditions when minifying (oven-sh#31330) Fix panic on anonymous export default class with an auto-accessor field (oven-sh#31331) node:http2: send GOAWAY frames on stream 0 (oven-sh#31353) parser: fix Scope mismatch while visiting panic from decorators on dropped class members (oven-sh#31340) webcrypto: reject oversized BufferSource inputs instead of aborting (oven-sh#31356) Error instead of crashing on deeply nested TypeScript types in the transpiler (oven-sh#31361) Resolved conflicts: - scripts/build.ts: kept both OHOS and macOS-cross argv entries - scripts/build/config.ts: kept both OHOS and macOS-cross config fields - scripts/build/deps/webkit.ts: kept OHOS fno-pic exclusion, adopted upstream -flto=thin
escape-skip-without-bounds-recheck:
In a loop, `if (buf[i] == ESCAPE_CHAR) { i += 1; }` followed by
unconditional `i += 1` without `i + 1 < buf.len` bounds guard.
Catches oven-sh/bun#31435 (fmt.zig JS syntax highlighter OOB read).
recursive-parse-fn-without-stack-check:
Parser/visitor/scanner fn (name contains parse/skip/visit/scan)
calls itself recursively without is_safe_to_recurse() guard.
Catches oven-sh/bun#31361 / #31333 class (stack overflow on
deeply nested TypeScript types / statement blocks).
Both are pure Tier 1 token walks, zero new infrastructure.
Problem
Parser fuzzing found a 33 KB TSX input that kills the process with a hard SIGSEGV (
sig:SIGSEGV:nostack) instead of a JavaScript error:bun -e 'new Bun.Transpiler({loader:"tsx", target:"bun", minifyWhitespace:true, deadCodeElimination:true}).transformSync(Buffer.from(Bun.gunzipSync(Buffer.from("H4sIAAAAAAACA+3WMYrCQBSAYWvBO7wLiAYTo3GRPYSdWAQdRBCLJIgge3fdA4g2gsj3PZjmwRQzzb9eAwDwOX5WkS5dOu3a8BjvMIzXZ/lwest4NqNR/HZ1s09dFanN3nDlLM+nZZ6Py0k5nhdFNs2KQd8PAwDIe3kv7wEAkPfyXt4DACDv5b28BwCQ98h7eQ8AIO+R9wAAyHt5L+8BAJD38l7eAwDIe3kv7+U9AIC8R97f836Xtse6SXGum6iruP4fbdccTvv4W9zXGwAA+HI3uJJ58sqBAAA=", "base64"))).toString("latin1"))'The input is ~28,000 nested
[brackets: 2,462 open an array-literal expression, then<T extends [[[[…makes the TSX parser skip the deeply nested tuple type in the generic arrow function'sextendsconstraint. Reduced shapes hit the same thing:Cause
parse_expr_common,parse_binding,parse_stmt, and (since #31242) the visit/print/DCE passes bound their recursion withStackCheck::is_safe_to_recurse(), but the TypeScript type-skipping recursion has no check at all:skip_type_script_type_with_optsrecurses directly for tuple members, nested generics, object types, conditional types, etc. (parse_skip_typescript.rs, theT::TOpenBracketarm is the frame in the crash backtrace), andskip_type_script_bindingrecurses directly for[/{destructuring patterns inside skipped function types.So an AST-free token skip of a deep enough type walks straight off the stack. The crash frame loops in
skip_type_script_type_with_opts → skip_type_script_type_with_optsuntil the guard page is hit, which is why the crash handler prints nothing.Fix
Add the same guard the sibling parse functions already use to both functions: when
is_safe_to_recurse()reports the stack is nearly exhausted, return theStackOverflowerror, which the parse entry point already converts into a logged "Maximum call stack size exceeded" error. No behavior change for inputs that fit on the stack; the type skipper allocates no AST, so the check is the only cost per nesting level.Verification
deeply nested TypeScript types error instead of crashing the processintest/bundler/transpiler/transpiler.test.jsspawns a subprocess that transpiles the TSX generic-arrow shape (bare and nested inside 1,000 array literals, matching the fuzz input), the nested tuple type alias, and the nested function-type destructuring pattern at depths 4,000 / 20,000 / 100,000, and asserts the process exits 0 with no signal. It fails on the unfixed build (child dies with SIGSEGV) and passes with this change.bun bd test test/bundler/transpiler/— 430 pass, 0 fail.