1530311 - Assertion failure: capacity >= aLen, at mozilla/HashTable.h:1579 with StructuredClone

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 6924dd16f7b1 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe):

var x = [];
x[1 << 30] = 1;
let y = intern();
serialize(y, x);

Backtrace:

received signal SIGSEGV, Segmentation fault.
mozilla::detail::HashTable<JSObject* const, mozilla::HashSet<JSObject*, JSStructuredCloneWriter::TransferableObjectsHasher, js::TempAllocPolicy>::SetHashPolicy, js::TempAllocPolicy>::bestCapacity (aLen=1073741825) at mozilla/HashTable.h:1579
#0  mozilla::detail::HashTable<JSObject* const, mozilla::HashSet<JSObject*, JSStructuredCloneWriter::TransferableObjectsHasher, js::TempAllocPolicy>::SetHashPolicy, js::TempAllocPolicy>::bestCapacity (aLen=1073741825) at mozilla/HashTable.h:1579
#1  mozilla::detail::HashTable<JSObject* const, mozilla::HashSet<JSObject*, JSStructuredCloneWriter::TransferableObjectsHasher, js::TempAllocPolicy>::SetHashPolicy, js::TempAllocPolicy>::reserve (aLen=1073741825, this=0x7fffffffc480) at mozilla/HashTable.h:1991
#2  mozilla::HashSet<JSObject*, JSStructuredCloneWriter::TransferableObjectsHasher, js::TempAllocPolicy>::reserve (aLen=1073741825, this=0x7fffffffc480) at mozilla/HashTable.h:502
#3  js::MutableWrappedPtrOperations<JS::GCHashSet<JSObject*, JSStructuredCloneWriter::TransferableObjectsHasher, js::TempAllocPolicy>, JS::Rooted<JS::GCHashSet<JSObject*, JSStructuredCloneWriter::TransferableObjectsHasher, js::TempAllocPolicy> > >::reserve (len=1073741825, this=0x7fffffffc468) at js/GCHashTable.h:318
#4  JSStructuredCloneWriter::parseTransferable (this=0x7fffffffc1a0) at js/src/vm/StructuredClone.cpp:1034
#5  0x0000555555d147d2 in JSStructuredCloneWriter::init (this=0x7fffffffc1a0) at js/src/vm/StructuredClone.cpp:474
#6  WriteStructuredClone (cx=cx@entry=0x7ffff5f17000, v=..., bufp=bufp@entry=0x7fffffffc6a8, scope=scope@entry=JS::StructuredCloneScope::SameProcessSameThread, cloneDataPolicy=..., cloneDataPolicy@entry=..., cb=cb@entry=0x0, cbClosure=0x0, transferable=...) at js/src/vm/StructuredClone.cpp:621
#7  0x0000555555d14a42 in JS_WriteStructuredClone (cx=0x7ffff5f17000, value=..., bufp=0x7fffffffc6a8, scope=JS::StructuredCloneScope::SameProcessSameThread, cloneDataPolicy=..., optionalCallbacks=0x0, closure=0x0, transferable=...) at js/src/vm/StructuredClone.cpp:3008
#8  0x0000555555d14b07 in JSAutoStructuredCloneBuffer::write (this=this@entry=0x7fffffffc6a0, cx=0x7ffff5f17000, value=..., transferable=..., cloneDataPolicy=..., cloneDataPolicy@entry=..., optionalCallbacks=optionalCallbacks@entry=0x0, closure=0x0) at js/src/vm/StructuredClone.cpp:3137
#9  0x0000555555c658e1 in js::testingFunc_serialize (cx=<optimized out>, cx@entry=0x7ffff5f17000, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/TestingFunctions.cpp:3106
#10 0x00005555558f6269 in CallJSNative (cx=0x7ffff5f17000, native=0x555555c65490 <js::testingFunc_serialize(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:440
[...]
#24 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:10970
rax	0x555557c31280	93825032983168
rbx	0x7fffffffbfc0	140737488338880
rcx	0x7ffff6c1c2dd	140737333281501
rdx	0x0	0
rsi	0x555556b0f8a0	93825015019680
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffc0b0	140737488339120
rsp	0x7fffffffbf50	140737488338768
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6cc0	140737354034368
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7fffffffc040	140737488339008
r13	0x7fffffffc1a0	140737488339360
r14	0x40000001	1073741825
r15	0x1	1
rip	0x555555cfc5d0 <JSStructuredCloneWriter::parseTransferable()+1760>
=> 0x555555cfc5d0 <JSStructuredCloneWriter::parseTransferable()+1760>:	movl   $0x0,0x0
   0x555555cfc5db <JSStructuredCloneWriter::parseTransferable()+1771>:	ud2

This could be a shell-only problem in testingFunc_serialize but I am not sure about that and the assertion indicates a potential out-of-bounds. Marking s-s.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/ad30dc53e38e
user: Nicholas Nethercote
date: Fri Aug 10 18:00:29 2018 +1000
summary: Bug 1481998 - Make mozilla::Hash{Map,Set}'s entry storage allocation lazy. r=luke,sfink

Nick, is bug 1481998 a likely regressor?

Comment 2

[Nicholas Nethercote [inactive]]

Nick, is bug 1481998 a likely regressor?

Yes. Good news is that this isn't s-s.

Prior to bug 1481998, the capacity computation code that is now in
bestCapacity() was only used when a HashTable was created, after checking that
aLen <= sMaxInit.

Bug 1481998 moved the computation into bestCapacity() and added a new use of
it, within the new reserve() function. reserve() doesn't check aLen before
passing it to bestCapacity(), and bestCapacity() overflows to a small value,
and reserve() won't actually reserve any space.

There is no security problem. Subsequent vanilla insertions might fail
unexpectedly but safely. putNewInfallible() could conceivably cause problems,
but it's only used in a handful of situations where the number of entries
reserved is small and overflow can't happen.

The fix is to check aLen in reserve() before calling bestCapacity().

I checked the code in xpcom/ds/PLDHashTable.{h,cpp}, because it's
implementation is very similar. It doesn't suffer the problem because it
doesn't have a reserve() function.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Bug 1530311 - Add a length check to HashTable::reserve(). r=luke

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4cc23840811
Add a length check to HashTable::reserve(). r=luke

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b4cc23840811