Crash [@ WasmGlobalExtractLane / WasmGlobalToString] — missing null check after WasmGlobalObject::create / JS_NewStringCopyZ under OOM
Categories
(Core :: JavaScript Engine: JIT, defect, P2)
Tracking
()
People
(Reporter: david.lie, Unassigned)
Details
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/147.0.0.0 Safari/537.36
Steps to reproduce:
Two shell testing helpers in js/src/builtin/TestingFunctions.cpp pass the return
value of a fallible allocation directly to args.rval().setObject() /
args.rval().setString() without null-checking, triggering a debug assertion abort
under OOM pressure.
Variant A — WasmGlobalExtractLane (TestingFunctions.cpp:1366):
Assertion failure: js::gc::IsCellPointerValid(&obj),
at js/include/js/Value.h:604
Variant B — WasmGlobalToString (TestingFunctions.cpp:1612):
Assertion failure: js::gc::IsCellPointerValid(str),
at js/include/js/Value.h:586
Both reproduce with and without --fuzzing-safe.
Steps to reproduce
Variant A (attached as poc_extract_lane.js):
gcparam("maxBytes", gcparam("gcBytes") + 4 * 1024);
var buf = new ArrayBuffer(16);
new Uint8Array(buf).fill(17);
var g = wasmGlobalFromArrayBuffer('v128', buf);
for (var i = 0; i < 5000; i++) {
wasmGlobalExtractLane(g, 'i32x4', i & 3);
}
Run:
./js --fuzzing-safe poc_extract_lane.js
Variant B (attached as poc_to_string.js):
gcparam("maxBytes", gcparam("gcBytes") + 4096);
var buf = new ArrayBuffer(16);
new Uint8Array(buf).fill(42);
var g = wasmGlobalFromArrayBuffer('v128', buf);
for (var i = 0; i < 10000; i++) {
wasmGlobalToString(g);
}
Run:
./js --fuzzing-safe poc_to_string.js
The gcparam("maxBytes", …) call caps the GC heap 4 KB above the current live
data. After a few hundred loop iterations the cap is reached, the allocation
inside WasmGlobalExtractLane or WasmGlobalToString returns null, and the
unchecked setObject / setString asserts.
No additional flags are required.
Root cause
Variant A — WasmGlobalExtractLane (TestingFunctions.cpp:1364–1366):
Rooted<WasmGlobalObject*> result(
cx, WasmGlobalObject::create(cx, val, false, proto));
args.rval().setObject(*result.get()); // ← no null check
WasmGlobalObject::create returns nullptr on OOM; *result.get() dereferences
null and passes a null reference to setObject(JSObject&).
The sibling helper WasmGlobalFromArrayBuffer (lines 1220–1232) already has the
correct null checks — this is a copy-paste omission in WasmGlobalExtractLane.
Variant B — WasmGlobalToString (TestingFunctions.cpp:1612):
args.rval().setString(JS_NewStringCopyZ(cx, result.get())); // ← no null check
JS_NewStringCopyZ returns nullptr on OOM; the result is passed directly to
setString(JSString*).
Fix
Add null checks consistent with the pattern in WasmGlobalFromArrayBuffer:
Variant A (after line 1365):
if (!result) {
return false;
}
Also add a proto null-check (after line 1363):
if (!proto) {
return false;
}
Variant B (split the one-liner at line 1612):
JSString* str = JS_NewStringCopyZ(cx, result.get());
if (!str) {
return false;
}
args.rval().setString(str);
The rest of TestingFunctions.cpp is worth a quick audit for the same pattern.
Build / repro environment
- Engine: SpiderMonkey
JavaScript-C142.0a1 - Tree HEAD:
5836a062726f("Bug 1960567 — remove last C++ scriptable APIs for legacy telemetry") - Platform: Linux x86_64
Actual results:
Actual results
Variant A:
[PID] Assertion failure: js::gc::IsCellPointerValid(&obj),
at /…/js/include/js/Value.h:604
Stack (UBSan symbolised):
#0 MOZ_CrashSequence(void*, long) mozilla/Assertions.h:248
#1 JS::Value::setObject(JSObject&) js/Value.h:604
#2 JS::ObjectValue(JSObject&) js/Value.h:1150
#3 MutableWrappedPtrOperations<…>::setObject js/Value.h:1362
#4 WasmGlobalExtractLane(…) builtin/TestingFunctions.cpp:1366
#5 [anon:js-executable-memory] (call trampoline)
Variant B:
[PID] Assertion failure: js::gc::IsCellPointerValid(str),
at /…/js/include/js/Value.h:586
Stack (UBSan symbolised):
#0 MOZ_CrashSequence(void*, long) mozilla/Assertions.h:248
#1 JS::Value::setString(JSString*) js/Value.h:586
#2 WasmGlobalToString(…) builtin/TestingFunctions.cpp:1612
#3 [anon:js-executable-memory] (call trampoline)
The follow-on UBSan SEGV is the deliberate MOZ_CrashSequence null write, not
an independent fault.
Expected results:
Expected results
Both functions should return false (with an OOM exception already set on the
context) when the internal allocation fails, rather than asserting.
Updated•1 month ago
|
Description
•