1089328 - Use-After-Free in WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate()

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Looben Yang]

Attachment: wsserver_DispatchPrivate.js

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.36 Safari/537.36

Steps to reproduce:

Run websocket server wsserver_DispatchPrivate.js  with Node.js module websocket, enter http://localhost:12345/ in Firefox browser.

Firefox Version: 36.0a1 (2014-10-25)
Operating System: Windows 8, 64 bit

Test case:
On server side, it accepts web socket at port 12345 with protocol "wsm1-protocol";
On client side, it initiate a web socket to port 12345 with protocol "wsm1-protocol" in web worker, and auto refresh.

Client and server side code have been combined in a single node.js source file wsserver_DispatchPrivate.js.


Actual results:

Firefox crashes  at random addresses in WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate(). The 5a5a5a5a pattern indicates that it may be a Use After Free.

(f9c.8a4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=1149d094 ebx=1b18c000 ecx=1b18c000 edx=0966f580 esi=5a5a5a5a edi=00000000
eip=0f8350c8 esp=07bbeb0c ebp=07bbeb1c iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+0xc:
0f8350c8 ff36            push    dword ptr [esi]      ds:002b:5a5a5a5a=????????
0:008> !analyze -v
*******************************************************************************
*                                                                             *
*                        Exception Analysis                                   *
*                                                                             *
*******************************************************************************

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Windows\SysWOW64\SogouPy.ime - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Windows\SysWOW64\atidxx32.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\360\360Safe\safemon\urlproc.dll - 
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files (x86)\360\360Safe\360NetBase.dll - 

FAULTING_IP: 
xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+c [c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\dom\workers\workerprivate.cpp @ 2104]
0f8350c8 ff36            push    dword ptr [esi]

EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
ExceptionAddress: 0f8350c8 (xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+0x0000000c)
   ExceptionCode: c0000005 (Access violation)
  ExceptionFlags: 00000000
NumberParameters: 2
   Parameter[0]: 00000000
   Parameter[1]: 5a5a5a5a
Attempt to read from address 5a5a5a5a

CONTEXT:  00000000 -- (.cxr 0x0;r)
eax=1149d094 ebx=1b18c000 ecx=1b18c000 edx=0966f580 esi=5a5a5a5a edi=00000000
eip=0f8350c8 esp=07bbeb0c ebp=07bbeb1c iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+0xc:
0f8350c8 ff36            push    dword ptr [esi]      ds:002b:5a5a5a5a=????????

FAULTING_THREAD:  000008a4

PROCESS_NAME:  firefox.exe

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

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

EXCEPTION_PARAMETER1:  00000000

EXCEPTION_PARAMETER2:  5a5a5a5a

READ_ADDRESS:  5a5a5a5a 

FOLLOWUP_IP: 
xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+c [c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\dom\workers\workerprivate.cpp @ 2104]
0f8350c8 ff36            push    dword ptr [esi]

NTGLOBALFLAG:  0

APPLICATION_VERIFIER_FLAGS:  0

APP:  firefox.exe

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

BUGCHECK_STR:  APPLICATION_FAULT_INVALID_POINTER_READ_FILL_PATTERN_5a5a5a5a

PRIMARY_PROBLEM_CLASS:  INVALID_POINTER_READ_FILL_PATTERN_5a5a5a5a

DEFAULT_BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_5a5a5a5a

LAST_CONTROL_TRANSFER:  from 0f6a950c to 0f8350c8

STACK_TEXT:  
07bbeb1c 0f6a950c 0963fa00 00000000 0963fa00 xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+0xc
07bbeb38 0f6a920a 1c7753b8 0963fa00 6d2e5f29 xul!mozilla::dom::workers::WorkerRunnable::DispatchInternal+0x19
07bbeb74 108ac563 00000000 00000000 1aa9c868 xul!mozilla::dom::workers::WorkerRunnable::Dispatch+0xf8
07bbeb88 1037b705 1c7753b8 0966f580 00000000 xul!mozilla::dom::WebSocketImpl::Dispatch+0x67
07bbeba8 1037a60e 00000000 1bb46960 09635bc0 xul!mozilla::net::WebSocketChannel::OnOutputStreamReady+0x185
07bbebc0 1037a7d4 1aa9c9c0 09635e30 1aa9c800 xul!mozilla::net::WebSocketChannel::EnqueueOutgoingMessage+0x4a
07bbebdc 1037cb37 07bbec5e 00000000 00000000 xul!mozilla::net::WebSocketChannel::GeneratePong+0x7b
07bbec30 1037b420 09bbec5c 00000002 029405c0 xul!mozilla::net::WebSocketChannel::ProcessInput+0x65e
07bbf460 0f836bd6 1aa9c864 1bb46940 1c75fa60 xul!mozilla::net::WebSocketChannel::OnInputStreamReady+0x136
07bbf474 0f6eecfc 1c75fa60 029405c0 0294470c xul!nsInputStreamReadyEvent::Run+0x1e
07bbf57c 0f6ee240 029405c0 00000001 07bbf59b xul!nsThread::ProcessNextEvent+0x6f1
07bbf590 0f753a3a 019405c0 00000001 00000000 xul!NS_ProcessNextEvent+0x1b
07bbf5b4 0f6eecfc 0194470c 0294b3c0 029cd610 xul!nsSocketTransportService::Run+0xd1
07bbf6bc 0f6ee240 029405c0 00000000 07bbf6db xul!nsThread::ProcessNextEvent+0x6f1
07bbf6d0 0fb631a1 019405c0 00000000 0294b3c0 xul!NS_ProcessNextEvent+0x1b
07bbf6fc 0fb56c5d 0294b3c0 555a49ca 029405c0 xul!mozilla::ipc::MessagePumpForNonMainThreads::Run+0x7f
07bbf734 0fb56a72 0294b3c0 00000001 02911500 xul!MessageLoop::RunHandler+0x53
07bbf754 0f9ee858 02906350 02911530 02906350 xul!MessageLoop::Run+0x19
07bbf76c 6bce3559 029405c0 00000000 05b941a0 xul!nsThread::ThreadFunc+0x87
07bbf788 6bcd2d9f 02911530 07bbf7cc 6be7c01d nss3!_PR_NativeRunThread+0x149
07bbf794 6be7c01d 02911530 2e0ac485 00000000 nss3!pr_root+0xf
07bbf7cc 6be7c001 00000000 07bbf7e4 75b686e3 MSVCR120!_callthreadstartex+0x1b
07bbf7d8 75b686e3 05b93dd8 07bbf828 778ebe19 MSVCR120!_threadstartex+0x7c
07bbf7e4 778ebe19 05b93dd8 32b89343 00000000 KERNEL32!BaseThreadInitThunk+0xe
07bbf828 778ebdec 6be7bfb4 05b93dd8 ffffffff ntdll!__RtlUserThreadStart+0x72
07bbf840 00000000 6be7bfb4 05b93dd8 00000000 ntdll!_RtlUserThreadStart+0x1b


STACK_COMMAND:  .cxr 0x0 ; kb

FAULTING_SOURCE_LINE:  c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\dom\workers\workerprivate.cpp

FAULTING_SOURCE_FILE:  c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\dom\workers\workerprivate.cpp

FAULTING_SOURCE_LINE_NUMBER:  2104

SYMBOL_STACK_INDEX:  0

SYMBOL_NAME:  xul!mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate+c

FOLLOWUP_NAME:  MachineOwner

MODULE_NAME: xul

IMAGE_NAME:  xul.dll

DEBUG_FLR_IMAGE_TIMESTAMP:  544b8df9

FAILURE_BUCKET_ID:  INVALID_POINTER_READ_FILL_PATTERN_5a5a5a5a_c0000005_xul.dll!mozilla::dom::workers::WorkerPrivateParent_mozilla::dom::workers::WorkerPrivate_::DispatchPrivate

BUCKET_ID:  APPLICATION_FAULT_INVALID_POINTER_READ_FILL_PATTERN_5a5a5a5a_xul!mozilla::dom::workers::WorkerPrivateParent_mozilla::dom::workers::WorkerPrivate_::DispatchPrivate+c

ANALYSIS_SOURCE:  UM

FAILURE_ID_HASH_STRING:  um:invalid_pointer_read_fill_pattern_5a5a5a5a_c0000005_xul.dll!mozilla::dom::workers::workerprivateparent_mozilla::dom::workers::workerprivate_::dispatchprivate

FAILURE_ID_HASH:  {4b223de6-1d8e-0720-cc95-f78d04301aaa}

Followup: MachineOwner
---------




Expected results:

Should not crash.

Comment 1

[Looben Yang]

Attachment: Uaf_DispatchPrivate.dmp

Comment 2

[Looben Yang]

I ran the same reproduction test case with Linux Asan build 36.0a1 (2014-10-27) on Ubuntu 64bit. It did report an use after free, although the call stack is slightly different:


=================================================================
==2850==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000d674d0 at pc 0x7fd43aa1c22a bp 0x7fff68f6a230 sp 0x7fff68f6a228
READ of size 8 at 0x619000d674d0 thread T0 (Web Content)
    #0 0x7fd43aa1c229 in get /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:222
    #1 0x7fd43aa1c229 in operator* /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:282
    #2 0x7fd43aa1c229 in operator mozilla::Mutex & /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerPrivate.h:108
    #3 0x7fd43aa1c229 in mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate(mozilla::dom::workers::WorkerRunnable*, nsIEventTarget*) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerPrivate.cpp:2110
    #4 0x7fd43aa2515d in Dispatch /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerPrivate.h:316
    #5 0x7fd43aa2515d in mozilla::dom::workers::WorkerRunnable::DispatchInternal() /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerRunnable.cpp:124
    #6 0x7fd43aa073bb in mozilla::dom::workers::WorkerRunnable::Dispatch(JSContext*) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerRunnable.cpp:93
    #7 0x7fd438352ab7 in mozilla::dom::WebSocketImpl::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/WebSocket.cpp:2405
    #8 0x7fd4369e5cb4 in mozilla::net::WebSocketChannelChild::RecvOnAcknowledge(unsigned int const&) /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/protocol/websocket/WebSocketChannelChild.cpp:322
    #9 0x7fd4369e602f in non-virtual thunk to mozilla::net::WebSocketChannelChild::RecvOnAcknowledge(unsigned int const&) /builds/slave/m-cen-l64-asan-000000000000000/build/netwerk/protocol/websocket/WebSocketChannelChild.cpp:327
    #10 0x7fd4372f96e0 in mozilla::net::PWebSocketChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/ipc/ipdl/./PWebSocketChild.cpp:436
    #11 0x7fd436e8a8aa in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/ipc/ipdl/./PContentChild.cpp:4421
    #12 0x7fd436c3d1c1 in DispatchAsyncMessage /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessageChannel.cpp:1110
    #13 0x7fd436c3d1c1 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessageChannel.cpp:1050
    #14 0x7fd436c33de5 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessageChannel.cpp:1037
    #15 0x7fd436bf4824 in RunTask /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:361
    #16 0x7fd436bf4824 in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:369
    #17 0x7fd436bf58d7 in MessageLoop::DoWork() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:447
    #18 0x7fd436c445a2 in mozilla::ipc::DoWorkRunnable::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessagePump.cpp:233
    #19 0x7fd4363acf44 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThread.cpp:830
    #20 0x7fd43640bf6a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/glue/nsThreadUtils.cpp:265
    #21 0x7fd436c43d09 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessagePump.cpp:99
    #22 0x7fd436bf33ac in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:233
    #23 0x7fd436bf33ac in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:226
    #24 0x7fd436bf33ac in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:200
    #25 0x7fd43ade1b57 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/widget/nsBaseAppShell.cpp:164
    #26 0x7fd43c8b8812 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/xre/nsEmbedFunctions.cpp:713
    #27 0x7fd436bf33ac in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:233
    #28 0x7fd436bf33ac in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:226
    #29 0x7fd436bf33ac in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:200
    #30 0x7fd43c8b7eaf in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/xre/nsEmbedFunctions.cpp:550
    #31 0x4894cf in content_process_main /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/app/../contentproc/plugin-container.cpp:158
    #32 0x4894cf in main /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/app/MozillaRuntimeMain.cpp:11
    #33 0x7fd433fd1ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #34 0x48930c in _start (/home/parnell/FirefoxBuilds/firefox/plugin-container+0x48930c)

0x619000d674d0 is located 80 bytes inside of 1048-byte region [0x619000d67480,0x619000d67898)
freed by thread T0 (Web Content) here:
    #0 0x471721 in free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7fd4362b689a in SnowWhiteKiller::~SnowWhiteKiller() /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/base/nsCycleCollector.cpp:2624
    #2 0x7fd4362b64a9 in nsCycleCollector::FreeSnowWhite(bool) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/base/nsCycleCollector.cpp:2797
    #3 0x7fd43747c9b9 in AsyncFreeSnowWhite::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/js/xpconnect/src/XPCJSRuntime.cpp:226
    #4 0x7fd4363acf44 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThread.cpp:830
    #5 0x7fd43640bf6a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/glue/nsThreadUtils.cpp:265
    #6 0x7fd436c43d09 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessagePump.cpp:99
    #7 0x7fd436bf33ac in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:233
    #8 0x7fd436bf33ac in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:226
    #9 0x7fd436bf33ac in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:200
    #10 0x7fd43ade1b57 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/widget/nsBaseAppShell.cpp:164
    #11 0x7fd43c8b8812 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/xre/nsEmbedFunctions.cpp:713
    #12 0x7fd436bf33ac in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:233
    #13 0x7fd436bf33ac in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:226
    #14 0x7fd436bf33ac in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:200
    #15 0x7fd43c8b7eaf in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/xre/nsEmbedFunctions.cpp:550
    #16 0x4894cf in content_process_main /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/app/../contentproc/plugin-container.cpp:158
    #17 0x4894cf in main /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/app/MozillaRuntimeMain.cpp:11
    #18 0x7fd433fd1ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

previously allocated by thread T0 (Web Content) here:
    #0 0x471921 in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7fd44245acbd in moz_xmalloc /builds/slave/m-cen-l64-asan-000000000000000/build/memory/mozalloc/mozalloc.cpp:52
    #2 0x7fd43aa036fc in operator new /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/dom/workers/../../dist/include/mozilla/mozalloc.h:208
    #3 0x7fd43aa036fc in mozilla::dom::workers::WorkerPrivate::Constructor(JSContext*, nsAString_internal const&, bool, mozilla::dom::workers::WorkerType, nsACString_internal const&, mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::LoadInfo*, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerPrivate.cpp:3745
    #4 0x7fd43aa03106 in Constructor /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerPrivate.cpp:3685
    #5 0x7fd43aa03106 in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/workers/WorkerPrivate.cpp:3626
    #6 0x7fd439a373eb in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/dom/bindings/./WorkerBinding.cpp:708
    #7 0x7fd43e7395d9 in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/jscntxtinlines.h:231
    #8 0x7fd43e7395d9 in CallJSNativeConstructor /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/jscntxtinlines.h:264
    #9 0x7fd43e7395d9 in js::InvokeConstructor(JSContext*, JS::CallArgs) /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/vm/Interpreter.cpp:579
    #10 0x7fd43e72bc75 in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/vm/Interpreter.cpp:2534
    #11 0x7fd43e70fe7c in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/vm/Interpreter.cpp:432
    #12 0x7fd43e6d8fef in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/vm/Interpreter.cpp:638
    #13 0x7fd43e73a794 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/vm/Interpreter.cpp:674
    #14 0x7fd43e3ab00d in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/js/src/jsapi.cpp:4794
    #15 0x7fd43859cd1d in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsJSUtils.cpp:246
    #16 0x7fd43859dbc0 in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsJSUtils.cpp:312
    #17 0x7fd43861cb51 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsScriptLoader.cpp:1127
    #18 0x7fd43861a25e in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsScriptLoader.cpp:960
    #19 0x7fd438614363 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsScriptLoader.cpp:773
    #20 0x7fd43860ff0e in nsScriptElement::MaybeProcessScript() /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsScriptElement.cpp:140
    #21 0x7fd437af0964 in operator-> /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/nsIScriptElement.h:220
    #22 0x7fd437af0964 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/slave/m-cen-l64-asan-000000000000000/build/parser/html/nsHtml5TreeOpExecutor.cpp:652
    #23 0x7fd437aeecd7 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/slave/m-cen-l64-asan-000000000000000/build/parser/html/nsHtml5TreeOpExecutor.cpp:481
    #24 0x7fd437af578b in nsHtml5ExecutorFlusher::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/parser/html/nsHtml5StreamParser.cpp:126
    #25 0x7fd4363acf44 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/threads/nsThread.cpp:830
    #26 0x7fd43640bf6a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/xpcom/glue/nsThreadUtils.cpp:265
    #27 0x7fd436c43d09 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/glue/MessagePump.cpp:99
    #28 0x7fd436bf33ac in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:233
    #29 0x7fd436bf33ac in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:226
    #30 0x7fd436bf33ac in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:200
    #31 0x7fd43ade1b57 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/widget/nsBaseAppShell.cpp:164
    #32 0x7fd43c8b8812 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/xre/nsEmbedFunctions.cpp:713
    #33 0x7fd436bf33ac in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:233
    #34 0x7fd436bf33ac in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:226
    #35 0x7fd436bf33ac in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/chromium/src/base/message_loop.cc:200
    #36 0x7fd43c8b7eaf in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/toolkit/xre/nsEmbedFunctions.cpp:550
    #37 0x4894cf in content_process_main /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/app/../contentproc/plugin-container.cpp:158
    #38 0x4894cf in main /builds/slave/m-cen-l64-asan-000000000000000/build/ipc/app/MozillaRuntimeMain.cpp:11
    #39 0x7fd433fd1ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-000000000000000/build/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:222 get
Shadow bytes around the buggy address:
  0x0c32801a4e40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4e50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4e60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4e70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32801a4e80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c32801a4e90: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
  0x0c32801a4ea0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4eb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4ec0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4ed0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32801a4ee0: 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 redzon==2850==ABORTING

Comment 3

[Andrea Marchesini [:baku]]

Attachment: bug.patch

Smaug, I don't like the fact that we have a check like: if (!mWorkerFeature)... because that could be racy, but I haven't found anything better.

The issue here is that the worker goes away before we dispatch the event. In order to prevent that we must have a feature to keep alive the worker since the creation of WebSocket. Plus, we should not dispatch event to a worker if we don't have an active feature (and here the issue with the check).

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8512724 [details] [diff] [review]
bug.patch

>+  // It can be that the worker has been terminated. In this case we have to
>+  // ignore this dispatching.
>+  if (!mWorkerFeature) {
>+    return NS_ERROR_FAILURE;
>+  }
>+
>   // If the target is a worker, we have to use a custom WorkerRunnableDispatcher
>   // runnable.
>   nsRefPtr<WorkerRunnableDispatcher> event =
>     new WorkerRunnableDispatcher(mWorkerPrivate, aEvent);

I'm not happy with this change, since this seems to use variable mX to protect using deleted object mY.
(correct me if I'm wrong)
We should never have mY pointing to a deleted object. Anything else is just too error prone.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: bug.patch

Comment 6

[Andrea Marchesini [:baku]]

Attachment: bug.patch

Small issue about locking in WorkerPrivate.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8512855 [details] [diff] [review]
bug.patch

># HG changeset patch
># Parent 8e1d87b5d02786aa2be4369ef527241b0dcdafcb
># User Andrea Marchesini <[e-mail]>
>
>diff --git a/dom/base/WebSocket.cpp b/dom/base/WebSocket.cpp
>--- a/dom/base/WebSocket.cpp
>+++ b/dom/base/WebSocket.cpp
>@@ -81,16 +81,19 @@ public:
>   , mOnCloseScheduled(false)
>   , mFailed(false)
>   , mDisconnected(false)
>   , mCloseEventWasClean(false)
>   , mCloseEventCode(nsIWebSocketChannel::CLOSE_ABNORMAL)
>   , mScriptLine(0)
>   , mInnerWindowID(0)
>   , mWorkerPrivate(nullptr)
>+#ifdef DEBUG
>+  , mHasFeatureRegistered(false)
>+#endif
>   {
>     if (!NS_IsMainThread()) {
>       mWorkerPrivate = GetCurrentThreadWorkerPrivate();
>       MOZ_ASSERT(mWorkerPrivate);
>     }
>   }
So why couldn't we move mWorkerPrivate and mWorkerFeature creation to an Init()?


>-class CallDispatchConnectionCloseEvents MOZ_FINAL : public nsRunnable
>+class CallDispatchConnectionCloseEvents MOZ_FINAL : public nsCancelableRunnable
This change could need an explanation.

>@@ -1028,16 +1051,22 @@ WebSocket::Constructor(const GlobalObjec
>   }
> 
>   // If we don't have a channel, the connection is failed and onerror() will be
>   // called asynchrounsly.
>   if (!webSocket->mImpl->mChannel) {
>     return webSocket.forget();
>   }
> 
>+  if (webSocket->mWorkerPrivate) {
>+    // In workers we have to keep the worker alive using a feature in order to
>+    // dispatch messages correctly.
>+    webSocket->UpdateMustKeepAlive();
>+  }
Ahaa, so here you effectively end up calling Init().
It is just UpdateMustKeepAlive() which creates the feature.
Why we need to map AddRefObject call to mWorkerFeature creation?
Couldn't you just create mWorkerFeature here?

>   nsCOMPtr<nsILoadGroup> loadGroup = do_QueryReferent(mWeakLoadGroup);
>   if (loadGroup) {
>     loadGroup->RemoveRequest(this, nullptr, NS_OK);
>+    loadGroup = nullptr;
>   }
Looks like a useless change.


>   // manually adding loadinfo to the channel since it
>   // was not set during channel creation.
>   nsCOMPtr<nsIDocument> doc = do_QueryReferent(mOriginDocument);
>+  mOriginDocument = nullptr;
No idea how this change is relevant to this bug.

> WebSocketImpl::ReleaseObject()
> {
>   AssertIsOnTargetThread();
> 
>-  if (mWorkerPrivate && !mWorkerFeature) {
>+  if (mWorkerPrivate && mWorkerFeature) {
yikes. How did I miss this earlier.

Comment 8

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]:

Comment 9

[Andrea Marchesini [:baku]]

> Ahaa, so here you effectively end up calling Init().
> It is just UpdateMustKeepAlive() which creates the feature.
> Why we need to map AddRefObject call to mWorkerFeature creation?
> Couldn't you just create mWorkerFeature here?

sure.

> >     loadGroup->RemoveRequest(this, nullptr, NS_OK);
> >+    loadGroup = nullptr;
> >   }
> Looks like a useless change.

Correct. but debugging this issue I found that weakReferences are not thread-safe.
This is the reason why I set it to null here.

> >   nsCOMPtr<nsIDocument> doc = do_QueryReferent(mOriginDocument);
> >+  mOriginDocument = nullptr;
> No idea how this change is relevant to this bug.

...and here.

Comment 10

[Andrea Marchesini [:baku]]

Attachment: bug.patch

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8513081 [details] [diff] [review]
bug.patch

>   nsCOMPtr<nsILoadGroup> loadGroup = do_QueryReferent(mWeakLoadGroup);
>   if (loadGroup) {
>     loadGroup->RemoveRequest(this, nullptr, NS_OK);
>+    loadGroup = nullptr;
>   }
Don't you want mWeakLoadGroup = nullptr;


>   // manually adding loadinfo to the channel since it
>   // was not set during channel creation.
>   nsCOMPtr<nsIDocument> doc = do_QueryReferent(mOriginDocument);
>+  mOriginDocument = nullptr;
loadgroup case and this one could use a comment why we want to set the value to null.

>+++ b/dom/workers/WorkerPrivate.cpp
Changes to this file look ok to me, but bent should look at this carefully.


The use of mutex with mFeatures really needs some comment.

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8513081 [details] [diff] [review]
bug.patch

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

This is just what we discussed, right?

1. Modifications only happen on the worker thread and they are protected with the lock.
2. Reads on non-worker threads are protected with the lock.
3. Reads on the worker thread need not be protected.

Please document this in the header.

Comment 13

[Andrea Marchesini [:baku]]

Attachment: bug.patch

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

there is a test attached.

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

Yes. The patch is also well documented.

Which older supported branches are affected by this flaw?

aurora.

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

none.

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

very easy.

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

The patch is actually quite simple. it protects WorkerPrivate::mFeatures with a lock, and WebSockets use it to keep themselves alive.

Approval Request Comment
[Feature/regressing bug #]: 504553
[User impact if declined]: A crash can happen in websocket in workers
[Describe test coverage new/current, TBPL]: none, hard to reproduce
[Risks and why]: the patch is easy.
[String/UUID change made/needed]: none

Comment 14

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8516854 [details] [diff] [review]
bug.patch

Actually, I'm sorry, I forgot the reason I didn't like this last week, but now I remembered...

We can't allow JS events to fire after the worker has started shutting down. The worker's queue has been cleared, we're not running GC, and there's no iloop protection at this point.

Comment 15

[Andrea Marchesini [:baku]]

Attachment: bug.patch

If we don't want to dispatch onclose events, then we can remove the worker part and keep just the webSocket+worker feature piece.

Comment 16

[Andrea Marchesini [:baku]]

Comment on attachment 8517383 [details] [diff] [review]
bug.patch

You already reviewed this part, but I guess I need a r+ to land just this part of the patch.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #14)
> We can't allow JS events to fire after the worker has started shutting down.
> The worker's queue has been cleared, we're not running GC, and there's no
> iloop protection at this point.

I'm not aware of us having any particular protection for this case in event dispatching code or
DOMEventTargetHelper (which all the EventTargets in workers inherit). Should we have?

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Could you explain why the changes to WorkerPrivate.cpp aren't needed now?

Comment 19

[Andrea Marchesini [:baku]]

The patch, previously was doing 2 things

1. keeping the websocket alive when in workers
2. dispatching the onclose() and all the events in any case also if the worker was shutting down.

The first part, you reviewed it and it's still valid.
The second part was implementing so that the Dispatch() method was keeping mFeatures in count to decide when to dispatch what. If we don't care about dispatching 'onclose' events because comment 14, we can still keep the first part of the patch. In this way the webSocket doesn't crash, and this is the goal of this bug.

Comment 20

[Andrea Marchesini [:baku]]

Comment on attachment 8517383 [details] [diff] [review]
bug.patch

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

there is a test attached.

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

Yes. The patch is also well documented.

Which older supported branches are affected by this flaw?

aurora.

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

none.

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

very easy.

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

The patch is actually quite simple. it keeps WebSockets alive using WorkerFeatures

Approval Request Comment
[Feature/regressing bug #]: 504553
[User impact if declined]: A crash can happen in websocket in workers
[Describe test coverage new/current, TBPL]: none, hard to reproduce
[Risks and why]: the patch is easy.
[String/UUID change made/needed]: none

Comment 21

[Daniel Veditz [:dveditz]]

(In reply to Andrea Marchesini (:baku) from comment #20)
> If not all supported branches, which bug introduced the flaw?
> 
> none.
...
> Approval Request Comment
> [Feature/regressing bug #]: 504553

These two statements contradict: they're worded differently but the two forms are asking the same question. (The redundancy is because sometimes branch-landing bugs aren't security bugs and some security-bugs aren't seeking approval to land on a branch.)

Comment 22

[Daniel Veditz [:dveditz]]

Olli: should I wait for :bent to answer comment 17 before sec-approval, or did :baku's answer in comment 19 suffice?

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

No need wait for bent's answer. It is just about future-proof API implementations in workers.

Comment 24

[Daniel Veditz [:dveditz]]

Comment on attachment 8517383 [details] [diff] [review]
bug.patch

Normally when checking in a security fix we'd prefer a comment that didn't shout "This is a security fix!!!!". Sometimes people leave the comment off altogether and just put a bug number but that's attention-grabbing in it's own way.

In this case since it doesn't affect the majority of our users who are on release we don't need to worry about it.

sec-approval+, a=dveditz for aurora

Comment 25

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Olli Pettay [:smaug] from comment #17)
> I'm not aware of us having any particular protection for this case in event
> dispatching code or
> DOMEventTargetHelper (which all the EventTargets in workers inherit). Should
> we have?

I'm not sure... We clear the worker's event queue entirely when we start closing it down so I don't think we can fire any events after that anyway (except for the special close event, but that has its own logic). But I guess we can't prevent people from writing new code that does something like this websocket event...

Comment 26

[Andrea Marchesini [:baku]]

Attachment: bug.patch

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=efece6a7518c

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cf47ce26ebb

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4cf47ce26ebb

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4f17bbb8fcd3

Can we land a test for this?

Comment 31

[Matt Wobensmith [:mwobensmith][:matt:]]

Marking verified based on https://bugzilla.mozilla.org/show_bug.cgi?id=1090142#c16.

Comment 32

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original crash several times using the steps/poc from comment #0 with the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/10/2014-10-25-03-02-02-mozilla-central/

Used the following to reproduce & test:

- Win 8.1 x64
- nodejs v0.10.35
- npm v1.4.28 (npm install websocket)

Verified using the following build(s):
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-06-03-02-01-mozilla-central/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-06-00-40-07-mozilla-aurora/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/35.0b8-candidates/build1/

I let the testcase from comment #0 run for about 20 minutes on each of the above builds and never received the original crash. I originally reproduced the crash within a few minutes.