1415883 - (CVE-2018-5094) Heap-buffer-overflow READ 8 with async generators

Reporter

Description

•

2 years ago

Posted file clusterfuzz-testcase-minimized-4564957816422400.js — Details

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

Ryan VanderMeulen [:RyanVM]

Updated

•

2 years ago

Group: core-security → javascript-core-security

Jan de Mooij [:jandem]

Assignee

Comment 1

•

2 years ago

Ni arai because async stuff :)

Flags: needinfo?(arai.unmht)

Tooru Fujisawa [:arai]

Comment 2

•

2 years ago

confirmed similar heap-buffer-overflow on macOS here.
I'll look into it

Assignee: nobody → arai.unmht

Status: NEW → ASSIGNED

Tooru Fujisawa [:arai]

Comment 3

•

2 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();

Tooru Fujisawa [:arai]

Comment 4

•

2 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"}

Tooru Fujisawa [:arai]

Comment 5

•

2 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)

Jan de Mooij [:jandem]

Assignee

Comment 6

•

2 years ago

Sure I can take a look at this tomorrow.

Al Billings [:abillings]

Comment 7

•

2 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.

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Keywords: csectype-bounds, sec-high

Jan de Mooij [:jandem]

Assignee

Comment 8

•

2 years ago

Posted patch Patch — Details — 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)

Tooru Fujisawa [:arai]

Comment 9

•

2 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+

Jan de Mooij [:jandem]

Assignee

Updated

•

2 years ago

status-firefox56: --- → unaffected

status-firefox57: --- → affected

status-firefox-esr52: --- → unaffected

tracking-firefox57: --- → ?

tracking-firefox58: --- → ?

Priority: -- → P1

Jan de Mooij [:jandem]

Assignee

Updated

•

2 years ago

status-firefox56: unaffected → affected

Jan de Mooij [:jandem]

Assignee

Comment 10

•

2 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?

Jan de Mooij [:jandem]

Assignee

Comment 11

•

2 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.

Julien Cristau [:jcristau] (back on August 5)

Comment 12

•

2 years ago

Too late for 57.

status-firefox56: affected → wontfix

status-firefox57: affected → wontfix

tracking-firefox57: ? → -

tracking-firefox58: ? → +

Al Billings [:abillings]

Comment 13

•

2 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.

status-firefox59: --- → affected

tracking-firefox59: --- → +

Whiteboard: [checkin on 11/28]

Al Billings [:abillings]

Updated

•

2 years ago

Attachment #8927290 - Flags: sec-approval? → sec-approval+

Jan de Mooij [:jandem]

Assignee

Updated

•

2 years ago

Duplicate of this bug: 1417898

Jan de Mooij [:jandem]

Assignee

Comment 15

•

2 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/2467d71d0e0de20103ce61cdd221461a48e4591b

Jan de Mooij [:jandem]

Assignee

Updated

•

2 years ago

Whiteboard: [checkin on 11/28]

Jan de Mooij [:jandem]

Assignee

Comment 16

•

2 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?

Ryan VanderMeulen [:RyanVM]

Comment 17

•

2 years ago

https://hg.mozilla.org/mozilla-central/rev/2467d71d0e0d

Status: ASSIGNED → RESOLVED

Closed: 2 years ago

status-firefox59: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla59

Al Billings [:abillings]

Updated

•

2 years ago

Attachment #8927290 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Ryan VanderMeulen [:RyanVM]

Comment 18

•

2 years ago

uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/701bfdada717

status-firefox58: affected → fixed

Daniel Veditz [:dveditz]

Updated

•

2 years ago

Group: javascript-core-security → core-security-release

Al Billings [:abillings]

Comment 19

•

2 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+]

Alex Gaynor [:Alex_Gaynor]

Reporter

Comment 20

•

2 years ago

Per https://github.com/google/oss-fuzz#process-overview these finds should be credited to OSS-Fuzz.

Flags: needinfo?(agaynor)

Al Billings [:abillings]

Updated

•

2 years ago

Alias: CVE-2018-5094

Matt Wobensmith [:mwobensmith][:matt:]

Updated

•

2 years ago

Flags: qe-verify-

Whiteboard: [adv-main58+] → [adv-main58+][post-critsmash-triage]

Daniel Veditz [:dveditz]

Updated

•

11 months ago

Group: core-security-release

clusterfuzz-testcase-minimized-4564957816422400.js 2 years ago Alex Gaynor [:Alex_Gaynor] 1.70 KB, application/x-javascript		Details
Patch 2 years ago Jan de Mooij [:jandem] 1.46 KB, patch	arai : review+ abillings : approval-mozilla-beta+ abillings : sec-approval+	Details \| Diff \| Splinter Review