Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

napi_unwrap does not reject objects that were never wrapped (V8 port faults, QuickJS port confuses types)

Aperta
  • Qualcosa non va?
#226 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@bghgary ci sta già lavorando.

Dal 21/8/2026.

  • #229 di @bghgary — aperta

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
68/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
cpp, javascript
Ambito
backend-api-design

Direzione di ricerca

Inizia leggendo inline napi_status Unwrap in Core/Node-API/Source/js_native_api_v8.cc e napi_unwrap in Core/Node-API/Source/js_native_api_quickjs.cc. Verifica che il percorso V8 controlli il proprio campo interno prima del dereferenziamento e che QuickJS rifiuti gli oggetti senza wrapper invece di percorrere la catena dei prototipi; è terminato quando entrambi i port restituiscono napi_invalid_arg per gli oggetti che non sono mai stati wrappati, senza accessi non sicuri né confusione di tipo.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Summary

napi_unwrap must fail when it is handed an object that was never wrapped. The V8 and QuickJS ports both violate that, so Napi::ObjectWrap<T>::Unwrap cannot safely be called on an object whose type has not already been established. On V8 it is an outright memory-safety hole.

This affects every Unwrap call site in a consumer, not one polyfill. I hit it in BabylonNative's Canvas polyfill (BabylonJS/BabylonNative#1844), where ctx.fill(Object.create(Path2D.prototype)) was an access violation.

V8 port — dereferences internal field 0 unchecked

Core/Node-API/Source/js_native_api_v8.cc, inline napi_status Unwrap(...) (~line 345). A [BABYLON-NATIVE-ADDITION] marked "Increase perf by using internal field instead of private property" replaced the private-property lookup, including its validity check:

// upstream
auto val = obj->GetPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)).ToLocalChecked();
RETURN_STATUS_IF_FALSE(env, val->IsExternal(), napi_invalid_arg);

with a bare obj->GetAlignedPointerFromInternalField(0), whose result is then dereferenced (reference->Data()).

For any object that is not a wrapped instance, internal field 0 is not a Reference*. The read returns garbage and the dereference faults. Reproduction:

const impostor = Object.create(Path2D.prototype);
ctx.fill(impostor);   // 0xC0000005

I confirmed this in a local V8 build: deterministic access violation, and it disappears when the unwrap is replaced with a checked lookup.

QuickJS port — walks the prototype chain

Core/Node-API/Source/js_native_api_quickjs.cc, napi_unwrap (~line 2495). After the fast path on js_wrap_class_id there is a "Fallback: search the prototype chain for a legacy wrapper object".

That returns some other object's native pointer — a type confusion rather than a crash. An object created with Object.create(RealType.prototype) unwraps to whatever instance is reachable on the chain. It does at least return napi_invalid_arg when nothing is found.

JSI port

No C API, and ObjectWrap<T>::Unwrap returns nullptr for a non-wrapped object (napi-inl.h:2268) rather than throwing. Safe, but inconsistent with the other two.

Why this is hard to work around downstream

While fixing the Canvas case I found no portable way to do a type check:

  • napi_type_tag_object / napi_check_object_type_tag exist only in js_native_api_v8.cc.
  • Napi::Object::DefineProperty / PropertyDescriptor are absent from the JSI port, so a non-enumerable brand cannot be installed.
  • GetInstanceData / SetInstanceData / AddCleanupHook are absent from the JSI port, so there is nowhere to keep per-Env C++ state.
  • Napi::ObjectWrap<T>::Value() throws on the QuickJS port, which rules out an identity check against a candidate instance.

I ended up branding each instance with a Napi::External<T> and validating the pointer against a registry of live addresses. That works, but every consumer having to invent this is a strong argument for fixing the ports.

Suggested fix

Restore the validity check in the V8 port (keep the internal-field fast path, but verify the field actually holds the wrapper before dereferencing), and drop the prototype-chain fallback in the QuickJS port so a non-wrapped object returns napi_invalid_arg.

Related: #225 (the napi_throw family reports failure on success), found while chasing the same PR.

Lingua principale
C++
Stelle
22
Fork
23
Merge medio
4g 8h
PR unite (30g)
3

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di BabylonJS/JsRuntimeHost

Tutte le issue di BabylonJS/JsRuntimeHost

Issue simili

Altre issue su C++

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.