Skip to content

Fix crash in Bun.inspect when JSX props is non-object or circular - #30555

Closed
robobun wants to merge 1 commit into
mainfrom
farm/4670e3c1/fix-jsx-inspect-crash
Closed

Fix crash in Bun.inspect when JSX props is non-object or circular#30555
robobun wants to merge 1 commit into
mainfrom
farm/4670e3c1/fix-jsx-inspect-crash

Conversation

@robobun

@robobun robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes two crashes in Bun.inspect / console.log when formatting JSX-like elements (objects with $$typeof set to the React element symbol):

  1. Null unwrap panic — the formatter assumed props was always an object and unwrapped props.getObject() with .?. When props was a primitive (e.g. a number), this panicked with attempt to use null value.

  2. Stack overflow — the .JSX tag was not included in canHaveCircularReferences, so the visited-object map and the isSafeToRecurse stack check were skipped. A circular reference reachable through props or props.children recursed until the process segfaulted.

Found by Fuzzilli (fingerprint 11b5374beaf7e44f).

Repro

const el = {
  $$typeof: Symbol.for("react.transitional.element"),
  type: "div",
  key: null,
  ref: null,
};
el.props = el;
Bun.inspect(el); // SIGSEGV before, prints [Circular] after
const el = {
  $$typeof: Symbol.for("react.transitional.element"),
  type: "div",
  key: null,
  ref: null,
  props: 1,
};
Bun.inspect(el); // panic: attempt to use null value before, prints <div /> after

How did you verify your code works?

  • Added regression tests to test/js/bun/util/inspect.test.js
  • bun bd test test/js/bun/util/inspect.test.js — all 75 tests pass
  • USE_SYSTEM_BUN=1 bun test test/js/bun/util/inspect.test.js -t jsx — segfaults without the fix
  • bun bd test test/js/bun/console/ and test/js/web/console/console-log.test.ts pass

Bun.inspect of a JSX-like element (object with $$typeof = react.element)
assumed the `props` value was always an object, unwrapping it with `.?`.
When it was a primitive, this panicked with a null unwrap.

Additionally, the JSX formatter was not included in the set of tags that
track visited objects, so a circular reference through `props` or
`props.children` would recurse until the stack overflowed.

- Skip props handling when `props` is not an object
- Add .JSX to canHaveCircularReferences so the visited map and stack
  check apply to JSX elements
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@robobun has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 19 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0db1fdb5-1e83-4f6d-b809-93338643b9f0

📥 Commits

Reviewing files that changed from the base of the PR and between 314ffe3 and 775103f.

📒 Files selected for processing (2)
  • src/jsc/ConsoleObject.zig
  • test/js/bun/util/inspect.test.js

Comment @coderabbitai help to get the list of available commands and usage tips.

@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 2:32 AM PT - May 12th, 2026

@robobun, your commit 775103f has 1 failures in Build #53689 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30555

That installs a local version of the PR into your bun-30555 executable, so you can run:

bun-30555 --bun

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. console: guard JSX/Proxy recursion in Bun.inspect (stack overflow SIGSEGV) #29709 - Fixes both the same crashes (circular JSX + non-object props) with identical changes to ConsoleObject.zig, plus additionally patches pretty_format.zig
  2. Fix stack overflow in Bun.inspect for circular JSX elements #30126 - Fixes the circular JSX stack overflow with the same canHaveCircularReferences change, but does not fix the non-object props null unwrap

🤖 Generated with Claude Code

@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as duplicate of #29709, which applies the same ConsoleObject.zig fixes and additionally patches the same bugs in src/test_runner/pretty_format.zig with more comprehensive test coverage.

@robobun robobun closed this May 12, 2026
@robobun
robobun deleted the farm/4670e3c1/fix-jsx-inspect-crash branch May 12, 2026 09:32

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — minimal, well-tested crash fix for fuzzer-found JSX inspect issues.

Extended reasoning...

Overview

This PR fixes two crashes in Bun.inspect / console.log when formatting JSX-shaped objects (objects with $$typeof set to a React element symbol). The Zig change in src/jsc/ConsoleObject.zig is two lines: (1) add .JSX to canHaveCircularReferences so the existing visited-map and isSafeToRecurse stack guard apply, and (2) replace an unsafe props.getObject().? unwrap with orelse break :props so a non-object props is treated as no props instead of panicking. Three regression tests are added to test/js/bun/util/inspect.test.js.

Security risks

None. This is purely output-formatting code in the inspector. The change makes the code strictly more defensive (replacing a null-unwrap panic and unbounded recursion with graceful handling). No auth, crypto, filesystem, or network surface is touched.

Level of scrutiny

Low. The diff is tiny and mechanical: adding one enum variant to an existing allowlist that gates well-established cycle/stack protection (lines 2092–2122), and swapping .? for orelse break :props on a labeled block. The defer this.quote_strings = prev_quote_strings is registered before the break, so state is correctly restored on early exit. Falling through to the closing /> after the block is the same path taken when props is absent, so <div /> is the expected output for the new test.

Other factors

The bugs were found by Fuzzilli, the PR includes targeted regression tests for all three scenarios (primitive props, el.props = el, el.props.children = el), and the author reports the full inspect.test.js suite plus console tests pass. No CODEOWNERS apply to these paths and there are no outstanding human review comments. The bug-hunting system found no issues.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant