AddressSanitizer: heap-buffer-overflow [@ js::jit::CodeGeneratorShared::allocateCache]

VERIFIED FIXED in Firefox 29, Firefox OS v1.2

Status

()

Core
JavaScript Engine: JIT
--
critical
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: decoder, Assigned: nbp)

Tracking

(Blocks: 2 bugs, {csectype-bounds, sec-high})

Trunk
mozilla31
x86_64
Linux
csectype-bounds, sec-high
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox28 wontfix, firefox29+ fixed, firefox30+ fixed, firefox31+ fixed, firefox-esr24+ fixed, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

Details

(Whiteboard: [adv-main29+][adv-esr24.5+])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
The following trace came up while doing OOM fuzzing on mozilla-central revision :


==46451==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000063ec7 at pc 0x8a0820 bp 0x7f53ff5718f0 sp 0x7f53ff5718e8
WRITE of size 32 at 0x611000063ec7 thread T3 (Analysis Helper)
out of memory
out of memory
    #0 0x8a081f in unsigned long js::jit::CodeGeneratorShared::allocateCache<js::jit::NameIC>(js::jit::NameIC const&) js/src/jit/IonCaches.h:127
    #1 0x8789f7 in js::jit::CodeGenerator::generateBody() js/src/jit/CodeGenerator.cpp:3240
    #2 0x89ad50 in js::jit::CodeGenerator::generate() js/src/jit/CodeGenerator.cpp:6235
    #3 0x969923 in js::jit::GenerateCode(js::jit::MIRGenerator*, js::jit::LIRGraph*, js::jit::MacroAssembler*) js/src/jit/Ion.cpp:1598
    #4 0xfc3f8f in js::WorkerThread::handleIonWorkload() js/src/jsworkers.cpp:798
    #5 0xfc3060 in js::WorkerThread::threadLoop() js/src/jsworkers.cpp:1034
    #6 0x10fe1a8 in nspr::Thread::ThreadRoutine(void*) js/src/vm/PosixNSPR.cpp:45
    #7 0x7f5405482e99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308
    #8 0x7f54043763fc in ?? /build/buildd/eglibc-2.15/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:112

0x611000063ec7 is located 7 bytes to the right of 256-byte region [0x611000063dc0,0x611000063ec0)
allocated by thread T3 (Analysis Helper) here:
    #0 0x461116 in __interceptor_realloc _asan_rtl_
    #1 0x8f269a in js_realloc(void*, unsigned long) js/src/opt64asan-oom/js/src/../../dist/include/js/Utility.h:115
    #2 0x8f1cd4 in mozilla::VectorBase<unsigned char, 0ul, js::SystemAllocPolicy, js::Vector<unsigned char, 0ul, js::SystemAllocPolicy> >::appendN(unsigned char const&, unsigned long) js/src/opt64asan-oom/js/src/../../dist/include/mozilla/Vector.h:925
    #3 0x8a0422 in unsigned long js::jit::CodeGeneratorShared::allocateCache<js::jit::NameIC>(js::jit::NameIC const&) js/src/jit/shared/CodeGenerator-shared.h:251
    #4 0x8789f7 in js::jit::CodeGenerator::generateBody() js/src/jit/CodeGenerator.cpp:3240
    #5 0x89ad50 in js::jit::CodeGenerator::generate() js/src/jit/CodeGenerator.cpp:6235
    #6 0x969923 in js::jit::GenerateCode(js::jit::MIRGenerator*, js::jit::LIRGraph*, js::jit::MacroAssembler*) js/src/jit/Ion.cpp:1598
    #7 0xfc3f8f in js::WorkerThread::handleIonWorkload() js/src/jsworkers.cpp:798
    #8 0xfc3060 in js::WorkerThread::threadLoop() js/src/jsworkers.cpp:1034
    #9 0x10fe1a8 in nspr::Thread::ThreadRoutine(void*) js/src/vm/PosixNSPR.cpp:45
    #10 0x7f5405482e99 in start_thread /build/buildd/eglibc-2.15/nptl/pthread_create.c:308

Thread T3 (Analysis Helper) created by T0 here:
    #0 0x424c06 in __interceptor_pthread_create _asan_rtl_
    #1 0x10fe427 in PR_CreateThread(PRThreadType, void (*)(void*), void*, PRThreadPriority, PRThreadScope, PRThreadState, unsigned int) js/src/vm/PosixNSPR.cpp:108
    #2 0xfbbe15 in js::GlobalWorkerThreadState::ensureInitialized() js/src/jsworkers.cpp:437
    #3 0xfc4e28 in js::EnsureWorkerThreadsInitialized(js::ExclusiveContext*) js/src/jsworkers.cpp:44
    #4 0xea90f3 in js::ScriptSource::setSourceCopy(js::ExclusiveContext*, char16_t const*, unsigned int, bool, js::SourceCompressionTask*) js/src/jsscript.cpp:1611
    #5 0x549af8 in js::frontend::CompileScript(js::ExclusiveContext*, js::LifoAlloc*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::ReadOnlyCompileOptions const&, char16_t const*, unsigned long, JSString*, unsigned int, js::SourceCompressionTask*) js/src/frontend/BytecodeCompiler.cpp:225
    #6 0xcdd2e5 in JS::Compile(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, char16_t const*, unsigned long) js/src/jsapi.cpp:4470
    #7 0xcdd854 in mozilla::VectorBase<char, 8ul, js::TempAllocPolicy, js::Vector<char, 8ul, js::TempAllocPolicy> >::begin() js/src/jsapi.cpp:4485
    #8 0x486317 in RunFile(JSContext*, JS::Handle<JSObject*>, char const*, _IO_FILE*, bool) js/src/shell/js.cpp:447
    #9 0x47ea00 in ProcessArgs(JSContext*, JSObject*, js::cli::OptionParser*) js/src/shell/js.cpp:5732
    #10 0x7f54042a376c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226

SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
  0x0c2280004780: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c2280004790: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800047a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800047b0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c22800047c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c22800047d0: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
  0x0c22800047e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c22800047f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2280004800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2280004810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c2280004820: 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
==46451==ABORTING


No test is available for this issue, reproducing it only yields an intended OOM crash. Thread scheduling is probably the reason that it doesn't reproduce properly.
(Assignee)

Comment 1

3 years ago
Created attachment 8402790 [details] [diff] [review]
OOM in allocateCache

All OOM flags are well propagated, but we do not check the oom flag before doing a placement new on the reserved memory allocated with the SystemAllocator.

This patch check the oom flags and return an error code which always flow into addCache, which is instrumented to check this error code and make the compilation fail in case of OOM.

Such issue is extremely hard to reproduce as this is used for Ion compilations only, which is run on an helper thread.  It might be easier to reproduce with --ion-parallel-compile=off.
(Assignee)

Updated

3 years ago
Attachment #8402790 - Flags: review?(efaustbmo)
(Assignee)

Comment 2

3 years ago
This issue is coming from Bug 814823.
status-b2g18: --- → unaffected
status-b2g-v1.1hd: --- → ?
status-b2g-v1.2: --- → affected
status-b2g-v1.3: --- → affected
status-b2g-v1.3T: --- → affected
status-b2g-v1.4: --- → affected
status-b2g-v2.0: --- → affected
status-firefox28: --- → affected
status-firefox29: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → affected
status-firefox-esr24: --- → affected

Comment 3

3 years ago
Comment on attachment 8402790 [details] [diff] [review]
OOM in allocateCache

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

Yep. Oops. r=me
Attachment #8402790 - Flags: review?(efaustbmo) → review+
(Assignee)

Comment 4

3 years ago
Comment on attachment 8402790 [details] [diff] [review]
OOM in allocateCache

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

 - On a multiple core computer, this is hard to reproduce as this involve doing an OOM in a separated thread which is not controlled by us.

 - On a Single core device, such as some low end phones.  This is more likely to be exploitable, but still hard to use for an exploit as the content of the memory written is hard to control from an attacker perspective (register allocation)

 Note that OOM are still handled and prevent using these data with executable flags.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

The patch it-self highlight that we did not handled OOMs.

> Which older supported branches are affected by this flaw?

All except b2g18.

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

Bug 814823

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

I do not have backports yet.
This should be trivial to make the same fixes, as the patch would not apply because of modification of near-by lines.

> How likely is this patch to cause regressions; how much testing does it need?

Unlikely.
Attachment #8402790 - Flags: sec-approval?
Comment on attachment 8402790 [details] [diff] [review]
OOM in allocateCache

sec-approval+. We should get this on Aurora, Beta, and ESR24. Please make and nominate patches ASAP as we're running out of time.
Attachment #8402790 - Flags: sec-approval? → sec-approval+
(Assignee)

Comment 6

3 years ago
Comment on attachment 8402790 [details] [diff] [review]
OOM in allocateCache

[Approval Request Comment]
(see comment 4)
String or IDL/UUID changes made by this patch:  N/A
Attachment #8402790 - Flags: approval-mozilla-release?
Attachment #8402790 - Flags: approval-mozilla-beta?
Attachment #8402790 - Flags: approval-mozilla-b2g28?
Attachment #8402790 - Flags: approval-mozilla-b2g26?
Attachment #8402790 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 7

3 years ago
Created attachment 8407481 [details] [diff] [review]
bug992968-esr24.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  sec-high
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
  (see comment 4)
String or UUID changes made by this patch:
  N/A

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8407481 - Flags: review+
Attachment #8407481 - Flags: approval-mozilla-esr24?
(Assignee)

Comment 8

3 years ago
(In reply to Al Billings [:abillings] from comment #5)
> sec-approval+. We should get this on Aurora, Beta, and ESR24. Please make
> and nominate patches ASAP as we're running out of time.

I am not sure to understand, should I land the patch right now, and wait for approvals on other branches or wait for approval of all branches before landing on inbound?
Comment on attachment 8402790 [details] [diff] [review]
OOM in allocateCache

Land on inbound now. This won't be uplifted to the release branches until it's on trunk.
Attachment #8402790 - Flags: approval-mozilla-release?
Attachment #8402790 - Flags: approval-mozilla-b2g28?
Attachment #8402790 - Flags: approval-mozilla-b2g26?
(Assignee)

Comment 10

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a12d31c2289
status-firefox28: affected → wontfix
tracking-firefox29: --- → +
tracking-firefox30: --- → +
tracking-firefox31: --- → +
tracking-firefox-esr24: --- → +
Attachment #8402790 - Flags: approval-mozilla-beta?
Attachment #8402790 - Flags: approval-mozilla-beta+
Attachment #8402790 - Flags: approval-mozilla-aurora?
Attachment #8402790 - Flags: approval-mozilla-aurora+
Attachment #8407481 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
https://hg.mozilla.org/mozilla-central/rev/1a12d31c2289
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-v1.1hd: ? → unaffected
status-b2g-v2.0: affected → fixed
status-firefox31: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
https://hg.mozilla.org/releases/mozilla-aurora/rev/8a18196731b1
https://hg.mozilla.org/releases/mozilla-beta/rev/df4f53346f73
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/6f97266da882
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/7c811ea48971
https://hg.mozilla.org/releases/mozilla-esr24/rev/51ede9160d4d
status-b2g-v1.2: affected → fixed
status-b2g-v1.3: affected → fixed
status-b2g-v1.4: affected → fixed
status-firefox29: affected → fixed
status-firefox30: affected → fixed
status-firefox-esr24: affected → fixed
status-b2g-v1.3T: affected → fixed
Whiteboard: [adv-main29+][adv-esr24.5+]
Hi decoder - can you verify that this was fixed before we release? Thanks!
Flags: needinfo?(choller)
(Reporter)

Comment 14

3 years ago
I'm not seeing this anymore popping up, so I assume it's fixed. I cannot verify this though for our release builds, since this is an OOM issue and we don't even have a reliable test.
Status: RESOLVED → VERIFIED
(Reporter)

Updated

3 years ago
Flags: needinfo?(choller)
Group: core-security
You need to log in before you can comment on or make changes to this bug.