Closed
Bug 1415883
(CVE-2018-5094)
Opened 7 years ago
Closed 7 years ago
Heap-buffer-overflow READ 8 with async generators
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: Alex_Gaynor, Assigned: jandem)
References
Details
(Keywords: csectype-bounds, oss-fuzz, sec-high, Whiteboard: [adv-main58+][post-critsmash-triage])
Attachments
(2 files)
1.70 KB,
application/x-javascript
|
Details | |
1.46 KB,
patch
|
arai
:
review+
abillings
:
approval-mozilla-beta+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
This bug was found by Google's OSS-Fuzz running their custom internal JS fuzzer. I am refiling it in our issue tracker.
Please note that they apply a 90-day disclose timeline to all bugs:
[Environment] ASAN_OPTIONS = redzone=256:strict_memcmp=0:allow_user_segv_handler=1:allocator_may_return_null=1:handle_sigfpe=1:handle_sigbus=1:detect_stack_use_after_return=0:alloc_dealloc_mismatch=0:print_scariness=1:max_uar_stack_size_log=16:detect_odr_violation=0:handle_sigill=1:coverage=0:use_sigaltstack=1:fast_unwind_on_fatal=1:detect_leaks=0:print_summary=1:handle_abort=1:check_malloc_usable_size=0:detect_container_overflow=1:symbolize=0:handle_segv=1
=================================================================
==29026==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61100032bcc0 at pc 0x0000005ff8fc bp 0x7fff9dd8dbb0 sp 0x7fff9dd8dba8
READ of size 8 at 0x61100032bcc0 thread T0
SCARINESS: 23 (8-byte-read-heap-buffer-overflow)
#0 0x5ff8fb in JS::Value::toTag() const mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Value.h:437:32
#1 0x5ff8fb in JS::Value::isString() const mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Value.h:512
#2 0x5ff8fb in _ZN2js13DispatchTypedINS_17PreBarrierFunctorIN2JS5ValueEEEJEEEDTclfp_scP8JSObjectLDnEspclsr7mozillaE7ForwardIT0_Efp1_EEET_RKS3_DpOS7_ mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Value.h:1431
#3 0x6ed1e9 in js::InternalBarrierMethods<JS::Value>::preBarrier(JS::Value const&) mozilla-central/js/src/gc/Barrier.h:283:9
#4 0x6ed1e9 in js::WriteBarrieredBase<JS::Value>::pre() mozilla-central/js/src/gc/Barrier.h:374
#5 0x6ed1e9 in js::HeapSlot::~HeapSlot() mozilla-central/js/src/gc/Barrier.h:678
#6 0x6ed1e9 in js::NativeObject::prepareElementRangeForOverwrite(unsigned long, unsigned long) mozilla-central/js/src/vm/NativeObject.h:1007
#7 0x6ed1e9 in js::NativeObject::setDenseInitializedLengthUnchecked(unsigned int) mozilla-central/js/src/vm/NativeObject.h:1179
#8 0x1e243d0 in js::NativeObject::setDenseInitializedLength(unsigned int) mozilla-central/js/src/vm/NativeObject.h:1196:9
#9 0x1e243d0 in js::AsyncGeneratorRequest* js::ShiftFromList<js::AsyncGeneratorRequest>(JSContext*, JS::Handle<js::NativeObject*>) mozilla-central/js/src/vm/List-inl.h:62
#10 0x1e10267 in js::AsyncGeneratorObject::dequeueRequest(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/vm/AsyncIteration.cpp:374:12
#11 0xa847c4 in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2769:13
#12 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#13 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#14 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#15 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#16 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#17 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#18 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#19 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#20 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#21 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#22 0x1e0bfd7 in js::AsyncGeneratorResume(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, js::CompletionKind, JS::Handle<JS::Value>) mozilla-central/js/src/vm/AsyncIteration.cpp:0
#23 0xa867e8 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2918:12
#24 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#25 0x1e0bfd7 in js::AsyncGeneratorResume(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, js::CompletionKind, JS::Handle<JS::Value>) mozilla-central/js/src/vm/AsyncIteration.cpp:0
#26 0xb405e3 in AsyncGeneratorPromiseReactionJob(JSContext*, JS::Handle<PromiseReactionRecord*>, JS::MutableHandle<JS::Value>) mozilla-central/js/src/builtin/Promise.cpp:1122:14
#27 0xb405e3 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) mozilla-central/js/src/builtin/Promise.cpp:1201
#28 0x8bc784 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) mozilla-central/js/src/jscntxtinlines.h:291:15
#29 0x8bc784 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) mozilla-central/js/src/vm/Interpreter.cpp:472
#30 0x8be967 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) mozilla-central/js/src/vm/Interpreter.cpp:540:10
#31 0x18777ce in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) mozilla-central/js/src/jsapi.cpp:3032:12
#32 0x18d8525 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JSObject*>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) mozilla-central/js/src/jsapi.h:3595:12
#33 0x18d8525 in js::RunJobs(JSContext*) mozilla-central/js/src/jscntxt.cpp:1213
#34 0x56a2f4 in Shell(JSContext*, js::cli::OptionParser*, char**) mozilla-central/js/src/shell/js.cpp:8561:9
#35 0x56a2f4 in main mozilla-central/js/src/shell/js.cpp:8967
#36 0x7fc3a22c282f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/libc-start.c:291
0x61100032bcc0 is located 0 bytes to the right of 64-byte region [0x61100032bc80,0x61100032bcc0)
allocated by thread T0 here:
#0 0x51a300 in __interceptor_realloc _asan_rtl_
#1 0x1913512 in js_realloc(void*, unsigned long) mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Utility.h:406:12
#2 0x1913512 in unsigned char* js_pod_realloc<unsigned char>(unsigned char*, unsigned long, unsigned long) mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Utility.h:594
#3 0x1913512 in unsigned char* js::MallocProvider<JS::Zone>::maybe_pod_realloc<unsigned char>(unsigned char*, unsigned long, unsigned long) mozilla-central/js/src/vm/MallocProvider.h:70
#4 0x1913512 in unsigned char* js::MallocProvider<JS::Zone>::pod_realloc<unsigned char>(unsigned char*, unsigned long, unsigned long) mozilla-central/js/src/vm/MallocProvider.h:173
#5 0x2a1f9d3 in js::Nursery::reallocateBuffer(JSObject*, void*, unsigned long, unsigned long) mozilla-central/js/src/gc/Nursery.cpp:403:29
#6 0x20e78c9 in js::HeapSlot* js::ReallocateObjectBuffer<js::HeapSlot>(JSContext*, JSObject*, js::HeapSlot*, unsigned int, unsigned int) mozilla-central/js/src/gc/Nursery-inl.h:135:48
#7 0x20e78c9 in js::NativeObject::shrinkElements(JSContext*, unsigned int) mozilla-central/js/src/vm/NativeObject.cpp:1028
#8 0x1e24364 in js::AsyncGeneratorRequest* js::ShiftFromList<js::AsyncGeneratorRequest>(JSContext*, JS::Handle<js::NativeObject*>) mozilla-central/js/src/vm/List-inl.h:59:15
#9 0x1e10267 in js::AsyncGeneratorObject::dequeueRequest(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/vm/AsyncIteration.cpp:374:12
#10 0xa847c4 in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2769:13
#11 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#12 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#13 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#14 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#15 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#16 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#17 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#18 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#19 0xa86073 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2890:16
#20 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#21 0x1e0bfd7 in js::AsyncGeneratorResume(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, js::CompletionKind, JS::Handle<JS::Value>) mozilla-central/js/src/vm/AsyncIteration.cpp:0
#22 0xa867e8 in AsyncGeneratorResumeNext(JSContext*, JS::Handle<js::AsyncGeneratorObject*>) mozilla-central/js/src/builtin/Promise.cpp:2918:12
#23 0xa84d8b in js::AsyncGeneratorResolve(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, JS::Handle<JS::Value>, bool) mozilla-central/js/src/builtin/Promise.cpp:2790:10
#24 0x1e0bfd7 in js::AsyncGeneratorResume(JSContext*, JS::Handle<js::AsyncGeneratorObject*>, js::CompletionKind, JS::Handle<JS::Value>) mozilla-central/js/src/vm/AsyncIteration.cpp:0
#25 0xb405e3 in AsyncGeneratorPromiseReactionJob(JSContext*, JS::Handle<PromiseReactionRecord*>, JS::MutableHandle<JS::Value>) mozilla-central/js/src/builtin/Promise.cpp:1122:14
#26 0xb405e3 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) mozilla-central/js/src/builtin/Promise.cpp:1201
#27 0x8bc784 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) mozilla-central/js/src/jscntxtinlines.h:291:15
#28 0x8bc784 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) mozilla-central/js/src/vm/Interpreter.cpp:472
#29 0x8be967 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) mozilla-central/js/src/vm/Interpreter.cpp:540:10
#30 0x18777ce in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) mozilla-central/js/src/jsapi.cpp:3032:12
#31 0x18d8525 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JSObject*>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) mozilla-central/js/src/jsapi.h:3595:12
#32 0x18d8525 in js::RunJobs(JSContext*) mozilla-central/js/src/jscntxt.cpp:1213
#33 0x56a2f4 in Shell(JSContext*, js::cli::OptionParser*, char**) mozilla-central/js/src/shell/js.cpp:8561:9
#34 0x56a2f4 in main mozilla-central/js/src/shell/js.cpp:8967
#35 0x7fc3a22c282f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/libc-start.c:291
SUMMARY: AddressSanitizer: heap-buffer-overflow (/mnt/scratch0/clusterfuzz/slave-bot/builds/clusterfuzz-builds-no-engine_spidermonkey_6aad6e0d14f81d36f48dbd887aa56b38e87859f7/revisions/js+0x5ff8fb)
Shadow bytes around the buggy address:
0x0c228005d740: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d750: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d760: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c228005d770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c228005d790: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
0x0c228005d7a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d7b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d7c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d7d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c228005d7e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==29026==ABORTING
Updated•7 years ago
|
Group: core-security → javascript-core-security
Comment 2•7 years ago
|
||
confirmed similar heap-buffer-overflow on macOS here.
I'll look into it
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Comment 3•7 years ago
|
||
here's reduced testcase.
looks like GC related (JS_GC_ZEAL=4 is necessary).
asan build
revision: m-i 486620c2345a
configure flag: --disable-jemalloc --enable-address-sanitizer --enable-debug --disable-optimize
build env: JS_GC_ZEAL=IncrementalMultipleSlices LLVM_SYMBOLIZER={TOOLTOOL_CHECKOUT}/clang/bin/llvm-symbolizer
run env: JS_GC_ZEAL=4
var f = async function* () {
yield 1;
};
var g = f();
g.next();
g.next();
g.next();
g.next();
g.next();
g.next();
g.next();
Comment 4•7 years ago
|
||
here's custom variant for autospider, just in case
{"debug": true, "configure-args": " --disable-jemalloc --enable-address-sanitizer", "optimize": false, "env": {"JS_GC_ZEAL": "IncrementalMultipleSlices", "LLVM_SYMBOLIZER": "{TOOLTOOL_CHECKOUT}/clang/bin/llvm-symbolizer"}, "compiler": "clang"}
Comment 5•7 years ago
|
||
apparently elements_ points out-of-bound after shrink from length=1 to 0.
https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/js/src/vm/List-inl.h#59-62
> template<class T>
> inline MOZ_MUST_USE T*
> ShiftFromList(JSContext* cx, HandleNativeObject list)
> {
> uint32_t length = list->getDenseInitializedLength();
> MOZ_ASSERT(length > 0);
>
> Rooted<T*> entry(cx, &list->getDenseElement(0).toObject().as<T>());
> if (!list->tryShiftDenseElements(1)) {
> list->moveDenseElements(0, 1, length - 1);
> list->shrinkElements(cx, length - 1);
> }
>
> list->setDenseInitializedLength(length - 1);
>
> return entry;
> }
https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/js/src/vm/NativeObject.cpp#1038
> void
> NativeObject::shrinkElements(JSContext* cx, uint32_t reqCapacity)
> {
> ...
> HeapSlot* oldHeaderSlots = reinterpret_cast<HeapSlot*>(getUnshiftedElementsHeader());
> HeapSlot* newHeaderSlots = ReallocateObjectBuffer<HeapSlot>(cx, this, oldHeaderSlots,
> oldAllocated, newAllocated);
> if (!newHeaderSlots) {
> cx->recoverFromOutOfMemory();
> return; // Leave elements at its old size.
> }
>
> ObjectElements* newheader = reinterpret_cast<ObjectElements*>(newHeaderSlots);
> elements_ = newheader->elements() + numShifted;
> getElementsHeader()->capacity = newCapacity;
there:
oldAllocated = 16
oldCapacity = 8
newAllocated = 8
newCapacity = 0
numShifted = 6
so, elements_ points the element after the last allocated element.
then, inside setDenseInitializedLength we try to pre-barrier on the 0-th elements, that is out-of-bound.
https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/js/src/vm/NativeObject.h#1004
> void prepareElementRangeForOverwrite(size_t start, size_t end) {
> MOZ_ASSERT(end <= getDenseInitializedLength());
> MOZ_ASSERT(!denseElementsAreCopyOnWrite());
> for (size_t i = start; i < end; i++)
> elements_[i].HeapSlot::~HeapSlot();
> }
so, I think we should do pre-barrier and resetting initializedLength inside shrinkElements, maybe?
jandem, I think you know better about the shift optimization.
what's the expected behavior for this case?
Flags: needinfo?(arai.unmht) → needinfo?(jdemooij)
Assignee | ||
Comment 6•7 years ago
|
||
Sure I can take a look at this tomorrow.
Comment 7•7 years ago
|
||
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #0)
> Created attachment 8926854 [details]
> clusterfuzz-testcase-minimized-4564957816422400.js
>
> This bug was found by Google's OSS-Fuzz running their custom internal JS
> fuzzer. I am refiling it in our issue tracker.
Did they notify you of it? I'm wondering if we're going to be funneled more bugs this way now.
Updated•7 years ago
|
Keywords: csectype-bounds,
sec-high
Assignee | ||
Comment 8•7 years ago
|
||
This is unrelated to the shift optimization. The problem is that we do:
list->shrinkElements(cx, length - 1);
..
list->setDenseInitializedLength(length - 1);
shrinkElements reallocates the elements in memory and sets capacity to |length - 1|. Then setDenseInitializedLength may invoke a GC pre-barrier on the last element but that's now uninitialized memory because it's outside our allocated capacity.
To catch similar issues in the future I added this assert to shrinkElements:
MOZ_ASSERT(reqCapacity >= getDenseInitializedLength());
That assert fails on many async-generator shell tests without the fix.
Also, tryShiftDenseElements updates the initialized length, so I moved setDenseInitializedLength inside the if-statement and added an assert at the end of the function.
Assignee: arai.unmht → jdemooij
Flags: needinfo?(jdemooij)
Attachment #8927290 -
Flags: review?(arai.unmht)
Comment 9•7 years ago
|
||
Comment on attachment 8927290 [details] [diff] [review]
Patch
Review of attachment 8927290 [details] [diff] [review]:
-----------------------------------------------------------------
ah, thanks, makes sense :)
Attachment #8927290 -
Flags: review?(arai.unmht) → review+
Assignee | ||
Updated•7 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → affected
status-firefox-esr52:
--- → unaffected
tracking-firefox57:
--- → ?
tracking-firefox58:
--- → ?
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8927290 [details] [diff] [review]
Patch
[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
An attacker can invoke a GC pre-barrier on uninitialized memory. Not easy to exploit but it's possible.
> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
No.
> Which older supported branches are affected by this flaw?
56+.
> If not all supported branches, which bug introduced the flaw?
Bug 1272697 added the code (for ReadableStream). Since bug 1390082 we use this code for async generators as well.
> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Should apply or be easy to backport.
> How likely is this patch to cause regressions; how much testing does it need?
Unlikely.
Attachment #8927290 -
Flags: sec-approval?
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #10)
> An attacker can invoke a GC pre-barrier on uninitialized memory.
Hm I just realized there's another way to attack this: because we invoke the pre-barrier on uninitialized memory, it means we *don't* barrier/trace the object we *should have* barriered. So this is also a GC hazard.
Comment 12•7 years ago
|
||
Too late for 57.
Comment 13•7 years ago
|
||
Sec-approval+ for checkin on November 28, two weeks into the new cycle. Please do not check in before that date.
We'll want this on Beta so a Beta patch should be made and nominated at that time as well.
Updated•7 years ago
|
Attachment #8927290 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 15•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Whiteboard: [checkin on 11/28]
Assignee | ||
Comment 16•7 years ago
|
||
Comment on attachment 8927290 [details] [diff] [review]
Patch
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1272697 added the code (for ReadableStream). Since bug 1390082 we use this code for async generators as well.
[User impact if declined]: Security issues.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Local/minor fix.
[String changes made/needed]: None.
Attachment #8927290 -
Flags: approval-mozilla-beta?
Comment 17•7 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•7 years ago
|
Attachment #8927290 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•7 years ago
|
||
uplift |
Updated•7 years ago
|
Group: javascript-core-security → core-security-release
Comment 19•7 years ago
|
||
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #0)
> Created attachment 8926854 [details]
> clusterfuzz-testcase-minimized-4564957816422400.js
>
> This bug was found by Google's OSS-Fuzz running their custom internal JS
> fuzzer. I am refiling it in our issue tracker.
>
> Please note that they apply a 90-day disclose timeline to all bugs:
I need the name of the reporter and/or who to credit for advisory since it isn't you and this is a third party report. Can you supply this?
Flags: needinfo?(agaynor)
Whiteboard: [adv-main58+]
Reporter | ||
Comment 20•7 years ago
|
||
Per https://github.com/google/oss-fuzz#process-overview these finds should be credited to OSS-Fuzz.
Flags: needinfo?(agaynor)
Updated•7 years ago
|
Alias: CVE-2018-5094
Updated•7 years ago
|
Flags: qe-verify-
Whiteboard: [adv-main58+] → [adv-main58+][post-critsmash-triage]
Updated•6 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•