1105194 - Use After Free in DispatchPrivate()

Reporter

Description

•

8 years ago

Attached file wsserver_uaf1048.js — Details

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

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 1

•

8 years ago

[Tracking Requested - why for this release]:
Most probably a regression from bug 504553.

tracking-firefox35: --- → ?

tracking-firefox36: --- → ?

Andrea Marchesini [:baku]

Assignee

Updated

•

8 years ago

Status: NEW → RESOLVED

Closed: 8 years ago

Resolution: --- → DUPLICATE

Looben Yang

Reporter

Comment 3

•

8 years ago

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 → ---

Olli Pettay [:smaug][bugs@pettay.fi]

Updated

•

8 years ago

Flags: needinfo?(amarchesini)

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Flags: sec-bounty?

Andrea Marchesini [:baku]

Assignee

Comment 4

•

8 years ago

Attached patch crash.patch (obsolete) — Details — Splinter Review

Flags: needinfo?(amarchesini)

Attachment #8529289 - Flags: review?(bugs)

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 5

•

8 years ago

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+

Andrea Marchesini [:baku]

Assignee

Comment 6

•

8 years ago

Attached patch id.patch (obsolete) — Details — Splinter Review

Attachment #8529298 - Flags: review?(bugs)

Olli Pettay [:smaug][bugs@pettay.fi]

Updated

•

8 years ago

Attachment #8529298 - Flags: review?(bugs) → review+

Andrea Marchesini [:baku]

Assignee

Comment 7

•

8 years ago

Attached patch crash.patch — Details — Splinter Review

patch merged

Attachment #8529289 - Attachment is obsolete: true

Attachment #8529298 - Attachment is obsolete: true

Andrea Marchesini [:baku]

Assignee

Comment 8

•

8 years ago

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)

Looben Yang

Reporter

Comment 9

•

8 years ago

(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)

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Attachment #8528914 - Attachment mime type: application/x-javascript → text/plain

Andrea Marchesini [:baku]

Assignee

Comment 10

•

8 years ago

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)

Looben Yang

Reporter

Comment 11

•

8 years ago

(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)

Andrea Marchesini [:baku]

Assignee

Comment 12

•

8 years ago

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?

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

status-firefox34: --- → unaffected

status-firefox35: --- → affected

status-firefox36: --- → affected

status-firefox-esr31: --- → unaffected

tracking-firefox35: ? → +

tracking-firefox36: ? → +

Al Billings [:abillings - ex-MoCo]

Comment 13

•

8 years ago

Is this actually a use-after-free? Before approval, I need to rate this bug for security, especially since it is an external report.

Al Billings [:abillings - ex-MoCo]

Comment 14

•

8 years ago

Marking as sec-high after discussion with Dan Veditz.

Keywords: sec-high

Al Billings [:abillings - ex-MoCo]

Comment 15

•

8 years ago

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+

Looben Yang

Reporter

Comment 16

•

8 years ago

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

Andrea Marchesini [:baku]

Assignee

Comment 17

•

8 years ago

Attached patch patch 2 (obsolete) — Details — 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)

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 18

•

8 years ago

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-

Andrea Marchesini [:baku]

Assignee

Comment 19

•

8 years ago

Attached patch patch 2 (obsolete) — Details — Splinter Review

Attachment #8529736 - Attachment is obsolete: true

Attachment #8529901 - Flags: review?(bugs)

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 20

•

8 years ago

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+

Andrea Marchesini [:baku]

Assignee

Comment 21

•

8 years ago

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)

Looben Yang

Reporter

Comment 22

•

8 years ago

(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)

Andrea Marchesini [:baku]

Assignee

Comment 23

•

8 years ago

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?

Andrea Marchesini [:baku]

Assignee

Comment 24

•

8 years ago

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?

Looben Yang

Reporter

Comment 25

•

8 years ago

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

Andrea Marchesini [:baku]

Assignee

Comment 26

•

8 years ago

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)

Looben Yang

Reporter

Comment 27

•

8 years ago

(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)

Looben Yang

Reporter

Comment 28

•

8 years ago

(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.

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 29

•

8 years ago

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)

Looben Yang

Reporter

Comment 30

•

8 years ago

(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.

Andrea Marchesini [:baku]

Assignee

Comment 31

•

8 years ago

(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)

Andrea Marchesini [:baku]

Assignee

Comment 32

•

8 years ago

Attached patch patch 2 — Details — Splinter Review

This patch is what I submitted to tbpl.

Attachment #8529901 - Attachment is obsolete: true

Attachment #8529901 - Flags: sec-approval?

Andrea Marchesini [:baku]

Assignee

Comment 33

•

8 years ago

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?

Olli Pettay [:smaug][bugs@pettay.fi]

Comment 34

•

8 years ago

But is 35 already the latest beta?

Blocks: 504553

Lukas Blakk [:lsblakk] use ?needinfo

Comment 35

•

8 years ago

(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)

Andrew McCreight [:mccr8]

Comment 36

•

8 years ago

This should be fine to take on beta 2.

Andrew McCreight [:mccr8]

Comment 37

•

8 years ago

ie it isn't being exploited in the wild as far as we know.

Flags: needinfo?(nobody)

Al Billings [:abillings - ex-MoCo]

Updated

•

8 years ago

Attachment #8531120 - Flags: sec-approval? → sec-approval+

Lukas Blakk [:lsblakk] use ?needinfo

Updated

•

8 years ago

status-firefox37: --- → affected

tracking-firefox37: --- → +

Andrew McCreight [:mccr8]

Updated

•

8 years ago

Assignee: nobody → amarchesini

Andrew McCreight [:mccr8]

Updated

•

8 years ago

Keywords: csectype-uaf

Whiteboard: [reporter-external]

Lukas Blakk [:lsblakk] use ?needinfo

Comment 38

•

8 years ago

Can you land this to trunk so we can get it uplifted soon?

Flags: needinfo?(amarchesini)

Andrea Marchesini [:baku]

Assignee

Comment 39

•

8 years ago

Yes we can. It should apply perfectly. In case, NI me and I'll provide a new version of the patch.

Flags: needinfo?(amarchesini)

Andrew McCreight [:mccr8]

Updated

•

8 years ago

Keywords: checkin-needed

Ryan VanderMeulen [:RyanVM]

Comment 40

•

8 years ago

Pushed as a folded patch w/ smaug's blessing.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6122b7c0dd2e

status-b2g-v1.4: --- → unaffected

status-b2g-v2.0: --- → unaffected

status-b2g-v2.0M: --- → unaffected

status-b2g-v2.1: --- → unaffected

status-b2g-v2.2: --- → affected

Flags: in-testsuite?

Keywords: checkin-needed

Ryan VanderMeulen [:RyanVM]

Comment 41

•

8 years ago

https://hg.mozilla.org/mozilla-central/rev/6122b7c0dd2e

Status: REOPENED → RESOLVED

Closed: 8 years ago → 8 years ago

status-b2g-v2.2: affected → fixed

status-firefox37: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla37

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Flags: sec-bounty? → sec-bounty+

Al Billings [:abillings - ex-MoCo]

Comment 42

•

8 years ago

Is one of the patches Beta only (based on noms) or should both be marked?

Flags: needinfo?(amarchesini)

Andrea Marchesini [:baku]

Assignee

Comment 43

•

8 years ago

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?

Lukas Blakk [:lsblakk] use ?needinfo

Updated

•

8 years ago

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

Attachment #8531120 - Flags: approval-mozilla-beta+

Attachment #8531120 - Flags: approval-mozilla-aurora?

Attachment #8531120 - Flags: approval-mozilla-aurora+

Lukas Blakk [:lsblakk] use ?needinfo

Updated

•

8 years ago

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

Attachment #8529299 - Flags: approval-mozilla-beta+

Attachment #8529299 - Flags: approval-mozilla-aurora?

Attachment #8529299 - Flags: approval-mozilla-aurora+

Ryan VanderMeulen [:RyanVM]

Comment 44

•

8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/7a83256f3e2f
https://hg.mozilla.org/releases/mozilla-beta/rev/f6020e8a9490

status-firefox35: affected → fixed

status-firefox36: affected → fixed

Kamil Jozwiak [:kjozwiak]

Comment 45

•

8 years ago

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)

Looben Yang

Reporter

Comment 46

•

8 years ago

(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)

Daniel Veditz [:dveditz]

Updated

•

8 years ago

Group: core-security

Christiane Ruetten [INACTIVE]

Updated

•

8 years ago

Whiteboard: [reporter-external] → [reporter-external][b2g-adv-main2.2-]

Ben Kelly [:bkelly, not reviewing]

Updated

•

6 years ago

Depends on: 1334165

wsserver_uaf1048.js 8 years ago Looben Yang 2.82 KB, text/plain		Details
crash.patch 8 years ago Andrea Marchesini [:baku] 10.93 KB, patch	smaug : review+	Details \| Diff \| Splinter Review
id.patch 8 years ago Andrea Marchesini [:baku] 798 bytes, patch	smaug : review+	Details \| Diff \| Splinter Review
crash.patch 8 years ago Andrea Marchesini [:baku] 11.07 KB, patch	lsblakk : approval-mozilla-aurora+ lsblakk : approval-mozilla-beta+ abillings : sec-approval+	Details \| Diff \| Splinter Review
patch 2 8 years ago Andrea Marchesini [:baku] 12.26 KB, patch	smaug : review-	Details \| Diff \| Splinter Review
patch 2 8 years ago Andrea Marchesini [:baku] 13.31 KB, patch	smaug : review+	Details \| Diff \| Splinter Review
patch 2 8 years ago Andrea Marchesini [:baku] 13.75 KB, patch	lsblakk : approval-mozilla-aurora+ lsblakk : approval-mozilla-beta+ abillings : sec-approval+	Details \| Diff \| Splinter Review