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)

Product:
Core
Component:
JavaScript Engine
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- wontfix
firefox57 - wontfix
firefox58 + fixed
firefox59 + fixed

People

(Reporter: Alex_Gaynor, Assigned: jandem)

References

Details

(Keywords: csectype-bounds, oss-fuzz, sec-high, Whiteboard: [adv-main58+][post-critsmash-triage])

Attachments

(2 files)

clusterfuzz-testcase-minimized-4564957816422400.js
1.70 KB, application/x-javascript
Details
Patch
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
Group: core-security → javascript-core-security
Ni arai because async stuff :)
Flags: needinfo?(arai.unmht)
confirmed similar heap-buffer-overflow on macOS here. I'll look into it
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
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();
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"}
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)
Sure I can take a look at this tomorrow.
(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.
Attached patch Patch — — Splinter Review
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 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+
status-firefox56: --- → unaffected
status-firefox57: --- → affected
status-firefox-esr52: --- → unaffected
tracking-firefox57: --- → ?
tracking-firefox58: --- → ?
Priority: -- → P1
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?
(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.
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.
status-firefox59: --- → affected
tracking-firefox59: --- → +
Whiteboard: [checkin on 11/28]
Attachment #8927290 - Flags: sec-approval? → sec-approval+
Whiteboard: [checkin on 11/28]
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?
https://hg.mozilla.org/mozilla-central/rev/2467d71d0e0d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Attachment #8927290 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Group: javascript-core-security → core-security-release
(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+]
Per https://github.com/google/oss-fuzz#process-overview these finds should be credited to OSS-Fuzz.
Flags: needinfo?(agaynor)
Alias: CVE-2018-5094
Flags: qe-verify-
Whiteboard: [adv-main58+] → [adv-main58+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: