Skip to content

Fix null deref in Bun.inspect when Proxy prototype traps throw #30030

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 1 commit into from
Closed

Fix null deref in Bun.inspect when Proxy prototype traps throw #30030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions src/bun.js/bindings/bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5285,10 +5285,11 @@
}

JSC::PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
if (!object->getPropertySlot(globalObject, property, slot))
continue;
bool hasProperty = object->getPropertySlot(globalObject, property, slot);
// Ignore exceptions from "Get" proxy traps.
CLEAR_IF_EXCEPTION(scope);
if (!hasProperty)
continue;

if ((slot.attributes() & PropertyAttribute::DontEnum) != 0) {
if (property == propertyNames->underscoreProto
Expand Down Expand Up @@ -5360,7 +5361,13 @@
break;
if (iterating == globalObject)
break;
iterating = iterating->getPrototype(globalObject).getObject();
JSValue proto = iterating->getPrototype(globalObject);
// Ignore exceptions from Proxy "getPrototypeOf" trap.
if (scope.exception()) [[unlikely]] {
(void)scope.tryClearException();
break;
}
iterating = proto.getObject();

Check notice on line 5370 in src/bun.js/bindings/bindings.cpp

View check run for this annotation

Claude / Claude Code Review

Same getPrototype().getObject() null-deref pattern exists in napi.cpp 

Heads up (pre-existing, not introduced by this PR): the same `getPrototype(globalObject).getObject()` null-deref pattern fixed here also exists at `src/bun.js/bindings/napi.cpp:1837` inside `napi_get_all_property_names`. A native addon calling that API with `napi_key_include_prototypes` + an enumerable/writable/configurable filter on an object whose prototype chain has a Proxy with a throwing `getPrototypeOf` (or `getOwnPropertyDescriptor`) trap will crash the same way. Might be worth applying t
Comment on lines +5364 to +5370

@claude claude Bot May 1, 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 getPrototype(globalObject).getObject() null-deref pattern fixed here also exists at src/bun.js/bindings/napi.cpp:1837 inside napi_get_all_property_names. A native addon calling that API with napi_key_include_prototypes + an enumerable/writable/configurable filter on an object whose prototype chain has a Proxy with a throwing getPrototypeOf (or getOwnPropertyDescriptor) trap will crash the same way. Might be worth applying the same guard there as a follow-up.

Extended reasoning...

What the bug is

This PR correctly fixes a null dereference in JSC__JSValue__forEachPropertyImpl where iterating->getPrototype(globalObject).getObject() crashes when a Proxy getPrototypeOf trap throws. The identical vulnerable pattern exists, untouched, at src/bun.js/bindings/napi.cpp:1837 inside napi_get_all_property_names:

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

There is no exception check between getPrototype() and .getObject(), and none after getOwnPropertyDescriptor() either (lines 1829–1861 contain zero RETURN_IF_EXCEPTION / CLEAR_IF_EXCEPTION).

Why it crashes (same mechanism as this PR)

When a Proxy getPrototypeOf trap throws, JSObject::getPrototype() returns an empty JSValue (encoded bits = 0). On JSVALUE64 an empty value reports isCell() == true with asCell() == nullptr, so JSValue::getObject() evaluates isCell() && asCell()->isObject() and dereferences a null JSCell* in ->isObject(). The if (!proto) break; at line 1838 cannot help — the segfault happens inside .getObject() before proto is ever assigned.

Code path that triggers it

  1. A native addon calls napi_get_all_property_names(env, obj, napi_key_include_prototypes, napi_key_enumerable /* or writable/configurable */, ...).
  2. obj's prototype chain contains a Proxy whose handler has a throwing getPrototypeOf trap (or a throwing getOwnPropertyDescriptor trap, which makes the while condition return false with a pending exception and then getPrototype runs under that exception).
  3. allPropertyKeys at line 1818 succeeds (it uses ownKeys, not getPrototypeOf/getOwnPropertyDescriptor), so NAPI_RETURN_IF_EXCEPTION at 1823 passes.
  4. Because key_filter & (enumerable|writable|configurable) is set, the inner loop runs.
  5. For a key not present as an own property on object, getOwnPropertyDescriptor returns false and the body executes current_object->getPrototype(globalObject).
  6. The Proxy trap throws → getPrototype() returns empty JSValue → .getObject() dereferences null → segfault.

Step-by-step proof

// JS side passed to a native addon:
const target = {};
const obj = Object.create(new Proxy(target, {
  getPrototypeOf() { throw new Error("nope"); },
  ownKeys() { return ["x"]; },                          // so allPropertyKeys yields a key
  getOwnPropertyDescriptor() { return { value: 1, enumerable: true, configurable: true }; },
}));
// Native addon:
// napi_get_all_property_names(env, obj, napi_key_include_prototypes,
//                              napi_key_enumerable, napi_key_numbers_to_strings, &result);
  • allPropertyKeys walks own keys of obj (none) then the proxy's ownKeys → ["x"]. No throw, line 1823 passes.
  • Loop iteration for "x": object (= obj) has no own "x" → getOwnPropertyDescriptor returns false → enter loop body.
  • current_object is obj; obj->getPrototype(globalObject) returns the Proxy (ordinary [[GetPrototypeOf]] on obj, no throw). proto = Proxy, current_object = Proxy.
  • Next while test: getOwnPropertyDescriptor on the Proxy for "x" returns the descriptor → loop exits. Fine so far.
  • But change the handler to getOwnPropertyDescriptor() { throw new Error("gOPD"); }: now the second while test throws and returns false, body runs again, and current_object->getPrototype(globalObject) invokes the Proxy's getPrototypeOf trap → throws → empty JSValue → .getObject() → null deref.
  • Alternatively, with a deeper chain (obj → plain → Proxy), the second body iteration calls getPrototype directly on the Proxy and crashes without needing the gOPD trap.

Either way the vulnerable line is reached with a throwing trap and no guard.

Why existing code doesn't prevent it

The only exception check in this function is NAPI_RETURN_IF_EXCEPTION(env) at line 1823, which runs before the descriptor-filtering loop. Inside the loop (1829–1861) there are no exception checks at all, so neither the throwing getOwnPropertyDescriptor nor the throwing getPrototype is caught.

Impact

Segfault of the Bun process when a native addon enumerates properties (with prototype walk + descriptor filter) on user-controlled objects whose prototype chain contains a hostile/broken Proxy. Lower exposure than the Bun.inspect case since it requires a native addon using this specific N-API call, but it's the same crash class this PR is hardening against.

Suggested fix (follow-up, not blocking)

Mirror this PR's fix in napi.cpp: after getOwnPropertyDescriptor and after getPrototype, check scope.exception() (or split out JSValue proto = current_object->getPrototype(globalObject); if (scope.exception() || !proto.isObject()) break;). Returning napi_pending_exception via NAPI_RETURN_IF_EXCEPTION inside the loop would also be reasonable.

This is pre-existing — napi.cpp is not touched by this PR and no new callers are added. Mentioning only because the PR is specifically about hardening this exact pattern.

}
}

Expand Down
38 changes: 38 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 @@ -453,6 +453,44 @@ const fixture = [
),
];

describe("Proxy prototype with throwing traps", () => {
it("getPrototypeOf trap that throws", () => {
const obj = {};
Object.setPrototypeOf(
obj,
new Proxy(
{},
{
getPrototypeOf() {
throw new Error("nope");
},
},
),
);
expect(Bun.inspect(obj)).toBe("{}");
});

it("get trap that throws", () => {
const obj = {};
const proto = {};
Object.defineProperty(proto, "x", {
get() {
throw new Error("getter throw");
},
enumerable: true,
});
Object.setPrototypeOf(
obj,
new Proxy(proto, {
get(t, k, r) {
return Reflect.get(t, k, r);
},
}),
);
expect(Bun.inspect(obj)).toBe("{}");
});
});

describe("crash testing", () => {
for (let input of fixture) {
it(`inspecting "${input.toString().slice(0, 20).replaceAll("\n", "\\n")}" doesn't crash`, async () => {
Expand Down
Loading