1415291 - (CVE-2018-5093) Heap-buffer-overflow READ 8 · js::WasmTableObject::getImpl

Reporter

Description

•

7 years ago

Attached file clusterfuzz-testcase-6215596157960192.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
	
	=================================================================
	==13600==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6110001b4c80 at pc 0x0000028d59ae bp 0x7ffdebc428b0 sp 0x7ffdebc428a8
	READ of size 8 at 0x6110001b4c80 thread T0
	SCARINESS: 23 (8-byte-read-heap-buffer-overflow)
	#0 0x28d59ad in js::WasmTableObject::getImpl(JSContext*, JS::CallArgs const&) mozilla-central/js/src/wasm/WasmJS.cpp:1719:15
	#1 0x28d5ea4 in bool JS::CallNonGenericMethod<&(IsTable(JS::Handle<JS::Value>)), &js::WasmTableObject::getImpl>(JSContext*, JS::CallArgs const&) mozilla-central/js/src/build_DBG.OBJ/dist/include/js/CallNonGenericMethod.h:100:16
	#2 0x28d5ea4 in js::WasmTableObject::get(JSContext*, unsigned int, JS::Value*) mozilla-central/js/src/wasm/WasmJS.cpp:1741
	#3 0x8bb754 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) mozilla-central/js/src/jscntxtinlines.h:291:15
	#4 0x8bb754 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) mozilla-central/js/src/vm/Interpreter.cpp:472
	#5 0x8a5edd in js::CallFromStack(JSContext*, JS::CallArgs const&) mozilla-central/js/src/vm/Interpreter.cpp:527:12
	#6 0x8a5edd in Interpret(JSContext*, js::RunState&) mozilla-central/js/src/vm/Interpreter.cpp:3061
	    #7 0x885088 in js::RunScript(JSContext*, js::RunState&) mozilla-central/js/src/vm/Interpreter.cpp:422:12
	    #8 0x8c10d4 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) mozilla-central/js/src/vm/Interpreter.cpp:705:15
	    #9 0x8c1dc6 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) mozilla-central/js/src/vm/Interpreter.cpp:737:12
	    #10 0x1894d40 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) mozilla-central/js/src/jsapi.cpp:4702:12
	    #11 0x1895267 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) mozilla-central/js/src/jsapi.cpp:4735:12
	    #12 0x5d5d8d in RunFile(JSContext*, char const*, _IO_FILE*, bool) mozilla-central/js/src/shell/js.cpp:695:14
	    #13 0x5d5d8d in Process(JSContext*, char const*, bool, FileKind) mozilla-central/js/src/shell/js.cpp:1048
	    #14 0x56a13f in ProcessArgs(JSContext*, js::cli::OptionParser*) mozilla-central/js/src/shell/js.cpp:8175:14
	    #15 0x56a13f in Shell(JSContext*, js::cli::OptionParser*, char**) mozilla-central/js/src/shell/js.cpp:8547
	    #16 0x56a13f in main mozilla-central/js/src/shell/js.cpp:8960
	    #17 0x7fe5647af82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/libc-start.c:291
	0x6110001b4c81 is located 0 bytes to the right of 1-byte region [0x6110001b4c80,0x6110001b4c81)
	allocated by thread T0 here:
	#0 0x51a0f0 in __interceptor_calloc _asan_rtl_
	#1 0x291ef2f in js_calloc(unsigned long) mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Utility.h:376:12
	#2 0x291ef2f in js::wasm::ExternalTableElem* js_pod_calloc<js::wasm::ExternalTableElem>(unsigned long) mozilla-central/js/src/build_DBG.OBJ/dist/include/js/Utility.h:571
	#3 0x291ef2f in js::wasm::ExternalTableElem* js::MallocProvider<JSContext>::maybe_pod_calloc<js::wasm::ExternalTableElem>(unsigned long) mozilla-central/js/src/vm/MallocProvider.h:62
	#4 0x291ef2f in js::wasm::ExternalTableElem* js::MallocProvider<JSContext>::pod_calloc<js::wasm::ExternalTableElem>(unsigned long) mozilla-central/js/src/vm/MallocProvider.h:132
	#5 0x28d2f4f in js::wasm::Table::create(JSContext*, js::wasm::TableDesc const&, JS::Handle<js::WasmTableObject*>) mozilla-central/js/src/wasm/WasmTable.cpp:52:35
	#6 0x28d2354 in js::WasmTableObject::create(JSContext*, js::wasm::Limits const&) mozilla-central/js/src/wasm/WasmJS.cpp:1619:25
	#7 0x28d3b0e in js::WasmTableObject::construct(JSContext*, unsigned int, JS::Value*) mozilla-central/js/src/wasm/WasmJS.cpp:1674:37
	#8 0x8efb57 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) mozilla-central/js/src/jscntxtinlines.h:291:15
	#9 0x8efb57 in js::CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) mozilla-central/js/src/jscntxtinlines.h:324
	#10 0x8bea61 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) mozilla-central/js/src/vm/Interpreter.cpp:567:20
	#11 0x8a55c1 in Interpret(JSContext*, js::RunState&) mozilla-central/js/src/vm/Interpreter.cpp:3053:18
	    #12 0x885088 in js::RunScript(JSContext*, js::RunState&) mozilla-central/js/src/vm/Interpreter.cpp:422:12
	    #13 0x8c10d4 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) mozilla-central/js/src/vm/Interpreter.cpp:705:15
	    #14 0x8c1dc6 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) mozilla-central/js/src/vm/Interpreter.cpp:737:12
	    #15 0x1894d40 in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) mozilla-central/js/src/jsapi.cpp:4702:12
	    #16 0x1895267 in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) mozilla-central/js/src/jsapi.cpp:4735:12
	    #17 0x5d5d8d in RunFile(JSContext*, char const*, _IO_FILE*, bool) mozilla-central/js/src/shell/js.cpp:695:14
	    #18 0x5d5d8d in Process(JSContext*, char const*, bool, FileKind) mozilla-central/js/src/shell/js.cpp:1048
	    #19 0x56a13f in ProcessArgs(JSContext*, js::cli::OptionParser*) mozilla-central/js/src/shell/js.cpp:8175:14
	    #20 0x56a13f in Shell(JSContext*, js::cli::OptionParser*, char**) mozilla-central/js/src/shell/js.cpp:8547
	    #21 0x56a13f in main mozilla-central/js/src/shell/js.cpp:8960
	    #22 0x7fe5647af82f 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+0x28d59ad)
	Shadow bytes around the buggy address:
	0x0c228002e940: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
	0x0c228002e950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e960: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
	0x0c228002e970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	=>0x0c228002e990:[01]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e9a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e9b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e9c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e9d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
	0x0c228002e9e0: 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
	==13600==ABORTING

Ryan VanderMeulen [:RyanVM]

Updated

•

7 years ago

Group: core-security → javascript-core-security

Christian Holler (:decoder)

Comment 1

•

7 years ago

This one looks like an actual bug. I'm reducing the test right now.

Needinfo for Benjamin due to wasm being involved.

Flags: needinfo?(bbouvier)

Christian Holler (:decoder)

Comment 2

•

7 years ago

Here's the reduced testcase:

Summary: AddressSanitizer: heap-buffer-overflow [@ js::WasmTableObject::getImpl] with READ of size 8
Build version: mozilla-central revision 4e6df5159df3
Build flags: --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --disable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2
Runtime options: --fuzzing-safe

Testcase:

v = new WebAssembly.Table({
  element: "anyfunc"
});
v.get(1);

Backtrace:

==24676==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200001d4c0 at pc 0x000001e9d789 bp 0x7ffe061ef930 sp 0x7ffe061ef928
READ of size 8 at 0x60200001d4c0 thread T0
    #0 0x1e9d788 in js::WasmTableObject::getImpl(JSContext*, JS::CallArgs const&) js/src/wasm/WasmJS.cpp:1719:15
    #1 0x1e9d9d9 in bool JS::CallNonGenericMethod<&(IsTable(JS::Handle<JS::Value>)), &js::WasmTableObject::getImpl>(JSContext*, JS::CallArgs const&) dist/include/js/CallNonGenericMethod.h:100:16
    #2 0x1e9d9d9 in js::WasmTableObject::get(JSContext*, unsigned int, JS::Value*) js/src/wasm/WasmJS.cpp:1741
    #3 0x7fa333 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:291:15
[...]

0x60200001d4c0 is located 15 bytes to the right of 1-byte region [0x60200001d4b0,0x60200001d4b1)
allocated by thread T0 here:
    #0 0x5042ac in __interceptor_calloc compiler-rt/lib/asan/asan_malloc_linux.cc:66
    #1 0x1edeff9 in js_calloc(unsigned long) dist/include/js/Utility.h:376:12
    #2 0x1edeff9 in js::wasm::ExternalTableElem* js_pod_calloc<js::wasm::ExternalTableElem>(unsigned long) dist/include/js/Utility.h:571
    #3 0x1edeff9 in js::wasm::ExternalTableElem* js::MallocProvider<JSContext>::maybe_pod_calloc<js::wasm::ExternalTableElem>(unsigned long) js/src/vm/MallocProvider.h:62
    #4 0x1edeff9 in js::wasm::ExternalTableElem* js::MallocProvider<JSContext>::pod_calloc<js::wasm::ExternalTableElem>(unsigned long) js/src/vm/MallocProvider.h:132
    #5 0x1e9b5ac in js::wasm::Table::create(JSContext*, js::wasm::TableDesc const&, JS::Handle<js::WasmTableObject*>) js/src/wasm/WasmTable.cpp:52:31
    #6 0x1e9afea in js::WasmTableObject::create(JSContext*, js::wasm::Limits const&) js/src/wasm/WasmJS.cpp:1619:25
    #7 0x1e9c4d9 in js::WasmTableObject::construct(JSContext*, unsigned int, JS::Value*) js/src/wasm/WasmJS.cpp:1674:37
    #8 0x7fbb79 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/jscntxtinlines.h:291:15
[...]

SUMMARY: AddressSanitizer: heap-buffer-overflow js/src/wasm/WasmJS.cpp:1719:15 in js::WasmTableObject::getImpl(JSContext*, JS::CallArgs const&)
Shadow bytes around the buggy address:
  0x0c047fffba80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fffba90: fa fa fa fa fa fa 01 fa[fa]fa fd fd fa fa fd fa
  0x0c047fffbaa0: fa fa 07 fa fa fa fd fd fa fa fd fa fa fa 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
  Heap right redzone:      fb
  Freed heap region:       fd

Alex Gaynor [:Alex_Gaynor]

Reporter

Comment 3

•

7 years ago

Basically the same issue exists in |js::wasm::Table::set|, the test case is a bit longer, let me know if you'd like a separate bug filed for it.

Luke Wagner [:luke]

Assignee

Comment 4

•

7 years ago

Oh dear, when ToNonWrappingUint32() was introduced, WasmTableObject::getImpl() started passing table.length()-1 which of course underflows when table.length()=0.  This is pretty bad; we should definitely get this fixed ASAP.

Flags: needinfo?(bbouvier)

Luke Wagner [:luke]

Assignee

Comment 5

•

7 years ago

Attached patch fix-table-len-zero (obsolete) — Details — Splinter Review

Assignee: nobody → luke

Attachment #8926250 - Flags: review?(bbouvier)

Luke Wagner [:luke]

Assignee

Updated

•

7 years ago

tracking-firefox58: --- → ?

Luke Wagner [:luke]

Assignee

Updated

•

7 years ago

status-firefox57: --- → affected

tracking-firefox57: --- → ?

Luke Wagner [:luke]

Assignee

Comment 6

•

7 years ago

Attached patch fix-table-len-zero — Details — Splinter Review

Attachment #8926250 - Attachment is obsolete: true

Attachment #8926250 - Flags: review?(bbouvier)

Attachment #8926252 - Flags: review?(bbouvier)

Benjamin Bouvier [:bbouvier] (inactive)

Comment 7

•

7 years ago

Comment on attachment 8926252 [details] [diff] [review]
fix-table-len-zero

Review of attachment 8926252 [details] [diff] [review]:
-----------------------------------------------------------------

Aouch.

::: js/src/wasm/WasmJS.cpp
@@ +1711,5 @@
> +    if (!ToNonWrappingUint32(cx, v, UINT32_MAX, "Table", noun, index))
> +        return false;
> +
> +    if (*index >= table.length()) {
> +        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_WASM_BAD_UINT32, "Table", noun);

(I'd have suggested to use JSMSG_WASM_OUT_OF_BOUNDS, but this has to be a RangeError, so BAD_UINT32 is more correct)

Attachment #8926252 - Flags: review?(bbouvier) → review+

Ryan VanderMeulen [:RyanVM]

Comment 8

•

7 years ago

Regression from bug 1284156 I guess. Wasm is disabled on ESR52, so setting the remaining flags accordingly.

status-firefox56: --- → wontfix

status-firefox-esr52: --- → disabled

Luke Wagner [:luke]

Assignee

Comment 9

•

7 years ago

Comment on attachment 8926252 [details] [diff] [review]
fix-table-len-zero

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Moderately easy.

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?
release, beta, but not ESR52

If not all supported branches, which bug introduced the flaw?
bug 1284156

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Easy

How likely is this patch to cause regressions; how much testing does it need?
Unlikely, pretty simple

Attachment #8926252 - Flags: sec-approval?

Luke Wagner [:luke]

Assignee

Comment 10

•

7 years ago

Comment on attachment 8926252 [details] [diff] [review]
fix-table-len-zero

Timing is tight with upcoming release, so I thought maybe I'd parallelize these requests.

Approval Request Comment
[Feature/Bug causing the regression]: 1284156
[User impact if declined]: exploitable security bug
[Is this code covered by automated tests?]: privately, but not landed (to avoid painting a target)
[Has the fix been verified in Nightly?]: not yet (due to imminent release)
[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?]: simple patch, only executed by wasm (not even asm.js)
[String changes made/needed]:

Attachment #8926252 - Flags: approval-mozilla-beta?

Al Billings [:abillings - ex-MoCo]

Comment 11

•

7 years ago

sec-approval+ for checkin on 11/28 or immediately after. Do not check this in before then as that is two weeks into the 58 cycle. 

This won't make the 57 release as it is too late for that. We are about to ship.

status-firefox57: affected → wontfix

tracking-firefox57: ? → ---

tracking-firefox58: ? → +

Whiteboard: [checkin on 11/28]

Al Billings [:abillings - ex-MoCo]

Updated

•

7 years ago

Attachment #8926252 - Flags: sec-approval?

Attachment #8926252 - Flags: sec-approval+

Attachment #8926252 - Flags: approval-mozilla-beta?

Jason Orendorff [:jorendorff]

Updated

•

7 years ago

Priority: -- → P1

Daniel Veditz [:dveditz]

Updated

•

7 years ago

Keywords: csectype-bounds, sec-high

Alex Gaynor [:Alex_Gaynor]

Reporter

Comment 12

•

7 years ago

Looks like this may have been (accidentally?) resolved by bug 1418195.

Luke Wagner [:luke]

Assignee

Comment 13

•

7 years ago

Bug 1418195 conflicts with this patch by renaming ToNonWrappingUint32() but it doesn't fix the underlying issue (the `table.length() - 1` is still there).  I'll rebase before landing and post a branch patch.

Luke Wagner [:luke]

Assignee

Comment 14

•

7 years ago

I rebased and landed on m-i (59):
  https://hg.mozilla.org/integration/mozilla-inbound/rev/622c05a8e7a1
The beta branch patch is actually exactly what was reviewed (no rebase necessary).  Is any further approval required to land on 58?

Ryan VanderMeulen [:RyanVM]

Comment 15

•

7 years ago

You still need to do the regular beta approval request, yes.

Whiteboard: [checkin on 11/28]

Luke Wagner [:luke]

Assignee

Comment 16

•

7 years ago

Comment on attachment 8926252 [details] [diff] [review]
fix-table-len-zero

Approval Request Comment
See comment 10.

Attachment #8926252 - Flags: approval-mozilla-beta?

Ryan VanderMeulen [:RyanVM]

Comment 17

•

7 years ago

https://hg.mozilla.org/mozilla-central/rev/622c05a8e7a1

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox59: --- → fixed

tracking-firefox59: --- → ?

Resolution: --- → FIXED

Target Milestone: --- → mozilla59

Julien Cristau [:jcristau]

Updated

•

7 years ago

tracking-firefox59: ? → +

Gerry Chang [:gchang]

Comment 18

•

7 years ago

Comment on attachment 8926252 [details] [diff] [review]
fix-table-len-zero

Fix a sec-high. Beta58+.

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

Ryan VanderMeulen [:RyanVM]

Comment 19

•

7 years ago

uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/70d5016333a4

status-firefox58: affected → fixed

Daniel Veditz [:dveditz]

Updated

•

7 years ago

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

Al Billings [:abillings - ex-MoCo]

Comment 21

•

6 years ago

(In reply to Alex Gaynor [:Alex_Gaynor] from comment #0)
> Created attachment 8926053 [details]
> clusterfuzz-testcase-6215596157960192.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)

Al Billings [:abillings - ex-MoCo]

Updated

•

6 years ago

Whiteboard: [adv-main58+]

Alex Gaynor [:Alex_Gaynor]

Reporter

Comment 22

•

6 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 - ex-MoCo]

Updated

•

6 years ago

Alias: CVE-2018-5093

Matt Wobensmith [:mwobensmith][:matt:]

Updated

•

6 years ago

Flags: qe-verify-

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

Daniel Veditz [:dveditz]

Updated

•

6 years ago

Group: core-security-release

Al Billings [:abillings - ex-MoCo]

Updated

•

5 years ago

Keywords: oss-fuzz

clusterfuzz-testcase-6215596157960192.js 7 years ago Alex Gaynor [:Alex_Gaynor] 115.21 KB, application/x-javascript		Details
fix-table-len-zero 7 years ago Luke Wagner [:luke] 1.45 KB, patch		Details \| Diff \| Splinter Review
fix-table-len-zero 7 years ago Luke Wagner [:luke] 1.45 KB, patch	bbouvier : review+ gchang : approval-mozilla-beta+ abillings : sec-approval+	Details \| Diff \| Splinter Review