Closed Bug 1551128 Opened 2 years ago Closed 2 years ago

Crash [@ JS::BigInt::digit] or Assertion failure: idx < storage_.size(), at dist/include/mozilla/Span.h:679 with BigInt


(Core :: JavaScript Engine, defect, P1)




Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 --- unaffected
firefox67.0.1 --- unaffected
firefox68 --- fixed
firefox69 --- verified


(Reporter: gkw, Assigned: wingo)




(7 keywords, Whiteboard: [jsbugmon:update])

Crash Data


(2 files)

The following testcase crashes on mozilla-central revision b83d8a064f16 (build with AR=ar 'CC="clang -m32 -msse2 -mfpmath=sse"' 'CXX="clang++ -m32 -msse2 -mfpmath=sse"' PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig sh ./configure --target=i686-pc-linux --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests --disable-cranelift, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

function g(f) {
    f(-Number.MIN_SAFE_INTEGER, BigInt(-1));


#0 mozilla::Span<unsigned int, 4294967295u>::operator[] (this=<optimized out>, idx=<optimized out>) at /home/ubuntu/shell-cache/js-dbg-32-linux-x86_64-b83d8a064f16/objdir-js/dist/include/mozilla/Span.h:679
#1 JS::BigInt::digit (this=<optimized out>, idx=<optimized out>) at js/src/vm/BigIntType.h:75
#2 JS::BigInt::truncateAndSubFromPowerOfTwo (cx=<optimized out>, x=..., bits=9007199254740991, resultNegative=<optimized out>) at js/src/vm/BigIntType.cpp:2260
#3 0x577729ae in JS::BigInt::asUintN (cx=0xf6b1c800, x=..., bits=9007199254740991) at js/src/vm/BigIntType.cpp:2309
#4 0x576aa3f5 in js::BigIntObject::asUintN (cx=0xf6b1c800, argc=2, vp=0xf66770d0) at js/src/builtin/BigInt.cpp:167
#5 0x57663302 in CallJSNative (cx=0xf6b1c800, native=0x576aa230 <js::BigIntObject::asUintN(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443

For detailed crash information, see attachment.

This also crashes opt shell [@ mozilla::Span<unsigned int, 4294967295u>::operator[]] with [@ JS::BigInt::digit] on the stack. Not sure if this is entirely benign so setting s-s as a start.

Crash Signature: [@ JS::BigInt::digit]

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
user: Andy Wingo
date: Wed Feb 06 13:41:56 2019 +0000
summary: Bug 1522436 - Enable BigInt compilation by default r=jandem,terpri,froydnj

Andy, guessing this is BigInt-related?

Flags: needinfo?(wingo)
Regressed by: 1522436
Assignee: nobody → wingo
Flags: needinfo?(wingo)

We can simplify the example to:

BigInt.asUintN(2**52, -1n)

Which tries to represent an all-bits-set bigint as a 2**52-bit value -- this is impossible as it's too much memory.

On x86-64, this example fails "normally" with:

js> BigInt.asUintN(2**52, -1n)
typein:7:8 RangeError: BigInt is too large to allocate

But I see that the failure is on 32-bit (x86). Surely an integer overflow error in some intermediate math :( Will investigate.

So, this bug comes from silently truncating a uint64_t bit-length value to a size_t. I have done a small audit over the code and I think this pattern only happens in the asUintN and asIntN APIs -- usually bit length is a size_t, and when a bit length comes from a bigint (and not from the user), it's in the range [0,MaxBitLength]. But these asUintN/asIntN APIs take a bit length directly, as an "index", meaning they are in [0,2**53-1].

This is perfectly valid in some cases, as e.g. BigInt.asUintN(2**53-1, 0n) -- i.e. the bigint 0n truncated to 2**53-1 bits -- is just zero. Truncating a positive bigint, whether to a signed or unsigned two's complement int, can only decrease the int's size. No vulnerability there.

However truncating a negative bigint (e.g. -1n) to a positive uintn can effectively fill a big bigint with a source of cheap 1 bits. That's the case that was mishandled and that's where this patch adds a fix. Note again that the mishandling was only on 32-bit systems.

A kind request to jwalden to review :)

Flags: needinfo?(jwalden)
Priority: -- → P1

FWIW here's a try build on linux32 debug: Seems to fix the issue.

Keywords: checkin-needed

(We discussed this a bit on IRC. I pushed it without sec-approval because it only affects Nightly: BigInt is disabled in 67.)

Flags: needinfo?(jwalden)
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 786f094a30ae).
Group: javascript-core-security → core-security-release
Closed: 2 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.