Closed Bug 1507180 Opened 6 years ago Closed 4 years ago

AddressSanitizer: heap-use-after-free nsTArray.h:372:37 in Length

Categories

(Core :: DOM: Service Workers, defect, P2)

Product:
Core
Component:
DOM: Service Workers
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla73
Tracking Flags:
Tracking Status
firefox-esr68 72+ fixed
firefox71 --- wontfix
firefox72 + fixed
firefox73 + fixed

People

(Reporter: bc, Assigned: ytausky)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-uaf, sec-moderate, Whiteboard: [post-critsmash-triage][adv-main72+r][adv-esr68.4+r])

Attachments

(2 files)

Bug 1507180 - Make copy of list before iterating over it
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr68+
tjr
: sec-approval+
Details | Review
Bug 1507180 - Add explanatory comment r=#dom-workers-and-storage-reviewers
47 bytes, text/x-phabricator-request
Details | Review 
Bughunter has seen this for quite some time (since March on 14 different urls), but I haven't been able to reproduce manually. I've retested and reproduced in automation for two urls on Fedora 29 and Ubuntu 18.10 and decided to file this to get it on the radar.

Most recently

1. https://www.iheart.com/ (Fedora)

2. https://news.iheart.com/featured/iheartradio-music-festival/ (Ubuntu)

mozversion INFO | application_buildid: 20181113214624
mozversion INFO | application_changeset: 24e87b02707bee36e1e98eb37c94fbaf3834e898
mozversion INFO | application_version: 65.0a1

==20940==ERROR: AddressSanitizer: heap-use-after-free on address 0x6080000abae0 at pc 0x7f9ab7f3d4bf bp 0x7f9aa3ffcb50 sp 0x7f9aa3ffcb48
READ of size 8 at 0x6080000abae0 thread T23 (IPDL Background)
    #0 0x7f9ab7f3d4be in Length /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:372:37
    #1 0x7f9ab7f3d4be in Length /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:91
    #2 0x7f9ab7f3d4be in HasMore /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTObserverArray.h:362
    #3 0x7f9ab7f3d4be in mozilla::dom::cache::StreamControl::CloseAllReadStreams() /builds/worker/workspace/build/src/dom/cache/StreamControl.cpp:72
    #4 0x7f9ab7ee5dd6 in NotifyCloseAll /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:192:3
    #5 0x7f9ab7ee5dd6 in mozilla::dom::cache::CacheStreamControlParent::CloseAll() /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:166
    #6 0x7f9ab7eed911 in mozilla::dom::cache::Context::CancelAll() /builds/worker/workspace/build/src/dom/cache/Context.cpp:902:23
    #7 0x7f9ab7f34825 in mozilla::dom::cache::Manager::Shutdown() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:2011:14
    #8 0x7f9ab7f2a3f3 in mozilla::dom::cache::Manager::Factory::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:315:18
    #9 0x7f9ab7f29f4a in mozilla::dom::cache::Manager::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1592:3
    #10 0x7f9ab95560cf in mozilla::dom::quota::QuotaManager::Shutdown() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3689:22
    #11 0x7f9ab9555c4b in mozilla::dom::quota::QuotaManager::ShutdownRunnable::Run() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:2855:19
    #12 0x7f9ab2576062 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1246:14
    #13 0x7f9ab257d1a8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:530:10
    #14 0x7f9ab35083b0 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:364:5
    #15 0x7f9ab345ac6f in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:325:10
    #16 0x7f9ab345ac6f in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:318
    #17 0x7f9ab345ac6f in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:298
    #18 0x7f9ab257013a in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:505:11
    #19 0x7f9ac9afe676 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #20 0x7f9accbe9163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)
    #21 0x7f9acc7cedee in clone /build/glibc-B9XfQf/glibc-2.28/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

0x6080000abae0 is located 64 bytes inside of 96-byte region [0x6080000abaa0,0x6080000abb00)
freed by thread T23 (IPDL Background) here:
    #0 0x560d198e65c2 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7f9ab34b212b in mozilla::ipc::BackgroundParentImpl::DeallocPCacheStreamControlParent(mozilla::dom::cache::PCacheStreamControlParent*) /builds/worker/workspace/build/src/ipc/glue/BackgroundParentImpl.cpp:806:3
    #2 0x7f9ab3c5c866 in mozilla::ipc::PBackgroundParent::RemoveManagee(int, mozilla::ipc::IProtocol*) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PBackgroundParent.cpp
    #3 0x7f9ab3d4b50f in mozilla::dom::cache::PCacheStreamControlParent::Send__delete__(mozilla::dom::cache::PCacheStreamControlParent*) /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/PCacheStreamControlParent.cpp:116:12
    #4 0x7f9ab7f3a094 in NoteClosed /builds/worker/workspace/build/src/dom/cache/StreamControl.cpp:35:3
    #5 0x7f9ab7f3a094 in NoteClosedOnOwningThread /builds/worker/workspace/build/src/dom/cache/ReadStream.cpp:457
    #6 0x7f9ab7f3a094 in mozilla::dom::cache::ReadStream::Inner::NoteClosed() /builds/worker/workspace/build/src/dom/cache/ReadStream.cpp:417
    #7 0x7f9ab7f3d371 in mozilla::dom::cache::StreamControl::CloseAllReadStreams() /builds/worker/workspace/build/src/dom/cache/StreamControl.cpp:73:21
    #8 0x7f9ab7ee5dd6 in NotifyCloseAll /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:192:3
    #9 0x7f9ab7ee5dd6 in mozilla::dom::cache::CacheStreamControlParent::CloseAll() /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:166
    #10 0x7f9ab7eed911 in mozilla::dom::cache::Context::CancelAll() /builds/worker/workspace/build/src/dom/cache/Context.cpp:902:23
    #11 0x7f9ab7f34825 in mozilla::dom::cache::Manager::Shutdown() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:2011:14
    #12 0x7f9ab7f2a3f3 in mozilla::dom::cache::Manager::Factory::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:315:18
    #13 0x7f9ab7f29f4a in mozilla::dom::cache::Manager::ShutdownAll() /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1592:3
    #14 0x7f9ab95560cf in mozilla::dom::quota::QuotaManager::Shutdown() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:3689:22
    #15 0x7f9ab9555c4b in mozilla::dom::quota::QuotaManager::ShutdownRunnable::Run() /builds/worker/workspace/build/src/dom/quota/ActorsParent.cpp:2855:19
    #16 0x7f9ab2576062 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1246:14
    #17 0x7f9ab257d1a8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:530:10
    #18 0x7f9ab35083b0 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:364:5
    #19 0x7f9ab345ac6f in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:325:10
    #20 0x7f9ab345ac6f in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:318
    #21 0x7f9ab345ac6f in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:298
    #22 0x7f9ab257013a in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:505:11
    #23 0x7f9ac9afe676 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #24 0x7f9accbe9163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)

previously allocated by thread T23 (IPDL Background) here:
    #0 0x560d198e6943 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x560d1991a2dd in moz_xmalloc /builds/worker/workspace/build/src/memory/mozalloc/mozalloc.cpp:70:17
    #2 0x7f9ab7ec7161 in operator new /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:139:12
    #3 0x7f9ab7ec7161 in mozilla::dom::cache::AutoParentOpResult::SerializeReadStream(nsID const&, mozilla::dom::cache::StreamList*, mozilla::dom::cache::CacheReadStream*) /builds/worker/workspace/build/src/dom/cache/AutoUtils.cpp:543
    #4 0x7f9ab7ec5bac in mozilla::dom::cache::AutoParentOpResult::SerializeResponseBody(mozilla::dom::cache::SavedResponse const&, mozilla::dom::cache::StreamList*, mozilla::dom::cache::CacheResponse*) /builds/worker/workspace/build/src/dom/cache/AutoUtils.cpp:527:3
    #5 0x7f9ab7ed8f80 in mozilla::dom::cache::CacheOpParent::OnOpComplete(mozilla::ErrorResult&&, mozilla::dom::cache::CacheOpResult const&, long, nsTArray<mozilla::dom::cache::SavedResponse> const&, nsTArray<mozilla::dom::cache::SavedRequest> const&, mozilla::dom::cache::StreamList*) /builds/worker/workspace/build/src/dom/cache/CacheOpParent.cpp:194:12
    #6 0x7f9ab7f28ae2 in mozilla::dom::cache::Manager::Listener::OnOpComplete(mozilla::ErrorResult&&, mozilla::dom::cache::CacheOpResult const&, mozilla::dom::cache::SavedResponse const&, mozilla::dom::cache::StreamList*) /builds/worker/workspace/build/src/dom/cache/Manager.cpp:1548:3
    #7 0x7f9ab7f4e2f3 in mozilla::dom::cache::Manager::CacheMatchAction::Complete(mozilla::dom::cache::Manager::Listener*, mozilla::ErrorResult&&) /builds/worker/workspace/build/src/dom/cache/Manager.cpp:572:18
    #8 0x7f9ab7f4d498 in mozilla::dom::cache::Manager::BaseAction::CompleteOnInitiatingThread(nsresult) /builds/worker/workspace/build/src/dom/cache/Manager.cpp:449:7
    #9 0x7f9ab7eeb0ff in mozilla::dom::cache::Context::ActionRunnable::Run() /builds/worker/workspace/build/src/dom/cache/Context.cpp:712:16
    #10 0x7f9ab2576062 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1246:14
    #11 0x7f9ab257d1a8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:530:10
    #12 0x7f9ab350822a in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:334:20
    #13 0x7f9ab345ac6f in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:325:10
    #14 0x7f9ab345ac6f in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:318
    #15 0x7f9ab345ac6f in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:298
    #16 0x7f9ab257013a in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:505:11
    #17 0x7f9ac9afe676 in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #18 0x7f9accbe9163 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x8163)

Thread T23 (IPDL Background) created by T0 here:
    #0 0x560d198cf25d in __interceptor_pthread_create /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
    #1 0x7f9ac9afb3a5 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:433:14
    #2 0x7f9ac9afaf8e in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:518:12
    #3 0x7f9ab2572499 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:719:8
    #4 0x7f9ab257c2fb in nsThreadManager::NewNamedThread(nsTSubstring<char> const&, unsigned int, nsIThread**) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:485:22
    #5 0x7f9ab257fa99 in NS_NewNamedThread(nsTSubstring<char> const&, nsIThread**, nsIRunnable*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:143:45
    #6 0x7f9ab34d99b9 in NS_NewNamedThread<16> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:75:10
    #7 0x7f9ab34d99b9 in (anonymous namespace)::ParentImpl::CreateBackgroundThread() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1015
    #8 0x7f9ab34de379 in RunOnMainThread /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1330:30
    #9 0x7f9ab34de379 in (anonymous namespace)::ParentImpl::CreateActorHelper::Run() /builds/worker/workspace/build/src/ipc/glue/BackgroundImpl.cpp:1351
    #10 0x7f9ab2576062 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1246:14
    #11 0x7f9ab257d1a8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:530:10
    #12 0x7f9ab257457c in SpinEventLoopUntil<mozilla::ProcessFailureBehavior::ReportToCaller, (lambda at /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:954:22)> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:347:25
    #13 0x7f9ab257457c in nsThread::Shutdown() /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:954
    #14 0x7f9ab3eb4d94 in applyImpl<nsIThread, nsresult (nsIThread::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1191:12
    #15 0x7f9ab3eb4d94 in apply<nsIThread, nsresult (nsIThread::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1197
    #16 0x7f9ab3eb4d94 in mozilla::detail::RunnableMethodImpl<RefPtr<nsIThread>, nsresult (nsIThread::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1242
    #17 0x7f9ab2576062 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1246:14
    #18 0x7f9ab257d1a8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:530:10
    #19 0x7f9ab257ca10 in SpinEventLoopUntil<mozilla::ProcessFailureBehavior::ReportToCaller, (lambda at /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:558:36)> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:347:25
    #20 0x7f9ab257ca10 in nsThreadManager::SpinEventLoopUntilInternal(nsINestedEventLoopCondition*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp:558
    #21 0x7f9ab25a6ae1 in NS_InvokeByIndex /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
    #22 0x7f9ab3f8803c in Invoke /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1723:12
    #23 0x7f9ab3f8803c in Call /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1268
    #24 0x7f9ab3f8803c in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1232
    #25 0x7f9ab3f8de16 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1020:12
    #26 0x7f9ac0043a1d in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:468:15
    #27 0x7f9ac0043a1d in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:560
    #28 0x7f9ac002d23a in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:620:12
    #29 0x7f9ac002d23a in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3461
    #30 0x7f9ac0010aa6 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:447:12
    #31 0x7f9ac00443c1 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:587:15
    #32 0x7f9ac0046042 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:633:10
    #33 0x7f9abe9ee370 in js::fun_apply(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/JSFunction.cpp:1381:12
    #34 0x7f9ac0043a1d in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:468:15
    #35 0x7f9ac0043a1d in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:560
    #36 0x7f9ac002d23a in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:620:12
    #37 0x7f9ac002d23a in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3461
    #38 0x7f9ac0010aa6 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:447:12
    #39 0x7f9ac00443c1 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:587:15
    #40 0x7f9ac0046042 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:633:10
    #41 0x7f9abf0e4a2a in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2911:12
    #42 0x7f9ab3f6f0bc in nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:1174:23
    #43 0x7f9ab25a81e8 in PrepareAndDispatch /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:127:37
    #44 0x7f9ab25a70ba in SharedStub (/mozilla/builds/nightly-asan/mozilla/firefox-opt/dist/bin/libxul.so+0x22700ba)
    #45 0x7f9abe390d0a in nsXREDirProvider::DoStartup() /builds/worker/workspace/build/src/toolkit/xre/nsXREDirProvider.cpp:1094:11
    #46 0x7f9abe36db31 in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4619:16
    #47 0x7f9abe370a64 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4934:8
    #48 0x7f9abe372490 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:5026:21
    #49 0x560d199191dc in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:233:22
    #50 0x560d199191dc in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:315
    #51 0x7f9acc6d809a in __libc_start_main /build/glibc-B9XfQf/glibc-2.28/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h:372:37 in Length
Shadow bytes around the buggy address:
  0x0c108000d700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d710: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d720: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d730: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d740: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c108000d750: fa fa fa fa fd fd fd fd fd fd fd fd[fd]fd fd fd
  0x0c108000d760: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c108000d770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c108000d7a0: fa fa fa fa 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
  Shadow gap:              cc
Group: core-security → dom-core-security
Keywords: csectype-uaf
Looks like it's in shutdown. Does this happen at the end of a run? or does bughunter open and close tabs that might be shutting down processes? Looks like the parent process though: /builds/worker/workspace/build/src/dom/cache/CacheStreamControlParent.cpp:166
Flags: needinfo?(bob)
Keywords: sec-moderate
The current version just opens the one tab then shuts down after a determined number of seconds. It does appear to happen when marionette begins shutting down.
Flags: needinfo?(bob)
Component: DOM → DOM: Service Workers

It looks like StreamControl::CloseAllReadStreams drains its mReadStreamList and on the last iteration line [1] reaches all the way down to [2], which deletes the this while StreamControl::CloseAllReadStreams still holds a reference to a member of the deleted object.

[1] https://searchfox.org/mozilla-central/source/dom/cache/StreamControl.cpp#62
[2] https://searchfox.org/mozilla-central/source/dom/cache/CacheStreamControlParent.cpp#149

I think we could trigger this reliably in a test by adding the result of a hanging fetch to the cache and then shutting down (i.e. make the server stop sending data in the middle of the stream without resetting the connection). I'll try it next week.

Assignee: nobody → ytausky
Priority: -- → P2

I dug a little deeper and found out that the offending code path is only used when retrieving something from the cache, not the other way around. This makes it somewhat hard to test, because the timing needs to be just right. I'll put this on hold for a while.

Hopefully this comment would prevent a future contributor from
removing the copy operation.

Comment on attachment 9112554 [details]
Bug 1507180 - Make copy of list before iterating over it

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: I don't think it's obvious, but I'm not sure.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which older supported branches are affected by this flaw?: All
  • If not all supported branches, which bug introduced the flaw?: None
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: The same patch should apply cleanly to all supported branches.
  • How likely is this patch to cause regressions; how much testing does it need?: Very unlikely.
Attachment #9112554 - Flags: sec-approval?

Comment on attachment 9112554 [details]
Bug 1507180 - Make copy of list before iterating over it

The patch is pretty obvious what the problem is; but given it's a hard to reproduce race in the parent (where you have limited grooming capability) I think it's okay to land now and request uplift.

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

https://hg.mozilla.org/integration/autoland/rev/cdf525897bffc445c4c860225ecb42eda5925a86
https://hg.mozilla.org/mozilla-central/rev/cdf525897bff

Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox73: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla73

Please nominate this patch for Beta and ESR68 approval when you get a chance.

status-firefox71: --- → wontfix
status-firefox72: --- → affected
status-firefox-esr68: --- → affected
tracking-firefox72: --- → +
tracking-firefox73: --- → +
tracking-firefox-esr68: --- → 72+
Flags: needinfo?(ytausky)

Comment on attachment 9112554 [details]
Bug 1507180 - Make copy of list before iterating over it

Beta/Release Uplift Approval Request

  • User impact if declined: If an exploit for this emerges, it could be used against users of beta.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's not risky because it's a one line change that creates a copy of a list and iterates over it instead of the original.
  • String changes made/needed: None
Flags: needinfo?(ytausky)
Attachment #9112554 - Flags: approval-mozilla-beta?

Comment on attachment 9112554 [details]
Bug 1507180 - Make copy of list before iterating over it

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: It's a low-risk fix.
  • User impact if declined: If an exploit for this emerges, it could be used against users of ESR.
  • Fix Landed on Version: 73
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's not risky because it's a straightforward one line change.
  • String or UUID changes made by this patch: None
Attachment #9112554 - Flags: approval-mozilla-esr68?

Comment on attachment 9112554 [details]
Bug 1507180 - Make copy of list before iterating over it

Fixes a service worker UAF bug. Approved for 72.0b6 and 68.4esr.

Attachment #9112554 - Flags: approval-mozilla-esr68?
Attachment #9112554 - Flags: approval-mozilla-esr68+
Attachment #9112554 - Flags: approval-mozilla-beta?
Attachment #9112554 - Flags: approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main72+r]
Whiteboard: [post-critsmash-triage][adv-main72+r] → [post-critsmash-triage][adv-main72+r][adv-esr68.4+r]

The explanatory comment of D55424 did not yet make it to mozilla-central. Can you please take care of it?

Flags: needinfo?(ssengupta)
Flags: needinfo?(ssengupta)

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #20)

Shall it also be ported to beta etc.?

I don't think we need to. It's just a comment and it's not likely to be something that other uplifts would depend on in their patch context.

Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: