Skip to content

fix(inspect): handle Proxy trap exceptions when walking prototype chain #30099

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

Open
robobun wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(inspect): handle Proxy trap exceptions when walking prototype chain #30099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d048ad
fix(inspect): handle exceptions from Proxy traps in forEachProperty
robobun May 2, 2026
fb85f69
ci: retrigger
robobun May 2, 2026
cc70adf
ci: retrigger after expired jobs
robobun May 4, 2026
c4eb9e3
ci: retrigger
robobun May 4, 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
12 changes: 9 additions & 3 deletions src/jsc/bindings/bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5254,6 +5254,8 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
prototypeCount = 1;
}
}
// Ignore exceptions from Proxy "getPrototypeOf" trap.
CLEAR_IF_EXCEPTION(scope);
}
}
auto* propertyNames = vm.propertyNames;
Expand Down Expand Up @@ -5369,10 +5371,11 @@ static void JSC__JSValue__forEachPropertyImpl(JSC::EncodedJSValue JSValue0, JSC:
}

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 @@ -5444,7 +5447,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 +5450 to +5453

@claude claude Bot May 2, 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.

🟣 Note: the same getPrototype(globalObject).getObject() null-deref pattern still exists at src/bun.js/bindings/napi.cpp:1837, reachable via napi_get_all_property_names with napi_key_include_prototypes + an enumerable/writable/configurable filter on an object with a throwing Proxy in its prototype chain. This is pre-existing (the PR doesn't touch napi.cpp), but it's the exact same root cause and might be worth applying the same CLEAR_IF_EXCEPTION + null-check there as a follow-up.

Extended reasoning...

What the bug is

This PR correctly fixes the null dereference in JSC__JSValue__forEachPropertyImpl by splitting iterating->getPrototype(globalObject).getObject() into two steps, clearing any pending exception, and null-checking the returned JSValue before calling .getObject(). However, the identical unsafe pattern still exists in src/bun.js/bindings/napi.cpp:

// src/bun.js/bindings/napi.cpp:1833-1842
if (key_mode == napi_key_include_prototypes) {
    // Climb up the prototype chain to find inherited properties
    JSObject* current_object = object;
    while (!current_object->getOwnPropertyDescriptor(globalObject, key.toPropertyKey(globalObject), desc)) {
        JSObject* proto = current_object->getPrototype(globalObject).getObject();  // <-- same pattern
        if (!proto) {
            break;
        }
        current_object = proto;
    }
}

How it manifests

There are two ways for getPrototype() to return an empty JSValue here, both via Proxy traps:

  1. Throwing getOwnPropertyDescriptor trap: The while-loop condition calls current_object->getOwnPropertyDescriptor(...). If current_object is a Proxy whose getOwnPropertyDescriptor trap throws, JSC returns false and leaves the exception pending. The loop body then calls current_object->getPrototype(globalObject) with a pending exception, which returns an empty JSValue.
  2. Throwing getPrototypeOf trap: If the loop condition returns false cleanly but current_object is a Proxy whose getPrototypeOf trap throws, getPrototype() itself throws and returns an empty JSValue.

In both cases, .getObject() is then called on an empty JSValue. An empty JSValue is encoded as 0, which has no NotCellMask bits set, so isCell() returns true, asCell() returns nullptr, and nullptr->getObject() reads m_type from a null pointer — the same UBSAN null deref this PR fixes in bindings.cpp. The if (!proto) break; on the next line doesn't help because the dereference has already happened inside .getObject().

Why existing code doesn't prevent it

The NAPI_RETURN_IF_EXCEPTION(env) at line 1823 only guards the initial key collection (allPropertyKeys / ownPropertyKeys). The per-key descriptor loop at lines 1829-1845 has no RETURN_IF_EXCEPTION, CLEAR_IF_EXCEPTION, or empty-value check around getOwnPropertyDescriptor or getPrototype, so exceptions thrown by Proxy traps during the prototype walk are neither observed nor cleared.

Step-by-step proof

  1. A native addon calls napi_get_all_property_names(env, obj, napi_key_include_prototypes, napi_key_enumerable, napi_key_numbers_to_strings, &result) where obj has new Proxy(proto, { getPrototypeOf() { throw new Error('boom'); } }) somewhere in its prototype chain.
  2. allPropertyKeys collects keys from the chain (this may or may not throw depending on trap order; assume it succeeds and returns at least one key not present as an own property of obj).
  3. Because key_filter & filter_by_any_descriptor is truthy, we enter the filter loop. For a key not owned by obj, getOwnPropertyDescriptor returns false and we enter the while-loop body.
  4. object->getPrototype(globalObject) walks to the Proxy and invokes its getPrototypeOf trap, which throws. getPrototype returns an empty JSValue.
  5. .getObject() → isCell() on encoded 0 → true → asCell() → nullptr → nullptr->getObject() reads JSCell::m_type at offset from null → UBSAN null deref / crash.

(The throwing-getOwnPropertyDescriptor variant is even simpler: trap throws → returns false → loop body runs with pending exception → getPrototype returns empty → same crash.)

Impact and fix

This is reachable from any native addon that calls napi_get_all_property_names with napi_key_include_prototypes and an enumerable/writable/configurable filter on user-supplied objects. The fix is the same as what this PR applies in bindings.cpp:

JSValue protoValue = current_object->getPrototype(globalObject);
CLEAR_IF_EXCEPTION(scope);  // or RETURN_IF_EXCEPTION if NAPI should propagate
JSObject* proto = protoValue ? protoValue.getObject() : nullptr;
if (!proto) break;

plus an exception check after getOwnPropertyDescriptor in the loop condition.

Relationship to this PR

This PR does not touch napi.cpp, add callers to it, or otherwise interact with this code path — so this is strictly a pre-existing issue and should not block merging. Mentioning it only because it is the exact same root cause fixed here, found by the same fuzzer pattern, and the author may want to apply the same fix in a follow-up.

}
}

Expand Down
30 changes: 30 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 @@ -451,6 +451,36 @@ const fixture = [
},
},
),
() => {
// Proxy in the prototype chain where a getter on the underlying
// prototype throws. This used to leave a pending exception that
// caused a null dereference when walking to the next prototype.
const proto = Object.create(Object.prototype, {
foo: { get: () => 1, enumerable: true, configurable: true },
bar: {
get() {
throw new Error("boom");
},
enumerable: true,
configurable: true,
},
baz: { get: () => 2, enumerable: true, configurable: true },
});
return Object.create(new Proxy(proto, {}));
},
() => {
// Proxy in the prototype chain whose getPrototypeOf trap throws.
const proto = Object.create(Object.prototype, {
foo: { get: () => 1, enumerable: true, configurable: true },
});
return Object.create(
new Proxy(proto, {
getPrototypeOf() {
throw new Error("getPrototypeOf trap");
},
}),
);
},
];

describe("crash testing", () => {
Expand Down
Loading