Incorrect assertion in Nursery::forwardBufferPointer
Categories
(Core :: JavaScript: GC, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox74 | --- | fixed |
People
(Reporter: bo13oy, Assigned: jonco)
Details
(Whiteboard: [reporter-external] [client-bounty-form] [verif?])
Attachments
(2 files)
Hello,
Tested Platforms:
1)Ubuntu 64-bit 16.04.3 (Memory 8G)+ SpiderMonkey Debug version JavaScript-C71.0.1
When using jsshell to load the poc.js, it manifests itself in the form of the following crash:
---cut---
==3014==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x606000022da0 at pc 0x55bfd56e26f1 bp 0x7ffe6957b100 sp 0x7ffe6957b0f8
READ of size 8 at 0x606000022da0 thread T0
#0 0x55bfd56e26f0 in IsWriteableAddress(void*) /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:654:11
#1 0x55bfd56e25f6 in js::Nursery::forwardBufferPointer(js::HeapSlot**) /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:678:3
#2 0x55bfd5cb1d2b in js::jit::UpdateIonJSFrameForMinorGC(JSRuntime*, js::jit::JSJitFrameIter const&) /builds/worker/workspace/build/src/js/src/jit/JitFrames.cpp:970:15
#3 0x55bfd5cb199e in js::jit::UpdateJitActivationsForMinorGC(JSRuntime*) /builds/worker/workspace/build/src/js/src/jit/JitFrames.cpp:1299:9
#4 0x55bfd56e4bb4 in js::Nursery::doCollection(JS::GCReason, js::gc::TenureCountCache&) /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:1069:3
#5 0x55bfd56e3e59 in js::Nursery::collect(JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:921:5
#6 0x55bfd562833d in js::gc::GCRuntime::minorGC(JS::GCReason, js::gcstats::PhaseKind) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7544:13
#7 0x55bfd55d0b12 in JSString* js::gc::GCRuntime::tryNewNurseryString<(js::AllowGC)1>(JSContext*, unsigned long, js::gc::AllocKind) /builds/worker/workspace/build/src/js/src/gc/Allocator.cpp:170:23
#8 0x55bfd55d110b in JSString* js::AllocateStringImpl<JSString, (js::AllowGC)1>(JSContext*, js::gc::InitialHeap) /builds/worker/workspace/build/src/js/src/gc/Allocator.cpp:210:16
#9 0x55bfd4b0533a in JSRope* JSRope::new_<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, unsigned long, js::gc::InitialHeap) /builds/worker/workspace/build/src/js/src/vm/StringType-inl.h:172:17
#10 0x55bfd4ec74f9 in JSString* js::ConcatStrings<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType) /builds/worker/workspace/build/src/js/src/vm/StringType.cpp:946:10
#11 0x14fdd0b16898 (<unknown module>)
0x606000022da0 is located 0 bytes to the right of 64-byte region [0x606000022d60,0x606000022da0)
allocated by thread T0 here:
#0 0x55bfd42d66dd in malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:145:3
#1 0x55bfd569f067 in js::HeapSlot* js_pod_arena_malloc<js::HeapSlot>(unsigned long, unsigned long) /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Utility.h:600:26
#2 0x55bfd56d6d47 in js::HeapSlot* js::MallocProvider<JS::Zone>::maybe_pod_malloc<js::HeapSlot>(unsigned long, unsigned long) /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:53:12
#3 0x55bfd564e332 in js::HeapSlot* js::MallocProvider<JS::Zone>::pod_malloc<js::HeapSlot>(unsigned long, unsigned long) /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:90:12
#4 0x55bfd564de3e in js::TenuringTracer::moveElementsToTenured(js::NativeObject*, js::NativeObject*, js::gc::AllocKind) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:3195:49
#5 0x55bfd564a98c in js::TenuringTracer::moveToTenuredSlow(JSObject*) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:3062:20
#6 0x55bfd564a290 in void js::TenuringTracer::traverse<JSObject>(JSObject**) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2772:11
#7 0x55bfd56d5b0a in ZZN2js14TenuringTracer8traverseIN2JS5ValueEEEvPT_ENKUlTyS4_E_clIP8JSObjectEEDaS4 /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2789:11
#8 0x55bfd56d5578 in ZN2js15MapGCThingTypedIZNS_14TenuringTracer8traverseIN2JS5ValueEEEvPT_EUlTyS5_E_EEDaRKS4_OS5 /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Value.h:1273:28
#9 0x55bfd564d067 in void js::TenuringTracer::traverse<JS::Value>(JS::Value*) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2788:18
#10 0x55bfd564d7ea in js::TenuringTracer::traceSlots(JS::Value*, JS::Value*) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2980:5
#11 0x55bfd564c907 in js::TenuringTracer::traceObjectSlots(js::NativeObject*, unsigned int, unsigned int) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2973:5
#12 0x55bfd564b97e in js::gc::StoreBuffer::SlotsEdge::trace(js::TenuringTracer&) const /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2851:11
#13 0x55bfd564b54e in js::gc::StoreBuffer::MonoTypeBuffer<js::gc::StoreBuffer::SlotsEdge>::trace(js::TenuringTracer&) /builds/worker/workspace/build/src/js/src/gc/Marking.cpp:2807:15
#14 0x55bfd56e4a15 in js::Nursery::doCollection(JS::GCReason, js::gc::TenureCountCache&) /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:1027:6
#15 0x55bfd56e3e59 in js::Nursery::collect(JS::GCReason) /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:921:5
#16 0x55bfd562833d in js::gc::GCRuntime::minorGC(JS::GCReason, js::gcstats::PhaseKind) /builds/worker/workspace/build/src/js/src/gc/GC.cpp:7544:13
#17 0x55bfd55d0b12 in JSString* js::gc::GCRuntime::tryNewNurseryString<(js::AllowGC)1>(JSContext*, unsigned long, js::gc::AllocKind) /builds/worker/workspace/build/src/js/src/gc/Allocator.cpp:170:23
#18 0x55bfd55d110b in JSString* js::AllocateStringImpl<JSString, (js::AllowGC)1>(JSContext*, js::gc::InitialHeap) /builds/worker/workspace/build/src/js/src/gc/Allocator.cpp:210:16
#19 0x55bfd4b0533a in JSRope* JSRope::new_<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, unsigned long, js::gc::InitialHeap) /builds/worker/workspace/build/src/js/src/vm/StringType-inl.h:172:17
#20 0x55bfd4ec74f9 in JSString* js::ConcatStrings<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType) /builds/worker/workspace/build/src/js/src/vm/StringType.cpp:946:10
#21 0x14fdd0b16898 (<unknown module>)
SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/worker/workspace/build/src/js/src/gc/Nursery.cpp:654:11 in IsWriteableAddress(void*)
Shadow bytes around the buggy address:
0x0c0c7fffc560: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
0x0c0c7fffc570: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
0x0c0c7fffc580: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
0x0c0c7fffc590: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
0x0c0c7fffc5a0: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
=>0x0c0c7fffc5b0: 00 00 00 00[fa]fa fa fa 00 00 00 00 00 00 00 00
0x0c0c7fffc5c0: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
0x0c0c7fffc5d0: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
0x0c0c7fffc5e0: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
0x0c0c7fffc5f0: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
0x0c0c7fffc600: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==3014==ABORTING
---cut---
Here's a snippet of the method.
-----source code -------
s
void js::Nursery::forwardBufferPointer(HeapSlot** pSlotsElems) {
HeapSlot* old = *pSlotsElems;
if (!isInside(old)) {
return;
}
// The new location for this buffer is either stored inline with it or in
// the forwardedBuffers table.
do {
if (ForwardedBufferMap::Ptr p = forwardedBuffers.lookup(old)) {
pSlotsElems = reinterpret_cast<HeapSlot>(p->value());
break;
}
*pSlotsElems = *reinterpret_cast<HeapSlot**>(old);
} while (false);
MOZ_ASSERT(!isInside(*pSlotsElems));
MOZ_ASSERT(IsWriteableAddress(*pSlotsElems));//heap-buffer-overflow
}
-----source code -------
Thank you,
bo13oy
Qihoo 360 Vulcan Team.
Updated•4 years ago
|
Comment 1•4 years ago
|
||
I was able to reproduce this using ASan debug JS shell builds pretty much immediately, but not in ASan opt JS shell builds at all (both builds from central).
Comment 2•4 years ago
|
||
It's a debug-only issue I think. What's happening:
- Nursery GC sets a forwarding pointer for object elements.
- Because elements capacity is 0, it's an indirect forwarding pointer (uses a table instead of storing the pointer inline).
- Later in
Nursery::forwardBufferPointer
, in debug builds we callIsWriteableAddress
which dereferences the elements pointer but that's invalid if capacity is 0. We shouldn't callIsWriteableAddress
for indirect forwarding pointers.
Updated•4 years ago
|
Comment 3•4 years ago
|
||
I think sfink worked on nursery strings, so I'll needinfo him, too.
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
•
|
||
Thanks for looking at this Jan. Yes, we can't assume that we can store a word in the target buffer (for indirect forwarded buffers).
Assignee | ||
Comment 5•4 years ago
|
||
Comment 6•4 years ago
|
||
Just to double check: this is just an overly strict assertion and not a security bug, so we can unhide this?
Assignee | ||
Comment 7•4 years ago
|
||
Yes, this is not a security issue.
Updated•4 years ago
|
Updated•4 years ago
|
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ccfa20c4155e Indirectly forwarded nursery buffers can be less that a word in size r=jandem
Comment 9•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Description
•