Closed Bug 1286487 Opened 8 years ago Closed 8 years ago

Arbitrary memory access in ConsoleCallData::Trace

Categories

(Core :: DOM: Core & HTML, defect)

50 Branch
x86
All
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla51
Tracking Status
firefox47 --- unaffected
firefox48 --- unaffected
firefox49 --- unaffected
firefox-esr45 --- unaffected
firefox50 + verified
firefox51 + verified

People

(Reporter: loobenyang, Assigned: baku)

References

Details

(Keywords: csectype-uaf, regression, sec-high, Whiteboard: [post-critsmash-triage])

Attachments

(2 files, 1 obsolete file)

Steps to reproduce:
1. Run server side script ArbitraryMemAccess_NoteJSObject_Repro.js in Node.js (node ArbitraryMemAccess_NoteJSObject_Repro.js ).
2. Enter http://localhost:12345 in Firefox browser.
3. Firefox crashes at arbitrary address in ChildFinder::NoteJSObject():


Firefox version: 50.0a1 (2016-07-12)

(52350.545a0): Access violation - code c0000005 (!!! second chance !!!)
eax=231ffff0 ebx=22d43a40 ecx=00fdeb48 edx=11988f6c esi=23103040 edi=00fdeb70
eip=0fb4ea12 esp=00fdeab8 ebp=00fdeabc iopl=0         nv up ei pl nz na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010206
xul!js::gc::IsInsideNursery+0xa [inlined in xul!ChildFinder::NoteJSObject+0x15]:
0fb4ea12 8a00            mov     al,byte ptr [eax]          ds:002b:231ffff0=??
0:000> !analzye -v
No export analzye found
0:000> !analyze -v
*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\SysWOW64\atidxx32.dll - 

FAULTING_IP: 
xul!ChildFinder::NoteJSObject+15 [c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\src\xpcom\base\nscyclecollector.cpp @ 2562]
0fb4ea12 8a00            mov     al,byte ptr [eax]

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 0fb4ea12 (xul!js::gc::IsInsideNursery+0x0000000a)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000000
   Parameter[1]: 231ffff0
Attempt to read from address 231ffff0

CONTEXT:  00000000 -- (.cxr 0x0;r)
eax=231ffff0 ebx=22d43a40 ecx=00fdeb48 edx=11988f6c esi=23103040 edi=00fdeb70
eip=0fb4ea12 esp=00fdeab8 ebp=00fdeabc iopl=0         nv up ei pl nz na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010206
xul!js::gc::IsInsideNursery+0xa [inlined in xul!ChildFinder::NoteJSObject+0x15]:
0fb4ea12 8a00            mov     al,byte ptr [eax]          ds:002b:231ffff0=??

FAULTING_THREAD:  000545a0

DEFAULT_BUCKET_ID:  INVALID_POINTER_READ

PROCESS_NAME:  firefox.exe

ERROR_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%p referenced memory at 0x%p. The memory could not be %s.

EXCEPTION_CODE: (NTSTATUS) 0xc0000005 - The instruction at 0x%p referenced memory at 0x%p. The memory could not be %s.

EXCEPTION_PARAMETER1:  00000000

EXCEPTION_PARAMETER2:  231ffff0

READ_ADDRESS:  231ffff0 

FOLLOWUP_IP: 
xul!ChildFinder::NoteJSObject+15 [c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\src\xpcom\base\nscyclecollector.cpp @ 2562]
0fb4ea12 8a00            mov     al,byte ptr [eax]

NTGLOBALFLAG:  0

APPLICATION_VERIFIER_FLAGS:  0

APP:  firefox.exe

ANALYSIS_VERSION: 6.3.9600.17029 (debuggers(dbg).140219-1702) x86fre

PRIMARY_PROBLEM_CLASS:  INVALID_POINTER_READ

BUGCHECK_STR:  APPLICATION_FAULT_INVALID_POINTER_READ

LAST_CONTROL_TRANSFER:  from 0fb4f4ef to 0fb4ea12

STACK_TEXT:  
00fdeabc 0fb4f4ef 00fdeb70 23103040 00000001 xul!ChildFinder::NoteJSObject+0x15
00fdead4 0fb50ced 23103040 119ae2c4 00fdeb70 xul!nsScriptObjectTracer::NoteJSChild+0x31
00fdeae8 1106aec9 22d43a44 119ae2c4 00fdeb70 xul!TraceCallbackFunc::Trace+0x16
00fdeb08 1106aef4 00fdeb48 00fdeb70 2235f900 xul!mozilla::dom::ConsoleCallData::Trace+0x45
00fdeb28 1106af8b 121665b4 2235f900 00fdeb48 xul!mozilla::dom::Console::cycleCollection::Trace+0x24
00fdeb50 0fc560f0 121665b4 2235f900 00fdeb70 xul!mozilla::dom::Console::cycleCollection::Traverse+0x5e
00fdeb90 0fe15468 0349009c 00fdebac 03447000 xul!nsPurpleBuffer::PurpleBlock::VisitEntries<RemoveSkippableVisitor>+0x2e0
00fdebc8 0fe15644 03490000 03447001 03447000 xul!nsPurpleBuffer::RemoveSkippable+0x47
00fdec04 0fe15715 03447001 03447000 8d37a8d0 xul!nsCycleCollector::ForgetSkippable+0x63
00fdec24 0fe15579 00000016 130e5301 000058a9 xul!nsCycleCollector_forgetSkippable+0x39
00fdec3c 0fe14c76 130e53e0 00fdec90 0fe14b9c xul!FireForgetSkippable+0x2d
00fdec58 0fe333bd 130e53e0 00000000 1cdc1190 xul!CCTimerFired+0xda
00fdece8 0fe332d9 1cdc1190 034066a4 034081d0 xul!nsTimerImpl::Fire+0xd0
00fded24 0fc4a5db 1cdc1190 034451d0 034451c0 xul!nsTimerEvent::Run+0x41
00fdedb4 0fc4896f 034081d0 00000000 00fdede7 xul!nsThread::ProcessNextEvent+0x254
00fdede8 0fdcc392 034730c0 4ff3aecb 034066a0 xul!mozilla::ipc::MessagePump::Run+0x70
00fdee20 0fdcc361 034081d0 00000001 0fc47500 xul!MessageLoop::RunHandler+0x20
00fdee40 0feeabaa 0f2d9d40 00000000 00fdee60 xul!MessageLoop::Run+0x19
00fdee50 0feea939 034066a0 0f2d9d40 00fdee74 xul!nsBaseAppShell::Run+0x34
00fdee60 0feea8ee 034066a0 00fdf1d5 1971a800 xul!nsAppShell::Run+0x26
00fdee74 0ff2a068 0f2d9d40 00fdf0ec 00fdf0d8 xul!nsAppStartup::Run+0x22
00fdf058 0fce67ba 03447110 00fdf204 03444150 xul!XREMain::XRE_mainRun+0x5e7
00fdf07c 0fce6291 00000000 03401050 00fdf204 xul!XREMain::XRE_main+0x16f
00fdf1e0 00e5197c 00000001 03401050 00fdf204 xul!XRE_main+0x39
00fdf47c 00e53be9 01385548 034061c0 00000001 firefox!do_main+0x3cc
00fdf80c 00e55005 00000001 fdf79160 01380358 firefox!wmain+0x449
00fdf858 749d38f4 00c05000 749d38d0 2991ff8d firefox!__scrt_common_main_seh+0xff
00fdf86c 77cb5de3 00c05000 2ae3712c 00000000 KERNEL32!BaseThreadInitThunk+0x24
00fdf8b4 77cb5dae ffffffff 77cdb7cb 00000000 ntdll!__RtlUserThreadStart+0x2f
00fdf8c4 00000000 00e55082 00c05000 00000000 ntdll!_RtlUserThreadStart+0x1b


STACK_COMMAND:  .cxr 0x0 ; kb

FAULTING_SOURCE_LINE:  c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\src\xpcom\base\nscyclecollector.cpp

FAULTING_SOURCE_FILE:  c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\src\xpcom\base\nscyclecollector.cpp

FAULTING_SOURCE_LINE_NUMBER:  2562

SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  xul!ChildFinder::NoteJSObject+15

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: xul

IMAGE_NAME:  xul.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  5784e20c

FAILURE_BUCKET_ID:  INVALID_POINTER_READ_c0000005_xul.dll!ChildFinder::NoteJSObject

BUCKET_ID:  APPLICATION_FAULT_INVALID_POINTER_READ_xul!ChildFinder::NoteJSObject+15

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:invalid_pointer_read_c0000005_xul.dll!childfinder::notejsobject

FAILURE_ID_HASH:  {59ea6cf6-54fb-ff4e-2053-d458d703b3b8}

Followup: MachineOwner
---------
Component: JavaScript: GC → DOM
Summary: Arbitrary memory access in ChildFinder::NoteJSObject() → Arbitrary memory access in ConsoleCallData::Trace
This looks like a regression from bug 1246091.

Andrea, could you take a look at this? I'm not sure if this is a use after free or what.
Blocks: 1246091
Flags: needinfo?(amarchesini)
Keywords: regression
Flags: sec-bounty?
Depends on: 1286615
Flags: needinfo?(amarchesini)
Attached patch crash.patch (obsolete) — Splinter Review
Recently we changed how some runnables are able to run in workers without using WorkerHolder but just changing the BusyCount value. Unfortunately this breaks quite a few things because of:

http://searchfox.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#4561
    if (currentStatus != Running && !HasActiveHolders()) {

This means that we consider the worker "loop" completed also if we have such runnables.

In general, our BusyCount code in worker is a confusing:
1. it doesn't go back to 0 because of 1286615 (ok, this is a regression...)
2. we use BusyCount also for dispatching the CloseEventRunnable (and btw we should get rid of it...)
3. it uses boolean it's unclear when we should use it.

With this patch I tried to clarify a bit of things in this way:

a. ModifyBusyCount doesn't receive a boolean but an enum value. This value can be: eIncreaseBusyCount and eDecreaseBusyCount. For internal use only: eIncreaseForCloseEvent and eDecreaseForCloseEvent.

b. We use BusyCount for everything instead having checks for timers, workerHolder and children.
Assignee: nobody → amarchesini
Attachment #8770678 - Flags: review?(khuey)
Group: core-security → dom-core-security
Comment on attachment 8770678 [details] [diff] [review]
crash.patch

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

This is incredibly verbose.  I'd rather you just made an external API for IncrementBusyCount/DecrementBusyCount.
Attachment #8770678 - Flags: review?(khuey) → review+
Can we release 48 with this issue?
Andrea, what are the security implications of this? Can it cause a use-after-free or a buffer overflow or something else?
Flags: needinfo?(amarchesini)
Flags: sec-bounty? → sec-bounty+
Keywords: sec-high
Attachment #8770453 - Attachment mime type: application/javascript → text/javascript
(In reply to Andrew McCreight [:mccr8] from comment #5)
> Andrea, what are the security implications of this? Can it cause a
> use-after-free or a buffer overflow or something else?

The life-time of a worker is not respected because of some of the latest changes in WorkerRunnables.
This means that a worker can be CCed/GCed also when some runnable is still pending and it will never be executed.
Console keeps an array of such runnables and it traces internal variables inside them. If this happens when the worker is shutting down, we play with invalid pointers. So, I guess, yes.

The bug has been introduced by (me) bug 1282366 landed Mon Jul 04.
Flags: needinfo?(amarchesini)
Thanks for the explanation.

(In reply to Andrea Marchesini (:baku) from comment #6)
> The bug has been introduced by (me) bug 1282366 landed Mon Jul 04.

Great, so only 50 is affected.
Ran the same test case in a customed Linux ASAN build with thread safty assert disabled, I got:

50.0a1 (2016-07-13)


=================================================================
==4441==ERROR: AddressSanitizer: heap-use-after-free on address 0x7f3a1fcfffe8 at pc 0x7f3a4b70157b bp 0x7ffc64547ac0 sp 0x7ffc64547ab8
READ of size 4 at 0x7f3a1fcfffe8 thread T0
    #0 0x7f3a4b70157a in IsInsideNursery /home/coder/OpenSrcCode/firefox/objdir-ff-asan/dist/include/js/HeapAPI.h:343:25
    #1 0x7f3a4b70157a in ObjectIsMarkedGray /home/coder/OpenSrcCode/firefox/objdir-ff-asan/dist/include/js/HeapAPI.h:383
    #2 0x7f3a4b70157a in ChildFinder::NoteJSObject(JSObject*) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:2562
    #3 0x7f3a4b701ca4 in nsScriptObjectTracer::NoteJSChild(JS::GCCellPtr, char const*, void*) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollectorTraceJSHelpers.cpp:33:5
    #4 0x7f3a5298210c in Trace /home/coder/OpenSrcCode/firefox/dom/console/Console.cpp:185:5
    #5 0x7f3a5298210c in mozilla::dom::Console::cycleCollection::Trace(void*, TraceCallbacks const&, void*) /home/coder/OpenSrcCode/firefox/dom/console/Console.cpp:802
    #6 0x7f3a52981e08 in mozilla::dom::Console::cycleCollection::Traverse(void*, nsCycleCollectionTraversalCallback&) /home/coder/OpenSrcCode/firefox/dom/console/Console.cpp:797:3
    #7 0x7f3a4b71f185 in MayHaveChild /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:2579:3
    #8 0x7f3a4b71f185 in RemoveSkippableVisitor::Visit(nsPurpleBuffer&, nsPurpleBufferEntry*) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:2814
    #9 0x7f3a4b702577 in VisitEntries<RemoveSkippableVisitor> /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:1035:11
    #10 0x7f3a4b702577 in VisitEntries<RemoveSkippableVisitor> /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:1062
    #11 0x7f3a4b702577 in nsPurpleBuffer::RemoveSkippable(nsCycleCollector*, bool, bool, void (*)()) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:2835
    #12 0x7f3a4b702dfe in nsCycleCollector::ForgetSkippable(bool, bool) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:2883:3
    #13 0x7f3a4b70af39 in nsCycleCollector_forgetSkippable(bool, bool) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:4113:3
    #14 0x7f3a4eeb9635 in FireForgetSkippable(unsigned int, bool) /home/coder/OpenSrcCode/firefox/dom/base/nsJSEnvironment.cpp:1260:3
    #15 0x7f3a4eebbdf1 in CCTimerFired(nsITimer*, void*) /home/coder/OpenSrcCode/firefox/dom/base/nsJSEnvironment.cpp:1799:7
    #16 0x7f3a4b84d24b in nsTimerImpl::Fire() /home/coder/OpenSrcCode/firefox/xpcom/threads/nsTimerImpl.cpp:524:7
    #17 0x7f3a4b82181b in nsTimerEvent::Run() /home/coder/OpenSrcCode/firefox/xpcom/threads/TimerThread.cpp:286:3
    #18 0x7f3a4b82eacd in nsThread::ProcessNextEvent(bool, bool*) /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:1068:7
    #19 0x7f3a4b8b3b38 in NS_ProcessNextEvent(nsIThread*, bool) /home/coder/OpenSrcCode/firefox/xpcom/glue/nsThreadUtils.cpp:290:10
    #20 0x7f3a4c885b61 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/coder/OpenSrcCode/firefox/ipc/glue/MessagePump.cpp:100:21
    #21 0x7f3a4c77ca18 in RunInternal /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:235:3
    #22 0x7f3a4c77ca18 in RunHandler /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:228
    #23 0x7f3a4c77ca18 in MessageLoop::Run() /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:208
    #24 0x7f3a52b0c59f in nsBaseAppShell::Run() /home/coder/OpenSrcCode/firefox/widget/nsBaseAppShell.cpp:156:3
    #25 0x7f3a54c0db41 in nsAppStartup::Run() /home/coder/OpenSrcCode/firefox/toolkit/components/startup/nsAppStartup.cpp:284:19
    #26 0x7f3a54d6b27d in XREMain::XRE_mainRun() /home/coder/OpenSrcCode/firefox/toolkit/xre/nsAppRunner.cpp:4250:10
    #27 0x7f3a54d6c7b3 in XREMain::XRE_main(int, char**, nsXREAppData const*) /home/coder/OpenSrcCode/firefox/toolkit/xre/nsAppRunner.cpp:4369:8
    #28 0x7f3a54d6d67a in XRE_main /home/coder/OpenSrcCode/firefox/toolkit/xre/nsAppRunner.cpp:4464:16
    #29 0x4fc454 in do_main /home/coder/OpenSrcCode/firefox/browser/app/nsBrowserApp.cpp:251:10
    #30 0x4fc454 in main /home/coder/OpenSrcCode/firefox/browser/app/nsBrowserApp.cpp:387
    #31 0x7f3a65471ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
    #32 0x41cd75 in _start (/home/coder/OpenSrcCode/firefox/objdir-ff-asan/dist/bin/firefox+0x41cd75)

0x7f3a1fcfffe8 is located 67560 bytes inside of 131048-byte region [0x7f3a1fcef800,0x7f3a1fd0f7e8)
freed by thread T48 (DOM Worker) here:
    #0 0x4c4e60 in __interceptor_free /home/coder/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38
    #1 0x7f3a4b712f2b in Clear /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:681:7
    #2 0x7f3a4b712f2b in CCGraph::Clear() /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:860
    #3 0x7f3a4b707075 in nsCycleCollector::CleanupAfterCollection() /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3564:3
    #4 0x7f3a4b7078e8 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3693:9
    #5 0x7f3a4b707464 in nsCycleCollector::ShutdownCollect() /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3607:10
    #6 0x7f3a4b70bd66 in Shutdown /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3899:5
    #7 0x7f3a4b70bd66 in nsCycleCollector_shutdown() /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:4217
    #8 0x7f3a52494dd0 in ~WorkerJSRuntime /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:870:5
    #9 0x7f3a52494dd0 in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:2596
    #10 0x7f3a4b82eacd in nsThread::ProcessNextEvent(bool, bool*) /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:1068:7
    #11 0x7f3a4b8b3b38 in NS_ProcessNextEvent(nsIThread*, bool) /home/coder/OpenSrcCode/firefox/xpcom/glue/nsThreadUtils.cpp:290:10
    #12 0x7f3a4c8871b6 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /home/coder/OpenSrcCode/firefox/ipc/glue/MessagePump.cpp:384:5
    #13 0x7f3a4c77ca18 in RunInternal /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:235:3
    #14 0x7f3a4c77ca18 in RunHandler /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:228
    #15 0x7f3a4c77ca18 in MessageLoop::Run() /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:208
    #16 0x7f3a4b82a0c7 in nsThread::ThreadFunc(void*) /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:463:5
    #17 0x7f3a66809b96 in _pt_root /home/coder/OpenSrcCode/firefox/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #18 0x7f3a66458181 in start_thread /build/buildd/eglibc-2.19/nptl/pthread_create.c:312

previously allocated by thread T48 (DOM Worker) here:
    #0 0x4c5168 in __interceptor_malloc /home/coder/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52
    #1 0x7f3a4b6fdc47 in Add /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:711:52
    #2 0x7f3a4b6fdc47 in CCGraphBuilder::AddNode(void*, nsCycleCollectionParticipant*) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:2224
    #3 0x7f3a4b6f18a5 in mozilla::CycleCollectedJSRuntime::TraverseNativeRoots(nsCycleCollectionNoteRootCallback&) /home/coder/OpenSrcCode/firefox/xpcom/base/CycleCollectedJSRuntime.cpp:778:7
    #4 0x7f3a4b708a45 in TraverseRoots /home/coder/OpenSrcCode/firefox/xpcom/base/CycleCollectedJSRuntime.cpp:1211:3
    #5 0x7f3a4b708a45 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3866
    #6 0x7f3a4b7077b0 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3666:9
    #7 0x7f3a4b707464 in nsCycleCollector::ShutdownCollect() /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3607:10
    #8 0x7f3a4b70bd66 in Shutdown /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:3899:5
    #9 0x7f3a4b70bd66 in nsCycleCollector_shutdown() /home/coder/OpenSrcCode/firefox/xpcom/base/nsCycleCollector.cpp:4217
    #10 0x7f3a52494dd0 in ~WorkerJSRuntime /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:870:5
    #11 0x7f3a52494dd0 in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:2596
    #12 0x7f3a4b82eacd in nsThread::ProcessNextEvent(bool, bool*) /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:1068:7
    #13 0x7f3a4b8b3b38 in NS_ProcessNextEvent(nsIThread*, bool) /home/coder/OpenSrcCode/firefox/xpcom/glue/nsThreadUtils.cpp:290:10
    #14 0x7f3a4c8871b6 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /home/coder/OpenSrcCode/firefox/ipc/glue/MessagePump.cpp:384:5
    #15 0x7f3a4c77ca18 in RunInternal /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:235:3
    #16 0x7f3a4c77ca18 in RunHandler /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:228
    #17 0x7f3a4c77ca18 in MessageLoop::Run() /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:208
    #18 0x7f3a4b82a0c7 in nsThread::ThreadFunc(void*) /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:463:5
    #19 0x7f3a66809b96 in _pt_root /home/coder/OpenSrcCode/firefox/nsprpub/pr/src/pthreads/ptthread.c:216:5
    #20 0x7f3a66458181 in start_thread /build/buildd/eglibc-2.19/nptl/pthread_create.c:312

Thread T48 (DOM Worker) created by T0 here:
    #0 0x42f389 in __interceptor_pthread_create /home/coder/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:243
    #1 0x7f3a66806758 in _PR_CreateThread /home/coder/OpenSrcCode/firefox/nsprpub/pr/src/pthreads/ptthread.c:457:14
    #2 0x7f3a6680636a in PR_CreateThread /home/coder/OpenSrcCode/firefox/nsprpub/pr/src/pthreads/ptthread.c:548:12
    #3 0x7f3a4b82b724 in nsThread::Init() /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:634:8
    #4 0x7f3a5254da37 in mozilla::dom::workers::WorkerThread::Create(mozilla::dom::workers::WorkerThreadFriendKey const&) /home/coder/OpenSrcCode/firefox/dom/workers/WorkerThread.cpp:92:7
    #5 0x7f3a5244cc9b in mozilla::dom::workers::RuntimeService::ScheduleWorker(mozilla::dom::workers::WorkerPrivate*) /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:1613:14
    #6 0x7f3a5244b364 in mozilla::dom::workers::RuntimeService::RegisterWorker(mozilla::dom::workers::WorkerPrivate*) /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:1464:19
    #7 0x7f3a525274bb in mozilla::dom::workers::WorkerPrivate::Constructor(JSContext*, nsAString_internal const&, bool, mozilla::dom::WorkerType, nsACString_internal const&, mozilla::dom::workers::WorkerLoadInfo*, mozilla::ErrorResult&) /home/coder/OpenSrcCode/firefox/dom/workers/WorkerPrivate.cpp:4199:8
    #8 0x7f3a52450e26 in mozilla::dom::workers::RuntimeService::CreateSharedWorkerFromLoadInfo(JSContext*, mozilla::dom::workers::WorkerLoadInfo*, nsAString_internal const&, nsACString_internal const&, mozilla::dom::workers::SharedWorker**) /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:2182:7
    #9 0x7f3a5245082d in mozilla::dom::workers::RuntimeService::CreateSharedWorker(mozilla::dom::GlobalObject const&, nsAString_internal const&, nsACString_internal const&, mozilla::dom::workers::SharedWorker**) /home/coder/OpenSrcCode/firefox/dom/workers/RuntimeService.cpp:2136:10
    #10 0x7f3a524f58da in mozilla::dom::workers::SharedWorker::Constructor(mozilla::dom::GlobalObject const&, JSContext*, nsAString_internal const&, mozilla::dom::Optional<nsAString_internal> const&, mozilla::ErrorResult&) /home/coder/OpenSrcCode/firefox/dom/workers/SharedWorker.cpp:68:17
    #11 0x7f3a500831f1 in mozilla::dom::SharedWorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /home/coder/OpenSrcCode/firefox/objdir-ff-asan/dom/bindings/SharedWorkerBinding.cpp:241:67
    #12 0x7f3a57220277 in CallJSNative /home/coder/OpenSrcCode/firefox/js/src/jscntxtinlines.h:232:15
    #13 0x7f3a57220277 in CallJSNativeConstructor /home/coder/OpenSrcCode/firefox/js/src/jscntxtinlines.h:265
    #14 0x7f3a57220277 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /home/coder/OpenSrcCode/firefox/js/src/vm/Interpreter.cpp:556
    #15 0x7f3a572085bb in ConstructFromStack /home/coder/OpenSrcCode/firefox/js/src/vm/Interpreter.cpp:582:12
    #16 0x7f3a572085bb in Interpret(JSContext*, js::RunState&) /home/coder/OpenSrcCode/firefox/js/src/vm/Interpreter.cpp:2865
    #17 0x7f3a571f06e6 in js::RunScript(JSContext*, js::RunState&) /home/coder/OpenSrcCode/firefox/js/src/vm/Interpreter.cpp:399:12
    #18 0x7f3a57221c0e in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) /home/coder/OpenSrcCode/firefox/js/src/vm/Interpreter.cpp:679:15
    #19 0x7f3a5722232f in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /home/coder/OpenSrcCode/firefox/js/src/vm/Interpreter.cpp:711:12
    #20 0x7f3a56d3396b in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::StaticScope*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /home/coder/OpenSrcCode/firefox/js/src/jsapi.cpp:4405:19
    #21 0x7f3a56d344b1 in Evaluate /home/coder/OpenSrcCode/firefox/js/src/jsapi.cpp:4432:12
    #22 0x7f3a56d344b1 in JS::Evaluate(JSContext*, JS::AutoVectorRooter<JSObject*>&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /home/coder/OpenSrcCode/firefox/js/src/jsapi.cpp:4493
    #23 0x7f3a4eec4ef8 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /home/coder/OpenSrcCode/firefox/dom/base/nsJSUtils.cpp:206:12
    #24 0x7f3a4eec5a09 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /home/coder/OpenSrcCode/firefox/dom/base/nsJSUtils.cpp:266:10
    #25 0x7f3a4ef63ddf in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*) /home/coder/OpenSrcCode/firefox/dom/base/nsScriptLoader.cpp:2010:12
    #26 0x7f3a4ef60ab5 in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*) /home/coder/OpenSrcCode/firefox/dom/base/nsScriptLoader.cpp:1808:10
    #27 0x7f3a4ef4bb49 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) /home/coder/OpenSrcCode/firefox/dom/base/nsScriptLoader.cpp:1546:10
    #28 0x7f3a4ef4892c in nsScriptElement::MaybeProcessScript() /home/coder/OpenSrcCode/firefox/dom/base/nsScriptElement.cpp:141:10
    #29 0x7f3a4e006473 in AttemptToExecute /home/coder/OpenSrcCode/firefox/dom/base/nsIScriptElement.h:221:18
    #30 0x7f3a4e006473 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /home/coder/OpenSrcCode/firefox/parser/html/nsHtml5TreeOpExecutor.cpp:664
    #31 0x7f3a4e004bd1 in nsHtml5TreeOpExecutor::RunFlushLoop() /home/coder/OpenSrcCode/firefox/parser/html/nsHtml5TreeOpExecutor.cpp:488:7
    #32 0x7f3a4e02eefb in nsHtml5ExecutorFlusher::Run() /home/coder/OpenSrcCode/firefox/parser/html/nsHtml5StreamParser.cpp:128:9
    #33 0x7f3a4b82eacd in nsThread::ProcessNextEvent(bool, bool*) /home/coder/OpenSrcCode/firefox/xpcom/threads/nsThread.cpp:1068:7
    #34 0x7f3a4b8b3b38 in NS_ProcessNextEvent(nsIThread*, bool) /home/coder/OpenSrcCode/firefox/xpcom/glue/nsThreadUtils.cpp:290:10
    #35 0x7f3a4c885b61 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/coder/OpenSrcCode/firefox/ipc/glue/MessagePump.cpp:100:21
    #36 0x7f3a4c77ca18 in RunInternal /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:235:3
    #37 0x7f3a4c77ca18 in RunHandler /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:228
    #38 0x7f3a4c77ca18 in MessageLoop::Run() /home/coder/OpenSrcCode/firefox/ipc/chromium/src/base/message_loop.cc:208
    #39 0x7f3a52b0c59f in nsBaseAppShell::Run() /home/coder/OpenSrcCode/firefox/widget/nsBaseAppShell.cpp:156:3
    #40 0x7f3a54c0db41 in nsAppStartup::Run() /home/coder/OpenSrcCode/firefox/toolkit/components/startup/nsAppStartup.cpp:284:19
    #41 0x7f3a54d6b27d in XREMain::XRE_mainRun() /home/coder/OpenSrcCode/firefox/toolkit/xre/nsAppRunner.cpp:4250:10
    #42 0x7f3a54d6c7b3 in XREMain::XRE_main(int, char**, nsXREAppData const*) /home/coder/OpenSrcCode/firefox/toolkit/xre/nsAppRunner.cpp:4369:8
    #43 0x7f3a54d6d67a in XRE_main /home/coder/OpenSrcCode/firefox/toolkit/xre/nsAppRunner.cpp:4464:16
    #44 0x4fc454 in do_main /home/coder/OpenSrcCode/firefox/browser/app/nsBrowserApp.cpp:251:10
    #45 0x4fc454 in main /home/coder/OpenSrcCode/firefox/browser/app/nsBrowserApp.cpp:387
    #46 0x7f3a65471ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287

SUMMARY: AddressSanitizer: heap-use-after-free /home/coder/OpenSrcCode/firefox/objdir-ff-asan/dist/include/js/HeapAPI.h:343:25 in IsInsideNursery
Shadow bytes around the buggy address:
  0x0fe7c3f97fa0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f97fb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f97fc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f97fd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f97fe0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0fe7c3f97ff0: fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd
  0x0fe7c3f98000: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f98010: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f98020: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f98030: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0fe7c3f98040: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
==4441==ABORTING
Tracking 50+ for this sec-high regression.
The fix is not trivial. And I don't know how long it will take: my patch fixes the issue but it introduces hangs in workers because our busy count algorithm is broken in many places. I'm fixing it step by step but I don't know when I'll finish it. Maybe it's 1 day, maybe it's 1 week.

In case we want to fix it urgently, I propose to revoke bug 1282366.

To share more info, the issue is this:

Workers were kept alive in a strange way:
1. for something we use the combination: active timers + sub workers + workerHolder.
2. for something else we use BusyCount.

my approach was to unify everything using busyCount. This is what the patch does. This is a good idea because we increase/decrease the busy count value any time we play with timers in workers, or when a child worker is added/removed and when a WorkerHolder is added/removed (it's not exactly "any" time, but for this description is OK).

The problem is that, sometimes we leak the busyCount decrease operation. and that makes my patch to leak worker threads.

I've already filed bugs to fix some of these leaks: bug 1288031, bug 1288033, bug 1286615.
But something else is missing. Still working on it...
Attached patch console.patchSplinter Review
Let's reintroduce the feature. I'll improve the busycount in separate bugs.
Attachment #8770678 - Attachment is obsolete: true
Attachment #8774034 - Flags: review?(khuey)
Attachment #8774034 - Flags: review?(khuey) → review?(bkelly)
Comment on attachment 8774034 [details] [diff] [review]
console.patch

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

Looks reasonable.  r=me
Attachment #8774034 - Flags: review?(bkelly) → review+
Comment on attachment 8774034 [details] [diff] [review]
console.patch

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

We trace an object after free.

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

Basically we revert 1 patch: no comments are about this crash.

Which older supported branches are affected by this flaw?

aurora.

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

bug 1282366.

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

easy to write a patch.

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

none.
Attachment #8774034 - Flags: sec-approval?
[Tracking Requested - why for this release]: sec-high
Comment on attachment 8774034 [details] [diff] [review]
console.patch

sec-approval+ for trunk. If you nominate a patch for Aurora, I'll get that approved as well.
Attachment #8774034 - Flags: sec-approval? → sec-approval+
Comment on attachment 8774034 [details] [diff] [review]
console.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1282366
[User impact if declined]: A crash can happen
[Describe test coverage new/current, TreeHerder]: no tests.
[Risks and why]: No risks: this patch reverts a changes about keeping alive workers using busy count. But our busycount system is quite buggy. Everything was fine when the worker was kept alive by WorkerHolder, so this patch reverts the code and brings back WorkerHolder for this kind of runnable.
[String/UUID change made/needed]: none.
Attachment #8774034 - Flags: approval-mozilla-aurora?
Attachment #8774034 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/f674ce2f53fd
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Part 2 also needs uplifted, yes?
Flags: needinfo?(amarchesini)
Yes, please.
Flags: needinfo?(amarchesini) → needinfo?(wkocher)
Group: dom-core-security → core-security-release
Flags: qe-verify+
QA Contact: mwobensmith
Whiteboard: [post-critsmash-triage]
Confirmed crash on Fx50 2016-08-10.
Verified fixed on Fx50b11 2016-11-08.
Status: RESOLVED → VERIFIED
Group: core-security-release
I reproduced this crash using Fx 50.0a1 (build ID:20160712030234).
I can confirm this crash is no longer reproducible. I verified using Fx 51.0b2(20161121093909).
Flags: qe-verify+
Version: 48 Branch → 50 Branch
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: