1349266 - AddressSanitizer: heap-buffer-overflow [@ GetParent] with READ of size 8

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Comment 1

[Jesse Schwartzentruber (:truber)]

Crash observed while fuzzing mozilla-central rev 62f80de05fa2. I haven't been able to reproduce from the saved test case.

==8821==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61a0003072f8 at pc 0x7f5acbf14846 bp 0x7f5aa2f663b0 sp 0x7f5aa2f663a8
READ of size 8 at 0x61a0003072f8 thread T62 (DOM Worker)
    #0 0x7f5acbf14845 in GetParent /home/worker/workspace/build/src/dom/workers/WorkerPrivate.h:453:12
    #1 0x7f5acbf14845 in mozilla::dom::workers::WorkerRunnable::Run() /home/worker/workspace/build/src/dom/workers/WorkerRunnable.cpp:282
    #2 0x7f5ac64f2d2c in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1269:14
    #3 0x7f5ac64ed0f6 in NS_ProcessPendingEvents(nsIThread*, unsigned int) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:331:19
    #4 0x7f5acbefc77a in ClearMainEventQueue /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:5514:5
    #5 0x7f5acbefc77a in mozilla::dom::workers::WorkerPrivate::DestroySyncLoop(unsigned int, nsIThreadInternal*) /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:5900
    #6 0x7f5acbf162ef in Run /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/WorkerPrivate.h:1634:27
    #7 0x7f5acbf162ef in mozilla::dom::workers::WorkerMainThreadRunnable::Dispatch(mozilla::dom::workers::Status, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/workers/WorkerRunnable.cpp:590
    #8 0x7f5acbe04cb0 in mozilla::dom::workers::scriptloader::ChannelFromScriptURLWorkerThread(JSContext*, mozilla::dom::workers::WorkerPrivate*, nsAString const&, mozilla::dom::workers::WorkerLoadInfo&) /home/worker/workspace/build/src/dom/workers/ScriptLoader.cpp:2151:11
    #9 0x7f5acbef215f in mozilla::dom::workers::WorkerPrivate::GetLoadInfo(JSContext*, nsPIDOMWindowInner*, mozilla::dom::workers::WorkerPrivate*, nsAString const&, bool, mozilla::dom::workers::WorkerPrivate::LoadGroupBehavior, mozilla::dom::WorkerType, mozilla::dom::workers::WorkerLoadInfo*) /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4707:10
    #10 0x7f5acbef0813 in mozilla::dom::workers::WorkerPrivate::Constructor(JSContext*, nsAString const&, bool, mozilla::dom::WorkerType, nsACString const&, mozilla::dom::workers::WorkerLoadInfo*, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4603:19
    #11 0x7f5acbef00ec in Constructor /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4572:10
    #12 0x7f5acbef00ec in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4513
    #13 0x7f5ac9ffea19 in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WorkerBinding.cpp:810:68
    #14 0x7f5acfefd662 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #15 0x7f5acfefd662 in CallJSNativeConstructor /home/worker/workspace/build/src/js/src/jscntxtinlines.h:315
    #16 0x7f5acfefd662 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:551
    #17 0x7f5acfee54d6 in ConstructFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:577:12
    #18 0x7f5acfee54d6 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2946
    #19 0x7f5acfecbc5e in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #20 0x7f5acfefcb76 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #21 0x7f5ad010f85a in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jit/BaselineIC.cpp:2347:14
    #22 0x1a942e26599f  (<unknown module>)

AddressSanitizer can not describe address in more detail (wild memory access suspected).
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/worker/workspace/build/src/dom/workers/WorkerPrivate.h:453:12 in GetParent
Shadow bytes around the buggy address:
  0x0c3480058e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c3480058e50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
  0x0c3480058e60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058e90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3480058ea0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
Thread T62 (DOM Worker) created by T0 here:
    #0 0x4a3a66 in __interceptor_pthread_create /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:245:3
    #1 0x7f5adf07eac9 in _PR_CreateThread /home/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:457:14
    #2 0x7f5adf07e6de in PR_CreateThread /home/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:548:12
    #3 0x7f5ac64ee024 in nsThread::Init(nsACString const&) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:680:8
    #4 0x7f5acbf222a7 in mozilla::dom::workers::WorkerThread::Create(mozilla::dom::workers::WorkerThreadFriendKey const&) /home/worker/workspace/build/src/dom/workers/WorkerThread.cpp:90:7
    #5 0x7f5acbdfb555 in mozilla::dom::workers::RuntimeService::ScheduleWorker(mozilla::dom::workers::WorkerPrivate*) /home/worker/workspace/build/src/dom/workers/RuntimeService.cpp:1881:14
    #6 0x7f5acbdf8fe7 in mozilla::dom::workers::RuntimeService::RegisterWorker(mozilla::dom::workers::WorkerPrivate*) /home/worker/workspace/build/src/dom/workers/RuntimeService.cpp:1708:19
    #7 0x7f5acbef095a in mozilla::dom::workers::WorkerPrivate::Constructor(JSContext*, nsAString const&, bool, mozilla::dom::WorkerType, nsACString const&, mozilla::dom::workers::WorkerLoadInfo*, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4649:24
    #8 0x7f5acbef00ec in Constructor /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4572:10
    #9 0x7f5acbef00ec in mozilla::dom::workers::WorkerPrivate::Constructor(mozilla::dom::GlobalObject const&, nsAString const&, mozilla::ErrorResult&) /home/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:4513
    #10 0x7f5ac9ffea19 in mozilla::dom::WorkerBinding::_constructor(JSContext*, unsigned int, JS::Value*) /home/worker/workspace/build/src/obj-firefox/dom/bindings/WorkerBinding.cpp:810:68
    #11 0x7f5acfefd662 in CallJSNative /home/worker/workspace/build/src/js/src/jscntxtinlines.h:282:15
    #12 0x7f5acfefd662 in CallJSNativeConstructor /home/worker/workspace/build/src/js/src/jscntxtinlines.h:315
    #13 0x7f5acfefd662 in InternalConstruct(JSContext*, js::AnyConstructArgs const&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:551
    #14 0x7f5acfee54d6 in ConstructFromStack /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:577:12
    #15 0x7f5acfee54d6 in Interpret(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:2946
    #16 0x7f5acfecbc5e in js::RunScript(JSContext*, js::RunState&) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:394:12
    #17 0x7f5acfefcb76 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:466:15
    #18 0x7f5acfefd382 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/vm/Interpreter.cpp:512:10
    #19 0x7f5ad087014b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/worker/workspace/build/src/js/src/jsapi.cpp:2887:12
    #20 0x7f5aca32e03e in mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /home/worker/workspace/build/src/obj-firefox/dom/bindings/FunctionBinding.cpp:36:8
    #21 0x7f5ac8bd3946 in Call<nsCOMPtr<nsISupports> > /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/FunctionBinding.h:72:12
    #22 0x7f5ac8bd3946 in nsGlobalWindow::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) /home/worker/workspace/build/src/dom/base/nsGlobalWindow.cpp:13010
    #23 0x7f5ac8d83766 in mozilla::dom::TimeoutManager::RunTimeout(mozilla::dom::Timeout*) /home/worker/workspace/build/src/dom/base/TimeoutManager.cpp:607:42
    #24 0x7f5ac8d7ec0d in mozilla::dom::(anonymous namespace)::TimerCallback(nsITimer*, void*) /home/worker/workspace/build/src/dom/base/Timeout.cpp:65:49
    #25 0x7f5ac65009af in nsTimerImpl::Fire(int) /home/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp:484:7
    #26 0x7f5ac64cf9fb in nsTimerEvent::Run() /home/worker/workspace/build/src/xpcom/threads/TimerThread.cpp:288:11
    #27 0x7f5ac64e2569 in mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() /home/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:200:22
    #28 0x7f5ac64e1e8f in mozilla::ThrottledEventQueue::Inner::Executor::Run() /home/worker/workspace/build/src/xpcom/threads/ThrottledEventQueue.cpp:74:15
    #29 0x7f5ac64f2d2c in nsThread::ProcessNextEvent(bool, bool*) /home/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1269:14
    #30 0x7f5ac64ef658 in NS_ProcessNextEvent(nsIThread*, bool) /home/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:389:10
    #31 0x7f5ac729bd51 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/worker/workspace/build/src/ipc/glue/MessagePump.cpp:96:21
    #32 0x7f5ac71fc880 in RunInternal /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:238:10
    #33 0x7f5ac71fc880 in RunHandler /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:231
    #34 0x7f5ac71fc880 in MessageLoop::Run() /home/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:211
    #35 0x7f5acc4716cf in nsBaseAppShell::Run() /home/worker/workspace/build/src/widget/nsBaseAppShell.cpp:156:27
    #36 0x7f5acf8ed281 in nsAppStartup::Run() /home/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:283:30
    #37 0x7f5acfaa931a in XREMain::XRE_mainRun() /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4492:22
    #38 0x7f5acfaaada3 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4670:8
    #39 0x7f5acfaac12c in XRE_main(int, char**, mozilla::BootstrapConfig const&) /home/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4761:21
    #40 0x4eb2b3 in do_main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:236:22
    #41 0x4eb2b3 in main /home/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:307
    #42 0x7f5ae161b82f in __libc_start_main /build/glibc-t3gR2i/glibc-2.23/csu/../csu/libc-start.c:291

==8821==ABORTING

Comment 2

[Andrew McCreight [:mccr8]]

Andrea, is there anything you can figure out from this stack? If not, maybe this should be marked sec-audit.

Comment 3

[Andrew McCreight [:mccr8]]

This hasn't been reproduced, but I'm going to err on the safe side and mark it sec-high.

Comment 4

[Andrea Marchesini [:baku]]

I know what it is happening here:

1. worker A that is going to be terminated
2. this worker A creates worker B
3. In WorkerPrivate::Constructor we do GetLoadInfo()
4. GetLoadInfo() calls ChannelFromScriptURLWorkerThread and this method creates a sync runnable.
5. At the end of the sync runnable, when we are destroying the sync event loop, we execute the pending control runnables
6. the worker A is finally terminated
7. worker B tries to do: parent->GetParent(), but parent has been released, BOOM.

We should use a WorkerHolder to keep worker A alive until the worker B registration is completed.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: worker_crash.patch

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 8870843 [details] [diff] [review]
worker_crash.patch

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

this is a heap used after free.

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

it's generic enough.

Which older supported branches are affected by this flaw?

all of them.

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

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

Comment 7

[Al Billings [:abillings - ex-MoCo]]

Sec-approval+ for trunk.
I see you have an ESR52 patch. We're going to want a patch nominated for Beta as well.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e144e625835b

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8870843 [details] [diff] [review]
worker_crash.patch

Sec-high, Beta54+, ESR52+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bafab90a3515
https://hg.mozilla.org/releases/mozilla-esr52/rev/dc4e3c64d781