Closed Bug 2029297 Opened 7 months ago Closed 7 months ago

Assertion failure: source->byteLength() >= sourceByteOffset in [@ ImmutableArrayBufferObject::slice] via sliceToImmutable resize side-effect

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
151 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox149 --- wontfix
firefox150 --- wontfix
firefox151 --- fixed

People

(Reporter: bugmon, Assigned: anba)

References

(Blocks 3 open bugs)

Details

(5 keywords, Whiteboard: [adv-main151+r])

Attachments

(3 files, 1 external file)

Assertion failure: source->byteLength() >= sourceByteOffset in [@ ImmutableArrayBufferObject::slice] via sliceToImmutable resize side-effect

ArrayBuffer.prototype.sliceToImmutable (Nightly-only, gated by --enable-arraybuffer-immutable / javascript.options.experimental.arraybuffer_immutable) contains an incomplete bounds re-check after the user-controllable ToIntegerIndex coercion of the end argument. When the coercion of end invokes user code that shrinks the resizable ArrayBuffer such that the post-resize length lands between final_ and first (i.e. final_ <= currentLen < first), the guard at step 14 passes incorrectly and an out-of-bounds offset is forwarded to ImmutableArrayBufferObject::slice.

The implementation at line 1096 checks currentLen < final_, which is only equivalent to the spec-mandated check first + newLen > currentLen when final_ >= first. By choosing arguments such that the coerced end is less than start, an attacker forces newLen = 0 (line 1080), making first + newLen = first, while the guard still tests against final_. The sibling implementation ArrayBufferObject::sliceImpl correctly guards with if (first < currentLen) at line 1016.

In debug builds this trips MOZ_ASSERT(source->byteLength() >= sourceByteOffset) at line 2849. In release builds the assertion is elided and execution reaches std::uninitialized_copy_n(source->dataPointer() + sourceByteOffset, 0, toFill) at line 2860. Because the bypass forces newLen=0 the copy is a no-op, and because first is clamped to the pre-resize byteLength which is <= maxByteLength (the size of the underlying mapping for resizable buffers), the pointer arithmetic stays within the reserved allocation. ASAN reports no fault. The observable release-build behavior is a spec deviation: a 0-byte immutable buffer is returned instead of a RangeError.

Build Info

Affected Code

File: js/src/vm/ArrayBufferObject.cpp, line 1061-1103

  // Step 5.
  size_t len = obj->byteLength();              // <-- 100 (pre-resize)

  // Steps 6 and 7.
  size_t first = 0;
  if (args.hasDefined(0)) {
    if (!ToIntegerIndex(cx, args[0], len, &first)) {  // <-- first = 80, clamped to old len
      return false;
    }
  }

  // Steps 6 and 8.
  size_t final_ = len;
  if (args.hasDefined(1)) {
    if (!ToIntegerIndex(cx, args[1], len, &final_)) { // <-- valueOf() resizes to 60, returns 50
      return false;
    }
  }

  // Step 9.
  size_t newLen = final_ >= first ? final_ - first : 0;  // <-- 50 < 80, so newLen = 0
  MOZ_ASSERT(newLen <= ArrayBufferObject::ByteLengthLimit);

  // Steps 10-11.
  if (obj->isDetached()) {
    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                              JSMSG_TYPED_ARRAY_DETACHED);
    return false;
  }

  // Step 13.
  //
  // Reacquire the length in case the buffer has been resized.
  size_t currentLen = obj->byteLength();       // <-- 60

  // Step 14.
  if (currentLen < final_) {                   // <-- BUG: 60 < 50 is FALSE, check passes
    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,  // Should check (first + newLen > currentLen)
                              JSMSG_ARRAYBUFFER_COPY_RANGE);  // i.e. (80 > 60) -> should throw
    return false;
  }

  // Steps 12 and 15.
  auto* newBuffer = ImmutableArrayBufferObject::slice(cx, newLen, obj, first);  // slice(cx, 0, obj, 80)

File: js/src/vm/ArrayBufferObject.cpp, line 2843-2861

/* static */ ImmutableArrayBufferObject* ImmutableArrayBufferObject::slice(
    JSContext* cx, size_t newByteLength, JS::Handle<ArrayBufferObject*> source,
    size_t sourceByteOffset) {
  MOZ_ASSERT(newByteLength <= ArrayBufferObject::ByteLengthLimit,
             "caller must validate the byte count it passes");
  MOZ_ASSERT(!source->isDetached());
  MOZ_ASSERT(source->byteLength() >= sourceByteOffset);              // <-- FAILS: 60 >= 80
  MOZ_ASSERT(source->byteLength() >= sourceByteOffset + newByteLength);

  AutoSetNewObjectMetadata metadata(cx);
  auto [newBuffer, toFill] = createBufferAndData<ImmutableArrayBufferObject,
                                                 FillContents::Uninitialized>(
      cx, newByteLength, metadata);
  if (!newBuffer) {
    return nullptr;
  }

  std::uninitialized_copy_n(source->dataPointer() + sourceByteOffset, // <-- ptr+80 into 60-byte logical buf
                            newByteLength, toFill);                   //     but newByteLength=0, no read

File: js/src/vm/ArrayBufferObject.cpp, line 1013-1022

// Comparison: regular slice() handles this correctly
size_t currentLen = obj->byteLength();

// Steps 24-25 and 27.
if (first < currentLen) {                    // <-- correct guard, missing in sliceToImmutable
  // Step 27.a.
  size_t count = std::min(newLen, currentLen - first);

  // Steps 24-25 and 27.b.
  ArrayBufferObject::copyData(unwrappedResult, 0, obj, first, count);
}

The guard at line 1096 incorrectly substitutes final_ for first + newLen. These are equal only when final_ >= first; when a valueOf side-effect on the end argument resizes the buffer and yields a value less than start, the resulting final_ < first makes newLen = 0, and the guard fails to detect that first now exceeds currentLen. The regular slice() implementation 80 lines above correctly guards on first < currentLen.

Exploit Chain

  1. Create a resizable ArrayBuffer: new ArrayBuffer(100, {maxByteLength: 200}).
  2. Call ab.sliceToImmutable(80, evilEnd) where 80 is a valid offset into the current 100-byte buffer.
  3. evilEnd.valueOf() executes during ToIntegerIndex at line 1074: it calls ab.resize(60), shrinking the buffer below first, then returns 50 (any value < 80 and <= 60 works).
  4. Line 1080: final_ (50) < first (80), so newLen = 0.
  5. Line 1093: currentLen is re-read as 60.
  6. Line 1096: check 60 < 50 is false; the guard passes despite first (80) > currentLen (60).
  7. Line 1103: ImmutableArrayBufferObject::slice is called with sourceByteOffset=80 against a 60-byte buffer.
  8. Debug build: MOZ_ASSERT at line 2849 fires. Release build: pointer arithmetic computes dataPointer()+80 (within the 200-byte maxByteLength reservation), copies 0 bytes, returns an empty immutable buffer instead of throwing RangeError.

Steps to Reproduce

  1. Build SpiderMonkey debug shell from mozilla-central (Nightly; this code is inside #ifdef NIGHTLY_BUILD).
  2. Save the testcase to test.js: var ab = new ArrayBuffer(100, {maxByteLength: 200}); ab.sliceToImmutable(80, { valueOf: function() { ab.resize(60); return 50; } });
  3. Run: js --fuzzing-safe --enable-arraybuffer-immutable test.js
  4. Observe: Assertion failure: source->byteLength() >= sourceByteOffset, at js/src/vm/ArrayBufferObject.cpp:2849

Security Impact

  • Severity: Low
  • Attacker capability: An attacker running JavaScript can trigger a debug-only MOZ_ASSERT crash. In release builds the bypass is constrained to newByteLength=0 (mathematically enforced by the same condition that enables the bypass), so std::uninitialized_copy_n performs no read. The out-of-bounds offset is bounded by the pre-resize byteLength, which itself is bounded by maxByteLength — the size of the reserved mapping for resizable buffers — so the computed source pointer remains within committed memory. No OOB read, no info leak; the only release-build effect is a spec deviation (returns a 0-byte buffer instead of throwing RangeError). Severity is Low because: (a) memory safety is not violated, (b) the feature is Nightly-only behind a pref, (c) the bypass cannot be widened to non-zero copy lengths.
  • Preconditions: Nightly build (code is inside #ifdef NIGHTLY_BUILD). Pref javascript.options.experimental.arraybuffer_immutable enabled (or shell flag --enable-arraybuffer-immutable). Attacker must be able to execute arbitrary JavaScript (standard web content threat model).

ASAN Report

[214551] Assertion failure: source->byteLength() >= sourceByteOffset, at /firefox/js/src/vm/ArrayBufferObject.cpp:2849
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==214551==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5a309c58dbaf bp 0x7ffcdfa3a840 sp 0x7ffcdfa3a800 T214551)
==214551==The signal is caused by a WRITE memory access.
==214551==Hint: address points to the zero page.
    #0 0x5a309c58dbaf in MOZ_CrashSequence(void*, long) /firefox/obj-js-debug/dist/include/mozilla/Assertions.h:235:3
    #1 0x5a309c58dbaf in js::ImmutableArrayBufferObject::slice(JSContext*, unsigned long, JS::Handle<js::ArrayBufferObject*>, unsigned long) /firefox/js/src/vm/ArrayBufferObject.cpp:2849:3
    #2 0x5a309c58d84c in js::ArrayBufferObject::sliceToImmutableImpl(JSContext*, JS::CallArgs const&) /firefox/js/src/vm/ArrayBufferObject.cpp:1103:21
    #3 0x5a309c58dd56 in bool JS::CallNonGenericMethod<&IsArrayBuffer(JS::Handle<JS::Value>), &js::ArrayBufferObject::sliceToImmutableImpl(JSContext*, JS::CallArgs const&)>(JSContext*, JS::CallArgs const&) /firefox/obj-js-debug/dist/include/js/CallNonGenericMethod.h:103:12
    #4 0x5a309c58dd56 in js::ArrayBufferObject::sliceToImmutable(JSContext*, unsigned int, JS::Value*) /firefox/js/src/vm/ArrayBufferObject.cpp:1122:10
    #5 0x5a309c54df04 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /firefox/js/src/vm/Interpreter.cpp:490:13
    #6 0x5a309c5292b5 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /firefox/js/src/vm/Interpreter.cpp:586:12
    #7 0x5a309c53905c in js::CallFromStack(JSContext*, JS::CallArgs const&, js::CallReason) /firefox/js/src/vm/Interpreter.cpp:658:10
    #8 0x5a309c53905c in js::Interpret(JSContext*, js::RunState&) /firefox/js/src/vm/Interpreter.cpp:3272:16
    #9 0x5a309c52892a in js::RunScript(JSContext*, js::RunState&) /firefox/js/src/vm/Interpreter.cpp:460:13
    #10 0x5a309c52b790 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) /firefox/js/src/vm/Interpreter.cpp:850:10
    #11 0x5a309c52bbee in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) /firefox/js/src/vm/Interpreter.cpp:880:10
    #12 0x5a309c7e5121 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) /firefox/js/src/vm/CompilationAndEvaluation.cpp:548:10
    #13 0x5a309c7e5303 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) /firefox/js/src/vm/CompilationAndEvaluation.cpp:572:10
    #14 0x5a309b12d52d in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool, bool) /firefox/js/src/shell/js.cpp:1388:10
    #15 0x5a309b12ca19 in Process(JSContext*, char const*, bool, FileKind) /firefox/js/src/shell/js.cpp
    #16 0x5a309b0e853c in ProcessArgs(JSContext*, js::cli::OptionParser*) /firefox/js/src/shell/js.cpp:12130:10
    #17 0x5a309b0e853c in Shell(JSContext*, js::cli::OptionParser*) /firefox/js/src/shell/js.cpp:12383:12
    #18 0x5a309b0df400 in main /firefox/js/src/shell/js.cpp:12789:12
    #19 0x792cddd061c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #20 0x792cddd0628a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #21 0x5a309b0a6bd8 in _start (/firefox/obj-js-debug/dist/bin/js+0x2690bd8) (BuildId: 2a964a40d2b37d7e976cf06bc05e2f06)

==214551==Register values:
rax = 0x0000000000000000  rbx = 0x0000000000000000  rcx = 0x0000000000000b21  rdx = 0x0000792cddee0563  
rdi = 0x0000792cddee1700  rsi = 0x0000000000000000  rbp = 0x00007ffcdfa3a840  rsp = 0x00007ffcdfa3a800  
 r8 = 0x0000000000000000   r9 = 0x0000000000000003  r10 = 0x0000000000000002  r11 = 0x0000000000000293  
r12 = 0x00005a30de02a640  r13 = 0x00002e457c264040  r14 = 0x0000000000000050  r15 = 0x00007ffcdfa3a880  
UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: SEGV /firefox/obj-js-debug/dist/include/mozilla/Assertions.h:235:3 in MOZ_CrashSequence(void*, long)
==214551==ABORTING
Attached file tmpnmzbfevq.js —
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Group: core-security → javascript-core-security
Blocks: 1952253
Attached file (secure) —
Assignee: nobody → andrebargull
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

(In reply to Bugmon [:jkratzer for issues] from comment #0)

[...] The observable release-build behavior is a spec deviation: a 0-byte immutable buffer is returned instead of a RangeError.

AI hallucination: Returning a zero byte immutable buffer is correct per spec.

Severity: -- → S4
Priority: -- → P3
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 151 Branch
QA Whiteboard: [sec] [qa-triage-done-c152/b151]
Whiteboard: [adv-main151+r][adv-esr115.36+r][adv-esr140.11+r]
Whiteboard: [adv-main151+r][adv-esr115.36+r][adv-esr140.11+r] → [adv-main151+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: