Fix cycle-GC use-after-free of async functions/generators with closures#6
Merged
Conversation
Port the CVE-2023-48184 / bellard#156 fix (bellard commit 7414e5f) to this quickjs-ng tree, adapted to quickjs-ng's array-based sf->var_refs[] model (JSVarRef.{var_ref_idx,stack_frame}) since bellard's list-based var_ref_list patch does not apply verbatim. The cycle collector could free a value still referenced by a suspended async generator's closure: a promise resolving function captured by a closure var (e.g. the `next`/`callback` in Node's internal/streams/readable.js createAsyncIterator) was collected as cycle garbage while a detached var_ref still pointed at it -> heap-use-after-free in gc_decref_child during a later GC. Reproduces deterministically with `for await (const x of socket)` under GC pressure; surfaces in async-heavy apps (Etherpad, undici) on normal allocation. - JSAsyncFunctionState is now a first-class GC object (JSGCObjectHeader header, is_completed, resolving_funcs[2]); the JSAsyncFunctionData wrapper is removed. Generator/async-generator data and async-fn resolve objects point at the state. - Add JSStackFrame.async_func (set in async_func_init, NULL in regular JS_CallInternal/js_call_c_function frames) and JSVarRef.async_func (the counted back-pointer) so get_var_ref can detect async frames. - Open var_refs become GC objects at creation; free_var_ref/close_var_ref release the async_func ref; all var_ref marks now mark open refs too, not just detached. - mark_children VAR_REF marks async_func for open refs; ASYNC_FUNCTION marks the frame; free_gc_object/gc_free_cycles handle the ASYNC_FUNCTION type. async_func_mark is removed. - Completion tracked via is_completed + FUNC_RET_INITIAL_YIELD; async_func_resume closes var_refs + frees the frame on completion; __async_func_free/async_func_free are refcount-driven. quickjs-ng's uncatchable-error handling in js_async_function_resume is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR ports the upstream CVE-2023-48184 fix to quickjs-ng by making async function/generator execution state visible to the cycle collector, preventing use-after-free when suspended async generators/functions have closures that keep variable references alive.
Changes:
- Promote
JSAsyncFunctionStateinto a first-class GC object and remove theJSAsyncFunctionDatawrapper. - Track async frames via
JSStackFrame.async_funcand retain a counted back-pointer from openJSVarRefto the async state so GC can traverse it. - Mark open
var_refs(not only detached ones) and integrateJS_GC_OBJ_TYPE_ASYNC_FUNCTIONinto marking and cycle-freeing logic; add completion tracking viais_completed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Arshia001
added a commit
to wasmerio/edgejs
that referenced
this pull request
Jun 30, 2026
Brings in wasmerio/napi#36 -> wasmerio/quickjs#6, porting the CVE-2023-48184 / bellard#156 fix to our quickjs-ng tree. The cycle collector no longer frees a Promise resolving function still referenced by a suspended async generator's closure var_ref (heap-use-after-free in gc_decref_child during a later GC). Surfaces in async-heavy apps iterating streams (Etherpad, undici); reproduces with `for await (const x of socket)` under GC pressure. This complements the handle-wrap teardown fix already in this branch; both are QuickJS GC use-after-free fixes. NOTE: not final until wasmerio/quickjs#6 and wasmerio/napi#36 merge; update the napi pointer to the merged commit afterward. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous comment claimed async_func and value alias in the union, but they don't overlap (value spans the same bytes as var_ref_idx/stack_frame, not async_func). Clarify that the real reason to release the async-state reference here is the transition to detached mode, after which the open-frame fields are no longer used. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous cycle-GC fix stores a frame's var_refs[] array inside its arg_buf allocation. When a still-incomplete async function/generator is collected as part of a reference cycle, async_func_free_frame() frees arg_buf without closing the open closure var_refs (it must not mutate the object graph mid cycle-collection). A var_ref captured by another cycle member that is finalized afterwards then runs free_var_ref(), which writes sf->var_refs[idx] = NULL into the already-freed arg_buf (and would also async_func_free() the freed state) -> heap-use-after-free. Neutralize any still-open var_refs into detached, empty refs before freeing arg_buf, so their eventual free_var_ref() touches neither the freed frame nor the freed async state. The captured value lives in the frame slot and is released by the arg_buf teardown, so it is not dup'd (respecting the no-graph-mutation constraint of cycle collection). The completion path is unaffected: close_var_refs() has already detached everything, so the loop is a no-op there. Reproduced deterministically by test-stream-iterator-helpers-test262-tests (SIGABRT "corrupted size vs. prev_size"); fixed and ASAN-clean after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port the CVE-2023-48184 / bellard#156 fix (bellard commit 7414e5f) to this quickjs-ng tree, adapted to quickjs-ng's array-based sf->var_refs[] model (JSVarRef.{var_ref_idx,stack_frame}) since bellard's list-based var_ref_list patch does not apply verbatim.
The cycle collector could free a value still referenced by a suspended async generator's closure: a promise resolving function captured by a closure var (e.g. the
next/callbackin Node's internal/streams/readable.js createAsyncIterator) was collected as cycle garbage while a detached var_ref still pointed at it -> heap-use-after-free in gc_decref_child during a later GC. Reproduces deterministically withfor await (const x of socket)under GC pressure; surfaces in async-heavy apps (Etherpad, undici) on normal allocation.