Skip to content

Fix null deref in Bun.inspect when Proxy getPrototypeOf trap throws #30318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 2 commits into from
Closed

Fix null deref in Bun.inspect when Proxy getPrototypeOf trap throws #30318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c02359
Fix null deref in forEachProperty when Proxy getPrototypeOf trap throws
robobun May 6, 2026
952850c
retrigger
robobun May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/jsc/bindings/bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5444,7 +5444,10 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
break;
if (iterating == globalObject)
break;
iterating = iterating->getPrototype(globalObject).getObject();
JSValue proto = iterating->getPrototype(globalObject);
// Ignore exceptions from Proxy "getPrototypeOf" trap.
CLEAR_IF_EXCEPTION(scope);
iterating = proto ? proto.getObject() : nullptr;
Comment on lines +5447 to +5450

@claude claude Bot May 6, 2026

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.

🟣 Heads up (pre-existing, not introduced by this PR): the same unguarded pattern lives at src/jsc/bindings/napi.cpp:1837 — current_object->getPrototype(globalObject).getObject() — reachable via napi_get_all_property_names with napi_key_include_prototypes + a writable/enumerable/configurable filter. A Proxy whose getPrototypeOf trap throws will cause the same null JSCell* deref there; the if (!proto) on the next line doesn't help because the UB happens inside .getObject(). Since you're fixing the same root cause here, you may want to apply the same JSValue-then-check guard there too.

Extended reasoning...

What the bug is

The exact pattern this PR fixes — calling .getObject() directly on the result of getPrototype(globalObject) without first checking whether the returned JSValue is empty — also exists at src/jsc/bindings/napi.cpp:1837:

while (!current_object->getOwnPropertyDescriptor(globalObject, key.toPropertyKey(globalObject), desc)) {
    JSObject* proto = current_object->getPrototype(globalObject).getObject();
    if (!proto) {
        break;
    }
    current_object = proto;
}

When current_object (or something in its prototype chain) is a ProxyObject whose getPrototypeOf trap throws, getPrototype() returns an empty JSValue. As demonstrated by the UBSAN trace in this PR's description, an empty JSValue (encoded as 0) passes the isCell() check (it doesn't have NotCellMask bits set), so .getObject() proceeds to call asCell()->isObject() on a null JSCell* — undefined behavior / null deref.

Why the existing if (!proto) doesn't help

It's tempting to think the if (!proto) break; on line 1838 covers this, but it doesn't: the null dereference happens inside .getObject() on line 1837, before proto is ever assigned. This is exactly the failure mode the PR fixes in bindings.cpp (the original code there was iterating->getPrototype(globalObject).getObject(), and the fix splits it into JSValue proto = ...; iterating = proto ? proto.getObject() : nullptr;).

How it's reachable

This loop sits inside napi_get_all_property_names and runs when called with key_mode = napi_key_include_prototypes and a key_filter that includes any of napi_key_writable | napi_key_enumerable | napi_key_configurable (so the per-key descriptor-lookup loop at line 1829 executes). The NAPI_RETURN_IF_EXCEPTION at line 1823 catches an exception thrown during the initial allPropertyKeys walk, but the per-key loop calls getPrototype again for every key — so a stateful trap (succeeds the first N calls, throws on call N+1), or a trap that only throws after getOwnPropertyDescriptor has run, will reach line 1837 with a pending exception and an empty return value.

Step-by-step proof

  1. Native addon calls napi_get_all_property_names(env, obj, napi_key_include_prototypes, napi_key_writable, napi_key_numbers_to_strings, &result) where obj has a Proxy in its prototype chain whose getPrototypeOf trap is stateful (e.g., returns {} the first few times, then throw new Error("trap")).
  2. Line 1818 allPropertyKeys walks the chain — trap returns normally, no exception. Line 1823 passes.
  3. key_filter & filter_by_any_descriptor is true → enter the per-key loop.
  4. For some key, getOwnPropertyDescriptor returns false on object, so we enter the while body and call current_object->getPrototype(globalObject).
  5. The Proxy trap now throws. getPrototype() returns an empty JSValue (encoded 0).
  6. .getObject() → isCell() on encoded 0 returns true → asCell() returns nullptr → nullptr->isObject() → null deref, identical to the UBSAN trace in this PR.

Impact and fix

Impact: a native module using this N-API combination on a user-controlled object with a hostile Proxy prototype hits UB / a crash. Same severity class as the bug being fixed in this PR, just on a less-traveled path.

Fix: apply the same guard this PR adds in bindings.cpp:

JSValue protoValue = current_object->getPrototype(globalObject);
RETURN_IF_EXCEPTION(scope, ...); // or CLEAR_IF_EXCEPTION + break, matching local conventions
JSObject* proto = protoValue ? protoValue.getObject() : nullptr;
if (!proto) break;

This is pre-existing — the PR doesn't touch napi.cpp and doesn't make it any worse — but since it's the same root cause and the same one-line fix, it seemed worth flagging here for completeness.

}
}

Expand Down
40 changes: 40 additions & 0 deletions test/js/bun/util/inspect.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,3 +772,43 @@ it("CustomEvent", () => {
}"
`);
});

describe("Proxy in prototype chain", () => {
it("does not crash when a Proxy getPrototypeOf trap throws", () => {
const obj = {};
Object.setPrototypeOf(
obj,
new Proxy(
{},
{
getPrototypeOf() {
throw new Error("trap");
},
},
),
);
expect(() => Bun.inspect(obj)).not.toThrow();
});

it("does not crash when the prototype is a Proxy wrapping a prototype with a throwing getter", () => {
class Base {}
Object.defineProperty(Base.prototype, "foo", {
get() {
throw new Error("bad getter");
},
enumerable: false,
});
const obj = new Base();
const originalPrototype = Object.getPrototypeOf(obj);
Object.setPrototypeOf(obj, new Proxy(originalPrototype, {}));
expect(() => Bun.inspect(obj)).not.toThrow();
});

it("does not crash when an expect() result has a Proxy prototype", () => {
const v = expect({});
const originalPrototype = Object.getPrototypeOf(v);
Object.setPrototypeOf(v, new Proxy(originalPrototype, {}));
expect(() => Bun.inspect(v)).not.toThrow();
expect(() => v.toContainKey(v)).toThrow();
});
});
Loading