Closed Bug 1105194 Opened 5 years ago Closed 5 years ago

Use After Free in DispatchPrivate()

Categories

(Core :: DOM: Workers, defect, critical)

36 Branch
x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox34 --- unaffected
firefox35 + fixed
firefox36 + fixed
firefox37 + fixed
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.2 --- fixed

People

(Reporter: loobenyang, Assigned: baku)

References

Details

(Keywords: csectype-uaf, sec-high, Whiteboard: [reporter-external][b2g-adv-main2.2-])

Attachments

(3 files, 4 obsolete files)

Attached file wsserver_uaf1048.js
Steps to reproduce:

Run websocket server wsserver_uaf1048.js  with Node.js module websocket, enter http://localhost:12345/ in Firefox browser. If it's not reproduced in one minute, just restart it.

Firefox Version: 36.0a1 (2014-11-23)
Operating System: Ubuntu 14.04 LTS 64bit

Test case:
On server side, the same as the websocket server in Bug 1089328 , 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, allocates a lot of strings to trigger GC and expose race condition, and auto refresh.

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


Actual results:

Asan reported Use After Free in DispatchPrivate():

=================================================================
==11546==ERROR: AddressSanitizer: heap-use-after-free on address 0x6190005a64d0 at pc 0x7f86c906da1a bp 0x7fff6a9d2c10 sp 0x7fff6a9d2c08
READ of size 8 at 0x6190005a64d0 thread T0 (Web Content)
    #0 0x7f86c906da19 in get /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:208
    #1 0x7f86c906da19 in operator* /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:268
    #2 0x7f86c906da19 in operator mozilla::Mutex & /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerPrivate.h:110
    #3 0x7f86c906da19 in mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate(mozilla::dom::workers::WorkerRunnable*, nsIEventTarget*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerPrivate.cpp:2111
    #4 0x7f86c907645d in Dispatch /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerPrivate.h:318
    #5 0x7f86c907645d in mozilla::dom::workers::WorkerRunnable::DispatchInternal() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerRunnable.cpp:124
    #6 0x7f86c903924b in mozilla::dom::workers::WorkerRunnable::Dispatch(JSContext*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerRunnable.cpp:93
    #7 0x7f86c6895bf7 in mozilla::dom::WebSocketImpl::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/WebSocket.cpp:2505
    #8 0x7f86c4eecae0 in mozilla::net::WebSocketChannelChild::RecvOnAcknowledge(unsigned int const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/protocol/websocket/WebSocketChannelChild.cpp:353
    #9 0x7f86c4eece8f in non-virtual thunk to mozilla::net::WebSocketChannelChild::RecvOnAcknowledge(unsigned int const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/netwerk/protocol/websocket/WebSocketChannelChild.cpp:358
    #10 0x7f86c5816fc0 in mozilla::net::PWebSocketChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/./PWebSocketChild.cpp:424
    #11 0x7f86c538e7d5 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/ipc/ipdl/./PContentChild.cpp:4630
    #12 0x7f86c5145fe1 in DispatchAsyncMessage /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1126
    #13 0x7f86c5145fe1 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1066
    #14 0x7f86c513be25 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessageChannel.cpp:1053
    #15 0x7f86c50fc654 in RunTask /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:361
    #16 0x7f86c50fc654 in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:369
    #17 0x7f86c50fd707 in MessageLoop::DoWork() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:447
    #18 0x7f86c514d5e2 in mozilla::ipc::DoWorkRunnable::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:233
    #19 0x7f86c48d33a4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #20 0x7f86c49323da in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #21 0x7f86c514cd49 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:99
    #22 0x7f86c50fb1dc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #23 0x7f86c50fb1dc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #24 0x7f86c50fb1dc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #25 0x7f86c947f347 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #26 0x7f86caf8c232 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #27 0x7f86c50fb1dc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #28 0x7f86c50fb1dc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #29 0x7f86c50fb1dc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #30 0x7f86caf8b8cf in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #31 0x4897ca in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:190
    #32 0x7f86c24a9ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #33 0x4895dc in _start (/home/parnell/FirefoxBuilds/firefox/plugin-container+0x4895dc)

0x6190005a64d0 is located 80 bytes inside of 1048-byte region [0x6190005a6480,0x6190005a6898)
freed by thread T0 (Web Content) here:
    #0 0x4719f1 in free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7f86c47db9a7 in SnowWhiteKiller::~SnowWhiteKiller() /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2646
    #2 0x7f86c47db542 in nsCycleCollector::FreeSnowWhite(bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2821
    #3 0x7f86c599bea9 in AsyncFreeSnowWhite::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/xpconnect/src/XPCJSRuntime.cpp:226
    #4 0x7f86c48d33a4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #5 0x7f86c49323da in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #6 0x7f86c514cd49 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:99
    #7 0x7f86c50fb1dc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #8 0x7f86c50fb1dc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #9 0x7f86c50fb1dc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #10 0x7f86c947f347 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #11 0x7f86caf8c232 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #12 0x7f86c50fb1dc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #13 0x7f86c50fb1dc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #14 0x7f86c50fb1dc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #15 0x7f86caf8b8cf in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #16 0x4897ca in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:190
    #17 0x7f86c24a9ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

previously allocated by thread T0 (Web Content) here:
    #0 0x471bf1 in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f86d0c12cbd in moz_xmalloc /builds/slave/m-cen-l64-asan-000000000000000/build/src/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f86c90552ac in operator new /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/mozilla/mozalloc.h:208
    #3 0x7f86c90552ac 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/src/dom/workers/WorkerPrivate.cpp:3874
    #4 0x7f86c9054cb6 in Constructor /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3814
    #5 0x7f86c9054cb6 in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, mozilla::ErrorResult&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3755
    #6 0x7f86c802f1cb in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/bindings/./WorkerBinding.cpp:705
    #7 0x7f86cce62e39 in CallJSNative /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:231
    #8 0x7f86cce62e39 in CallJSNativeConstructor /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jscntxtinlines.h:264
    #9 0x7f86cce62e39 in js::InvokeConstructor(JSContext*, JS::CallArgs) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:579
    #10 0x7f86cce55d3a in Interpret(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:2523
    #11 0x7f86cce399ef in js::RunScript(JSContext*, js::RunState&) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:432
    #12 0x7f86cce00f5f 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/src/js/src/vm/Interpreter.cpp:641
    #13 0x7f86cce63ff4 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/vm/Interpreter.cpp:677
    #14 0x7f86ccac831b in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:4787
    #15 0x7f86ccac8a5e in Evaluate /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:4814
    #16 0x7f86ccac8a5e in JS::Evaluate(JSContext*, JS::AutoObjectVector&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/m-cen-l64-asan-000000000000000/build/src/js/src/jsapi.cpp:4869
    #17 0x7f86c6ab442e 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/src/dom/base/nsJSUtils.cpp:256
    #18 0x7f86c6ab53bb in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsJSUtils.cpp:328
    #19 0x7f86c6b296f4 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsScriptLoader.cpp:1138
    #20 0x7f86c6b26e0e in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*, void**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsScriptLoader.cpp:968
    #21 0x7f86c6b20e67 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsScriptLoader.cpp:781
    #22 0x7f86c6b1c8be in nsScriptElement::MaybeProcessScript() /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsScriptElement.cpp:140
    #23 0x7f86c600ff14 in operator-> /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/nsIScriptElement.h:220
    #24 0x7f86c600ff14 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:663
    #25 0x7f86c600e228 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/slave/m-cen-l64-asan-000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:488
    #26 0x7f86c601504b in nsHtml5ExecutorFlusher::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/parser/html/nsHtml5StreamParser.cpp:127
    #27 0x7f86c48d33a4 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #28 0x7f86c49323da in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #29 0x7f86c514cd49 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/glue/MessagePump.cpp:99
    #30 0x7f86c50fb1dc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #31 0x7f86c50fb1dc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #32 0x7f86c50fb1dc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #33 0x7f86c947f347 in nsBaseAppShell::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #34 0x7f86caf8c232 in XRE_RunAppShell /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #35 0x7f86c50fb1dc in RunInternal /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #36 0x7f86c50fb1dc in RunHandler /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #37 0x7f86c50fb1dc in MessageLoop::Run() /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #38 0x7f86caf8b8cf in XRE_InitChildProcess /builds/slave/m-cen-l64-asan-000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #39 0x4897ca in content_process_main(int, char**) /builds/slave/m-cen-l64-asan-000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:190

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:208 get
Shadow bytes around the buggy address:
  0x0c32800acc40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800acc50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800acc60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800acc70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32800acc80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c32800acc90: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
  0x0c32800acca0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800accb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800accc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800accd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800acce0: 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==11546==ABORTING
[Tracking Requested - why for this release]:
Most probably a regression from bug 504553.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1089328
Thanks Olli and Andrea for the quick response.
However, this bug is not simply a duplicate of Bug 1089328. This bug was found in a new official Asan instrumented build 36.0a1 (2014-11-23) after Bug 1089328  was fixed. I spent nearly one week to get this test case.

We can tell the patch for Bug 1089328 was already in the build from the source line numbers in the call stacks.

Backtrace of Bug 1089328 :
    #7 0x7fd438352ab7 in mozilla::dom::WebSocketImpl::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/dom/base/WebSocket.cpp:2405

Backtrace of this bug:
    #7 0x7f86c6895bf7 in mozilla::dom::WebSocketImpl::Dispatch(nsIRunnable*, unsigned int) /builds/slave/m-cen-l64-asan-000000000000000/build/src/dom/base/WebSocket.cpp:2505
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Flags: needinfo?(amarchesini)
Flags: sec-bounty?
Attached patch crash.patch (obsolete) — Splinter Review
Flags: needinfo?(amarchesini)
Attachment #8529289 - Flags: review?(bugs)
Comment on attachment 8529289 [details] [diff] [review]
crash.patch

We need some way to reproduce the crash and verify this fixes it.
But this makes the code anyway a tad safer and easier to understand.
Attachment #8529289 - Flags: review?(bugs) → review+
Attached patch id.patch (obsolete) — Splinter Review
Attachment #8529298 - Flags: review?(bugs)
Attachment #8529298 - Flags: review?(bugs) → review+
Attached patch crash.patchSplinter Review
patch merged
Attachment #8529289 - Attachment is obsolete: true
Attachment #8529298 - Attachment is obsolete: true
Looben Yang, thanks for your help.
Can you please test this patch? I'm not able to reproduce this issue locally but we must be sure that this bug is fixed properly.
Flags: needinfo?(loobenyang)
(In reply to Andrea Marchesini (:baku) from comment #8)
> Looben Yang, thanks for your help.
> Can you please test this patch? I'm not able to reproduce this issue locally
> but we must be sure that this bug is fixed properly.

This bug is kind of hard to reproduce, usually I need to restart (both websocketserver and Firefox) 10+ times to trigger it.

If you can give me a patched asan build, sure I can try to verify it.
Flags: needinfo?(loobenyang)
Attachment #8528914 - Attachment mime type: application/x-javascript → text/plain
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6227d0364aa9
https://tbpl.mozilla.org/?tree=Try&rev=6227d0364aa9

here 2 links. when the build is ready you can download it. It's for linux 64bits.
I guess it should be fine, right?
Flags: needinfo?(loobenyang)
(In reply to Andrea Marchesini (:baku) from comment #10)
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6227d0364aa9
> https://tbpl.mozilla.org/?tree=Try&rev=6227d0364aa9
> 
> here 2 links. when the build is ready you can download it. It's for linux
> 64bits.
> I guess it should be fine, right?

Yes, will do it tonight after work.
Flags: needinfo?(loobenyang)
Comment on attachment 8529299 [details] [diff] [review]
crash.patch

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

Very hard I guess. I tried for hours with no results. Probably this bug can be reproduced just with an ASAN build.

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

WebSocketChannelChild dispatches events after the disconnection. This is wrong and, when the Worker is kept alive by the webSocket, after disconnect it goes away and no messages can be dispatched after that.

Which older supported branches are affected by this flaw?

aurora

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

bug 504553 shows this issue but the bug seems to be in the WebSocketChannelChild implementation, that is much older than that bug.

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

Easy to backport. No particular risk. We add additional security checks about when a websocket should dispatch messages.

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

This patch doesn't have a mochitest or something else to test the issue.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5adf86af7da9
Attachment #8529299 - Flags: sec-approval?
Is this actually a use-after-free? Before approval, I need to rate this bug for security, especially since it is an external report.
Marking as sec-high after discussion with Dan Veditz.
Keywords: sec-high
Comment on attachment 8529299 [details] [diff] [review]
crash.patch

Sec-approval+ for trunk. Please request approval for Beta so we can fix it in both affected places.
Attachment #8529299 - Flags: sec-approval? → sec-approval+
This patch does not fix this bug.

I performed the following tests:

unpatched build - restart 15 times - reproduced;
patched build   - restart  0 times - reproduced;
patched build   - restart 53 times - reproduced.

The unpatched build is 36.0a1 (2014-11-23);
The patched build is 36.0a1 (2014-11-26), downloaded from /pub/mozilla.org/firefox/try-builds/amarchesini@mozilla.com-6227d0364aa9/try-linux64-asan.

If it's not reproduced in 1 minute or 2, I restarted both the websocket server and the Firefox browser.


The Asan report I got with the patched build:


=================================================================
==4721==ERROR: AddressSanitizer: heap-use-after-free on address 0x6190001ea0d0 at pc 0x7f7c7580b4ba bp 0x7fff9ca5a910 sp 0x7fff9ca5a908
READ of size 8 at 0x6190001ea0d0 thread T0 (Web Content)
    #0 0x7f7c7580b4b9 in get /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:208
    #1 0x7f7c7580b4b9 in operator* /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:268
    #2 0x7f7c7580b4b9 in operator mozilla::Mutex & /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.h:110
    #3 0x7f7c7580b4b9 in mozilla::dom::workers::WorkerPrivateParent<mozilla::dom::workers::WorkerPrivate>::DispatchPrivate(mozilla::dom::workers::WorkerRunnable*, nsIEventTarget*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:2111
    #4 0x7f7c75813efd in Dispatch /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.h:318
    #5 0x7f7c75813efd in mozilla::dom::workers::WorkerRunnable::DispatchInternal() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerRunnable.cpp:124
    #6 0x7f7c757d6ceb in mozilla::dom::workers::WorkerRunnable::Dispatch(JSContext*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerRunnable.cpp:93
    #7 0x7f7c72fed30d in mozilla::dom::WebSocketImpl::Dispatch(nsIRunnable*, unsigned int) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:2515
    #8 0x7f7c716436e0 in mozilla::net::WebSocketChannelChild::RecvOnAcknowledge(unsigned int const&) /builds/slave/try-l64-asan-00000000000000000/build/src/netwerk/protocol/websocket/WebSocketChannelChild.cpp:353
    #9 0x7f7c71643a8f in non-virtual thunk to mozilla::net::WebSocketChannelChild::RecvOnAcknowledge(unsigned int const&) /builds/slave/try-l64-asan-00000000000000000/build/src/netwerk/protocol/websocket/WebSocketChannelChild.cpp:358
    #10 0x7f7c71f6f160 in mozilla::net::PWebSocketChild::OnMessageReceived(IPC::Message const&) /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/ipc/ipdl/./PWebSocketChild.cpp:424
    #11 0x7f7c71ae6975 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/ipc/ipdl/./PContentChild.cpp:4630
    #12 0x7f7c7189cbe1 in DispatchAsyncMessage /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessageChannel.cpp:1126
    #13 0x7f7c7189cbe1 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessageChannel.cpp:1066
    #14 0x7f7c71892a25 in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessageChannel.cpp:1053
    #15 0x7f7c71853254 in RunTask /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:361
    #16 0x7f7c71853254 in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:369
    #17 0x7f7c71854307 in MessageLoop::DoWork() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:447
    #18 0x7f7c718a41e2 in mozilla::ipc::DoWorkRunnable::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessagePump.cpp:233
    #19 0x7f7c7102a234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #20 0x7f7c7108926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #21 0x7f7c718a3928 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessagePump.cpp:140
    #22 0x7f7c71851ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #23 0x7f7c71851ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #24 0x7f7c71851ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #25 0x7f7c75c1cfa7 in nsBaseAppShell::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #26 0x7f7c77735272 in XRE_RunAppShell /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #27 0x7f7c71851ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #28 0x7f7c71851ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #29 0x7f7c71851ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #30 0x7f7c7773490f in XRE_InitChildProcess /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #31 0x48a001 in content_process_main(int, char**) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:216
    #32 0x7f7c6ebf9ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #33 0x489d7c in _start (/home/parnell/FirefoxBuilds/patched/firefox/plugin-container+0x489d7c)

0x6190001ea0d0 is located 80 bytes inside of 1048-byte region [0x6190001ea080,0x6190001ea498)
freed by thread T0 (Web Content) here:
    #0 0x472191 in free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    #1 0x7f7c70f32707 in SnowWhiteKiller::~SnowWhiteKiller() /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2646
    #2 0x7f7c70f322a2 in nsCycleCollector::FreeSnowWhite(bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/base/nsCycleCollector.cpp:2821
    #3 0x7f7c720f3a29 in AsyncFreeSnowWhite::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/js/xpconnect/src/XPCJSRuntime.cpp:226
    #4 0x7f7c7102a234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #5 0x7f7c7108926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #6 0x7f7c718a3949 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessagePump.cpp:99
    #7 0x7f7c71851ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #8 0x7f7c71851ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #9 0x7f7c71851ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #10 0x7f7c75c1cfa7 in nsBaseAppShell::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #11 0x7f7c77735272 in XRE_RunAppShell /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #12 0x7f7c71851ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #13 0x7f7c71851ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #14 0x7f7c71851ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #15 0x7f7c7773490f in XRE_InitChildProcess /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #16 0x48a001 in content_process_main(int, char**) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:216
    #17 0x7f7c6ebf9ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

previously allocated by thread T0 (Web Content) here:
    #0 0x472391 in __interceptor_malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f7c7d3d2cbd in moz_xmalloc /builds/slave/try-l64-asan-00000000000000000/build/src/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f7c757f2d4c in operator new /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/mozilla/mozalloc.h:208
    #3 0x7f7c757f2d4c 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/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3874
    #4 0x7f7c757f2756 in Constructor /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3814
    #5 0x7f7c757f2756 in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, mozilla::ErrorResult&) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3755
    #6 0x7f7c74788d7b in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/bindings/./WorkerBinding.cpp:705
    #7 0x7f7c79611c79 in CallJSNative /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jscntxtinlines.h:231
    #8 0x7f7c79611c79 in CallJSNativeConstructor /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jscntxtinlines.h:264
    #9 0x7f7c79611c79 in js::InvokeConstructor(JSContext*, JS::CallArgs) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:579
    #10 0x7f7c79604b7a in Interpret(JSContext*, js::RunState&) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:2523
    #11 0x7f7c795e882f in js::RunScript(JSContext*, js::RunState&) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:432
    #12 0x7f7c795b0aaf in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:641
    #13 0x7f7c79612e34 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:677
    #14 0x7f7c79278c5b in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4787
    #15 0x7f7c7927936e in Evaluate /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4814
    #16 0x7f7c7927936e in JS::Evaluate(JSContext*, JS::AutoObjectVector&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4869
    #17 0x7f7c7320ecae in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsJSUtils.cpp:256
    #18 0x7f7c7320fc3b in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsJSUtils.cpp:328
    #19 0x7f7c73283f74 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptLoader.cpp:1138
    #20 0x7f7c7328168e in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptLoader.cpp:968
    #21 0x7f7c7327b6e7 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptLoader.cpp:781
    #22 0x7f7c7327713e in nsScriptElement::MaybeProcessScript() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptElement.cpp:140
    #23 0x7f7c72767a94 in operator-> /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsIScriptElement.h:220
    #24 0x7f7c72767a94 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/slave/try-l64-asan-00000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:663
    #25 0x7f7c72765da8 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/slave/try-l64-asan-00000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:488
    #26 0x7f7c7276cbcb in nsHtml5ExecutorFlusher::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/parser/html/nsHtml5StreamParser.cpp:127
    #27 0x7f7c7102a234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #28 0x7f7c7108926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #29 0x7f7c718a3949 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessagePump.cpp:99
    #30 0x7f7c71851ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #31 0x7f7c71851ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #32 0x7f7c71851ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #33 0x7f7c75c1cfa7 in nsBaseAppShell::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #34 0x7f7c77735272 in XRE_RunAppShell /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #35 0x7f7c71851ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #36 0x7f7c71851ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #37 0x7f7c71851ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #38 0x7f7c7773490f in XRE_InitChildProcess /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #39 0x48a001 in content_process_main(int, char**) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:216

SUMMARY: AddressSanitizer: heap-use-after-free /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/workers/../../dist/include/nsRefPtr.h:208 get
Shadow bytes around the buggy address:
  0x0c32800353c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32800353d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32800353e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c32800353f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3280035400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c3280035410: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
  0x0c3280035420: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c3280035430: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c3280035440: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c3280035450: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c3280035460: 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==4721==ABORTING

###!!! [Parent][MessageChannel] Error: Channel error: cannot send/recv
Attached patch patch 2 (obsolete) — Splinter Review
The main issues with this bug is that in workers we have 2 event queues: normal and control. When the worker goes away, we receive a notification and that is dispatched from a control runnable. This means it can run before any other normal runnable, breaking the logic of many methods.

What this patch does is:

1. force a disconnect when the worker goes away. We will not receive any OnStop() message, so we have to disconnect the WebSocket directly.

2. mDisconnecting is used to avoid double Disconnect() calls 1 into the other - for the issue described at the beginning of this comment.

3. ignore any message arrived after a disconnect. We don't need to play with mDisconnecting because all of these methods are called from a normal events.

4. The feature has to be registered before any runnable - in the constructor.

5. Check if the webSocketImpl is gone away after any runnable in the constructor.
Attachment #8529736 - Flags: review?(bugs)
Comment on attachment 8529736 [details] [diff] [review]
patch 2

>@@ -76,25 +76,27 @@ public:
>   NS_DECL_THREADSAFE_ISUPPORTS
>   NS_DECL_NSIEVENTTARGET
> 
>   explicit WebSocketImpl(WebSocket* aWebSocket)
>   : mWebSocket(aWebSocket)
>   , mOnCloseScheduled(false)
>   , mFailed(false)
>   , mDisconnected(false)
>+  , mDisconnecting(false)
I don't understand why we need two flags.
mDisconnectingOrDisconnected should be fine, and just set it true when you set now mDisconnecting true


>+WebSocketImpl::ForceDisconnect()
>+{
>+  AssertIsOnTargetThread();
>+
>+  MOZ_ASSERT(mWorkerShuttingDown);
>+  mWebSocket->UpdateMustKeepAlive();
I don't understand this method call. Disconnect does a DontKeepAliveAnyMore call

>+  Disconnect();
>+}

ForceDisconnect isn't really needed. It could be done in MaybeDisconnect(), right?
Attachment #8529736 - Flags: review?(bugs) → review-
Attached patch patch 2 (obsolete) — Splinter Review
Attachment #8529736 - Attachment is obsolete: true
Attachment #8529901 - Flags: review?(bugs)
Comment on attachment 8529901 [details] [diff] [review]
patch 2

>+  ~MaybeDisconnect()
>+  {
>+    if (!mImpl->mDisconnectingOrDisconnected && mImpl->mWorkerShuttingDown) {
You don't need !mImpl->mDisconnectingOrDisconnected && 

>@@ -1059,16 +1124,22 @@ WebSocket::Constructor(const GlobalObjec
>   nsRefPtr<WebSocketImpl> kungfuDeathGrip = webSocket->mImpl;
> 
>   bool connectionFailed = true;
> 
>   if (NS_IsMainThread()) {
>     webSocket->mImpl->Init(aGlobal.Context(), principal, aUrl, protocolArray,
>                            EmptyCString(), 0, aRv, &connectionFailed);
>   } else {
>+    // In workers we have to keep the worker alive using a feature in order to
>+    // dispatch messages correctly.
>+    if (!webSocket->mImpl->RegisterFeature()) {
>+      return nullptr;
>+    }
You need to set aRv to some error value.
aRv.Throw(NS_ERROR_FAILURE); for example
Attachment #8529901 - Flags: review?(bugs) → review+
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1b8ff63be385
https://tbpl.mozilla.org/?tree=Try&rev=1b8ff63be3ti85

Looben Yang, here a fresh build (still compiling), can you please test it again? Thanks!
Flags: needinfo?(loobenyang)
(In reply to Andrea Marchesini (:baku) from comment #21)
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1b8ff63be385
> https://tbpl.mozilla.org/?tree=Try&rev=1b8ff63be3ti85
> 
> Looben Yang, here a fresh build (still compiling), can you please test it
> again? Thanks!

Sure, will do it by Monday night, cause I may have no spare time in this weekend.
Flags: needinfo?(loobenyang)
Comment on attachment 8529901 [details] [diff] [review]
patch 2

[Security approval request comment]

Read comment 12.

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

This patch adds more security checks about double Disconnect() calls.
I tested this patch for hours plus, the reporter will give us a feedback on Monday.
Attachment #8529901 - Flags: sec-approval?
Comment on attachment 8529299 [details] [diff] [review]
crash.patch

Approval Request Comment
[Feature/regressing bug #]: bug 504553
[User impact if declined]: possible crashes
[Describe test coverage new/current, TBPL]: none. Just locally because this bug is reproducible only with an ASAN build.
[Risks and why]: These 2 patches add more security checks about when the webSocket is disconnected. The only problem I see is that maybe all of these checks are not enough, but I don't see big risks to accept this patch.
[String/UUID change made/needed]:none
Attachment #8529299 - Flags: approval-mozilla-beta?
Attachment #8529299 - Flags: approval-mozilla-aurora?
Run the same test case with this second patched build. I got a crash by null pointer reference:

ASAN:SIGSEGV
=================================================================
==2792==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000a8 (pc 0x7ffd7716e863 sp 0x7ffd4ea4aec0 bp 0x7ffd4ea4b050 T29)
    #0 0x7ffd7716e862 in Lock /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/base/../../dist/include/mozilla/Mutex.h:69
    #1 0x7ffd7716e862 in BaseAutoLock /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/base/../../dist/include/mozilla/Mutex.h:164
    #2 0x7ffd7716e862 in operator-> /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:2084
    #3 0x7ffd7716e862 in mozilla::dom::WebSocketImpl::ConsoleError() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:504
    #4 0x7ffd77174426 in FailConnection /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:523
    #5 0x7ffd77174426 in ~ClearWebSocket /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:1193
    #6 0x7ffd77174426 in mozilla::dom::WebSocket::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, mozilla::dom::Sequence<nsString> const&, mozilla::ErrorResult&) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:1231
    #7 0x7ffd77174ac1 in mozilla::dom::WebSocket::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, nsAString_internal const&, mozilla::ErrorResult&) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/WebSocket.cpp:920
    #8 0x7ffd789845d9 in mozilla::dom::WebSocketBinding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/bindings/./WebSocketBinding.cpp:883
    #9 0x7ffd7d7a2099 in CallJSNative /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jscntxtinlines.h:231
    #10 0x7ffd7d7a2099 in CallJSNativeConstructor /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jscntxtinlines.h:264
    #11 0x7ffd7d7a2099 in js::InvokeConstructor(JSContext*, JS::CallArgs) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:579
    #12 0x7ffd7d794f9a in Interpret(JSContext*, js::RunState&) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:2523
    #13 0x7ffd7d778c4f in js::RunScript(JSContext*, js::RunState&) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:432
    #14 0x7ffd7d740ecf in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:641
    #15 0x7ffd7d7a3254 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:677
    #16 0x7ffd7d40907b in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4787
    #17 0x7ffd7994ac84 in (anonymous namespace)::ScriptExecutorRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/ScriptLoader.cpp:762
    #18 0x7ffd799a54e1 in mozilla::dom::workers::WorkerRunnable::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerRunnable.cpp:326
    #19 0x7ffd751ba234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #20 0x7ffd7521926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #21 0x7ffd7998fc2f in mozilla::dom::workers::WorkerPrivate::RunCurrentSyncLoop() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:5020
    #22 0x7ffd7993b6b9 in Run /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.h:1328
    #23 0x7ffd7993b6b9 in (anonymous namespace)::LoadAllScripts(JSContext*, mozilla::dom::workers::WorkerPrivate*, nsTArray<(anonymous namespace)::ScriptLoadInfo>&, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/ScriptLoader.cpp:836
    #24 0x7ffd7993b233 in mozilla::dom::workers::scriptloader::LoadWorkerScript(JSContext*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/ScriptLoader.cpp:934
    #25 0x7ffd799cf301 in (anonymous namespace)::CompileScriptRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:768
    #26 0x7ffd799a54e1 in mozilla::dom::workers::WorkerRunnable::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerRunnable.cpp:326
    #27 0x7ffd751ba234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #28 0x7ffd7521926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #29 0x7ffd7998745d in mozilla::dom::workers::WorkerPrivate::DoRunLoop(JSContext*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:4242
    #30 0x7ffd7994e1bf in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/RuntimeService.cpp:2828
    #31 0x7ffd751ba234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #32 0x7ffd7521926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #33 0x7ffd75a34928 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessagePump.cpp:368
    #34 0x7ffd759e1ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #35 0x7ffd759e1ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #36 0x7ffd759e1ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #37 0x7ffd751b6cc5 in nsThread::ThreadFunc(void*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:350
    #38 0x7ffd80f20405 in _pt_root /builds/slave/try-l64-asan-00000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:212
    #39 0x7ffd81778181 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8181)
    #40 0x7ffd72e6330c (/lib/x86_64-linux-gnu/libc.so.6+0xfb30c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/base/../../dist/include/mozilla/Mutex.h:69 Lock
Thread T29 (DOM Worker) created by T0 (Web Content) here:
    #0 0x45ec05 in pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:175
    #1 0x7ffd80f1cd8d in _PR_CreateThread /builds/slave/try-l64-asan-00000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:453
    #2 0x7ffd80f1c90a in PR_CreateThread /builds/slave/try-l64-asan-00000000000000000/build/src/nsprpub/pr/src/pthreads/ptthread.c:544
    #3 0x7ffd751b81db in nsThread::Init() /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:455
    #4 0x7ffd7992d83e in mozilla::dom::workers::RuntimeService::WorkerThread::Create() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/RuntimeService.cpp:2602
    #5 0x7ffd7992cc76 in mozilla::dom::workers::RuntimeService::ScheduleWorker(JSContext*, mozilla::dom::workers::WorkerPrivate*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/RuntimeService.cpp:1618
    #6 0x7ffd7992a728 in mozilla::dom::workers::RuntimeService::RegisterWorker(JSContext*, mozilla::dom::workers::WorkerPrivate*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/RuntimeService.cpp:1483
    #7 0x7ffd799831eb 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/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3878
    #8 0x7ffd79982b76 in Constructor /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3814
    #9 0x7ffd79982b76 in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString_internal const&, mozilla::ErrorResult&) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/workers/WorkerPrivate.cpp:3755
    #10 0x7ffd7891919b in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/obj-firefox/dom/bindings/./WorkerBinding.cpp:705
    #11 0x7ffd7d7a2099 in CallJSNative /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jscntxtinlines.h:231
    #12 0x7ffd7d7a2099 in CallJSNativeConstructor /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jscntxtinlines.h:264
    #13 0x7ffd7d7a2099 in js::InvokeConstructor(JSContext*, JS::CallArgs) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:579
    #14 0x7ffd7d794f9a in Interpret(JSContext*, js::RunState&) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:2523
    #15 0x7ffd7d778c4f in js::RunScript(JSContext*, js::RunState&) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:432
    #16 0x7ffd7d740ecf in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:641
    #17 0x7ffd7d7a3254 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/vm/Interpreter.cpp:677
    #18 0x7ffd7d40907b in Evaluate(JSContext*, JS::Handle<JSObject*>, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4787
    #19 0x7ffd7d40978e in Evaluate /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4814
    #20 0x7ffd7d40978e in JS::Evaluate(JSContext*, JS::AutoObjectVector&, JS::ReadOnlyCompileOptions const&, JS::SourceBufferHolder&, JS::MutableHandle<JS::Value>) /builds/slave/try-l64-asan-00000000000000000/build/src/js/src/jsapi.cpp:4869
    #21 0x7ffd7739f0ce in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, nsJSUtils::EvaluateOptions const&, JS::MutableHandle<JS::Value>, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsJSUtils.cpp:256
    #22 0x7ffd773a005b in nsJSUtils::EvaluateString(JSContext*, JS::SourceBufferHolder&, JS::Handle<JSObject*>, JS::CompileOptions&, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsJSUtils.cpp:328
    #23 0x7ffd77414394 in nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, JS::SourceBufferHolder&, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptLoader.cpp:1138
    #24 0x7ffd77411aae in nsScriptLoader::ProcessRequest(nsScriptLoadRequest*, void**) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptLoader.cpp:968
    #25 0x7ffd7740bb07 in nsScriptLoader::ProcessScriptElement(nsIScriptElement*) /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptLoader.cpp:781
    #26 0x7ffd7740755e in nsScriptElement::MaybeProcessScript() /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsScriptElement.cpp:140
    #27 0x7ffd768f7a94 in operator-> /builds/slave/try-l64-asan-00000000000000000/build/src/dom/base/nsIScriptElement.h:220
    #28 0x7ffd768f7a94 in nsHtml5TreeOpExecutor::RunScript(nsIContent*) /builds/slave/try-l64-asan-00000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:663
    #29 0x7ffd768f5da8 in nsHtml5TreeOpExecutor::RunFlushLoop() /builds/slave/try-l64-asan-00000000000000000/build/src/parser/html/nsHtml5TreeOpExecutor.cpp:488
    #30 0x7ffd768fcbcb in nsHtml5ExecutorFlusher::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/parser/html/nsHtml5StreamParser.cpp:127
    #31 0x7ffd751ba234 in nsThread::ProcessNextEvent(bool, bool*) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/threads/nsThread.cpp:830
    #32 0x7ffd7521926a in NS_ProcessNextEvent(nsIThread*, bool) /builds/slave/try-l64-asan-00000000000000000/build/src/xpcom/glue/nsThreadUtils.cpp:265
    #33 0x7ffd75a33949 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/glue/MessagePump.cpp:99
    #34 0x7ffd759e1ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #35 0x7ffd759e1ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #36 0x7ffd759e1ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #37 0x7ffd79dad3c7 in nsBaseAppShell::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/widget/nsBaseAppShell.cpp:164
    #38 0x7ffd7b8c5692 in XRE_RunAppShell /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:731
    #39 0x7ffd759e1ddc in RunInternal /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:233
    #40 0x7ffd759e1ddc in RunHandler /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:226
    #41 0x7ffd759e1ddc in MessageLoop::Run() /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/chromium/src/base/message_loop.cc:200
    #42 0x7ffd7b8c4d2f in XRE_InitChildProcess /builds/slave/try-l64-asan-00000000000000000/build/src/toolkit/xre/nsEmbedFunctions.cpp:568
    #43 0x48a001 in content_process_main(int, char**) /builds/slave/try-l64-asan-00000000000000000/build/src/ipc/app/../contentproc/plugin-container.cpp:216
    #44 0x7ffd72d89ec4 (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)

==2792==ABORTING
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=253826bbd96d
https://tbpl.mozilla.org/?tree=Try&rev=253826bbd96d

Finally I'm able to reproduce this issue. I didn't see any crash with this version of the patch.
Flags: needinfo?(loobenyang)
(In reply to Andrea Marchesini (:baku) from comment #26)
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=253826bbd96d
> https://tbpl.mozilla.org/?tree=Try&rev=253826bbd96d
> 
> Finally I'm able to reproduce this issue. I didn't see any crash with this
> version of the patch.

Glad to see you can reproduce the issue, then you don't need to wait for my test to check in your patch.

Anyway, I'll do the test on my side too in one day or two and let you know.
Flags: needinfo?(loobenyang)
(In reply to Andrea Marchesini (:baku) from comment #26)
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=253826bbd96d
> https://tbpl.mozilla.org/?tree=Try&rev=253826bbd96d
> 
> Finally I'm able to reproduce this issue. I didn't see any crash with this
> version of the patch.

I ran the same test case for an hour with the third patched build, could not reproduce the issue.
Baku, want to ask the drivers if it is still possible to get this to 35, and if not, we need
to disable WS in Workers in 35.
Flags: needinfo?(amarchesini)
(In reply to Looben Yang from comment #28)
> (In reply to Andrea Marchesini (:baku) from comment #26)
> > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=253826bbd96d
> > https://tbpl.mozilla.org/?tree=Try&rev=253826bbd96d
> > 
> > Finally I'm able to reproduce this issue. I didn't see any crash with this
> > version of the patch.
> 
> I ran the same test case for an hour with the third patched build, could not
> reproduce the issue.

I ran two more runs, still could not reproduce this issue.
One about 8 hours overnight - Only got a UAF of Bug 1091962 .
The other about 9 hours the whole day - nothing.
(In reply to Olli Pettay [:smaug] from comment #29)
> Baku, want to ask the drivers if it is still possible to get this to 35, and
> if not, we need
> to disable WS in Workers in 35.

Patches are ready. I guess we can land them.
Flags: needinfo?(amarchesini)
Attached patch patch 2Splinter Review
This patch is what I submitted to tbpl.
Attachment #8529901 - Attachment is obsolete: true
Attachment #8529901 - Flags: sec-approval?
Comment on attachment 8531120 [details] [diff] [review]
patch 2

Approval request in comment 24.
Attachment #8531120 - Flags: sec-approval?
Attachment #8531120 - Flags: approval-mozilla-aurora?
But is 35 already the latest beta?
Blocks: 504553
(In reply to Olli Pettay [:smaug] from comment #34)
> But is 35 already the latest beta?

Yes, we're expecting to ship 35.0beta 1 this Thursday - is this safe to take on Beta 2 or do we have a known exploit in 35 if it ships without this fix?
Flags: needinfo?(nobody)
This should be fine to take on beta 2.
ie it isn't being exploited in the wild as far as we know.
Flags: needinfo?(nobody)
Attachment #8531120 - Flags: sec-approval? → sec-approval+
Assignee: nobody → amarchesini
Keywords: csectype-uaf
Whiteboard: [reporter-external]
Can you land this to trunk so we can get it uplifted soon?
Flags: needinfo?(amarchesini)
Yes we can. It should apply perfectly. In case, NI me and I'll provide a new version of the patch.
Flags: needinfo?(amarchesini)
https://hg.mozilla.org/mozilla-central/rev/6122b7c0dd2e
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Flags: sec-bounty? → sec-bounty+
Is one of the patches Beta only (based on noms) or should both be marked?
Flags: needinfo?(amarchesini)
Comment on attachment 8531120 [details] [diff] [review]
patch 2

both, but actually we decided to disable WebSocket in beta because there are too many regressions. So probably aurora is enough.
Flags: needinfo?(amarchesini)
Attachment #8531120 - Flags: approval-mozilla-beta?
Attachment #8531120 - Flags: approval-mozilla-beta?
Attachment #8531120 - Flags: approval-mozilla-beta+
Attachment #8531120 - Flags: approval-mozilla-aurora?
Attachment #8531120 - Flags: approval-mozilla-aurora+
Attachment #8529299 - Flags: approval-mozilla-beta?
Attachment #8529299 - Flags: approval-mozilla-beta+
Attachment #8529299 - Flags: approval-mozilla-aurora?
Attachment #8529299 - Flags: approval-mozilla-aurora+
Hi Looben,

I can't seem to reproduce the original issue :/ Would you mind taking a quick look at Aurora/BETA to see if it's been fixed for you as you were able to reproduce it pretty consistently?

Builds:

- http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/35.0-candidates/build1/
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-06-00-40-07-mozilla-aurora/
Flags: needinfo?(loobenyang)
(In reply to Kamil Jozwiak [:kjozwiak] from comment #45)
> Hi Looben,
> 
> I can't seem to reproduce the original issue :/ Would you mind taking a
> quick look at Aurora/BETA to see if it's been fixed for you as you were able
> to reproduce it pretty consistently?
> 
> Builds:
> 
> -
> http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/35.0-candidates/
> build1/
> -
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015-01-06-00-40-07-
> mozilla-aurora/

I have verified that it was fixed: https://bugzilla.mozilla.org/show_bug.cgi?id=1105194#c30
Flags: needinfo?(loobenyang)
Group: core-security
Whiteboard: [reporter-external] → [reporter-external][b2g-adv-main2.2-]
You need to log in before you can comment on or make changes to this bug.