Closed Bug 1539219 Opened 5 years ago Closed 5 years ago

AddressSanitizer: heap-use-after-free [@ IsCurrentThread] with READ of size 8

Categories

(Core :: Storage: IndexedDB, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla69
Tracking Status
firefox-esr60 68+ verified
firefox67 --- wontfix
firefox68 + verified
firefox69 + verified

People

(Reporter: jkratzer, Assigned: tt)

References

(Blocks 3 open bugs)

Details

(4 keywords, Whiteboard: [post-critsmash-triage][adv-main68+][adv-esr60.8+])

Attachments

(3 files)

Testcase found while fuzzing mozilla-central rev 4572f6055a6a. I'm currently reducing the testcase and will update once complete.

==8994==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000481b40 at pc 0x7fbb1c28a4d0 bp 0x7ffdec95d640 sp 0x7ffdec95d638
READ of size 8 at 0x611000481b40 thread T0 (file:// Content)
#0 0x7fbb1c28a4cf in IsCurrentThread /src/xpcom/base/nsISupportsImpl.cpp:45:10
#1 0x7fbb1c28a4cf in nsAutoOwningThread::AssertCurrentThreadOwnsMe(char const*) const /src/xpcom/base/nsISupportsImpl.cpp:38
#2 0x7fbb25ed445e in AssertOwnership<27> /src/obj-firefox/dist/include/nsISupportsImpl.h:59:5
#3 0x7fbb25ed445e in AssertIsOnOwningThread /src/dom/indexedDB/IDBRequest.h:153
#4 0x7fbb25ed445e in SetTransaction /src/dom/indexedDB/IDBRequest.cpp:444
#5 0x7fbb25ed445e in mozilla::dom::indexedDB::BackgroundVersionChangeTransactionChild::RecvComplete(nsresult const&) /src/dom/indexedDB/ActorsChild.cpp:2391
#6 0x7fbb1e4e22e0 in mozilla::dom::indexedDB::PBackgroundIDBVersionChangeTransactionChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PBackgroundIDBVersionChangeTransactionChild.cpp:496:20
#7 0x7fbb1df3de77 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:2828:28
#8 0x7fbb1d77c289 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2151:21
#9 0x7fbb1d777fca in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2078:9
#10 0x7fbb1d77a207 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1937:3
#11 0x7fbb1d77af97 in mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1968:13
#12 0x7fbb1c466c65 in mozilla::SchedulerGroup::Runnable::Run() /src/xpcom/threads/SchedulerGroup.cpp:295:32
#13 0x7fbb1c4a65e1 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1180:14
#14 0x7fbb1c4ae9ed in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:482:10
#15 0x7fbb1d785684 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:110:5
#16 0x7fbb1d65bc5e in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
#17 0x7fbb1d65bc5e in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
#18 0x7fbb1d65bc5e in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
#19 0x7fbb26b09eb3 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
#20 0x7fbb2b0fac7e in XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:933:20
#21 0x7fbb1d65bc5e in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
#22 0x7fbb1d65bc5e in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
#23 0x7fbb1d65bc5e in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
#24 0x7fbb2b0f9e0c in XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:771:34
#25 0x55bc213c5834 in content_process_main /src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#26 0x55bc213c5834 in main /src/browser/app/nsBrowserApp.cpp:263
#27 0x7fbb4044482f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#28 0x55bc212eaebc in _start (/home/ubuntu/builds/m-c-20190326095147-fuzzing-asan-opt/firefox+0x2debc)

0x611000481b40 is located 64 bytes inside of 248-byte region [0x611000481b00,0x611000481bf8)
freed by thread T0 (file:// Content) here:
#0 0x55bc213929e2 in free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
#1 0x7fbb1c26ad81 in SnowWhiteKiller::~SnowWhiteKiller() /src/xpcom/base/nsCycleCollector.cpp:2416:7
#2 0x7fbb1c269433 in nsCycleCollector::FreeSnowWhite(bool) /src/xpcom/base/nsCycleCollector.cpp:2607:3
#3 0x7fbb1c275d12 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:3578:3
#4 0x7fbb1c274fa5 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /src/xpcom/base/nsCycleCollector.cpp:3407:9
#5 0x7fbb1c279f76 in nsCycleCollector_collect(nsICycleCollectorListener*) /src/xpcom/base/nsCycleCollector.cpp:3943:21
#6 0x7fbb20bc261a in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /src/dom/base/nsJSEnvironment.cpp:1414:3
#7 0x7fbb235682b9 in mozilla::dom::FuzzingFunctions_Binding::cycleCollect(JSContext*, unsigned int, JS::Value*) /src/obj-firefox/dom/bindings/FuzzingFunctionsBinding.cpp:66:3
#8 0x7fbb2b3dba37 in CallJSNative /src/js/src/vm/Interpreter.cpp:442:13
#9 0x7fbb2b3dba37 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:534
#10 0x7fbb2b3c3e4a in CallFromStack /src/js/src/vm/Interpreter.cpp:593:10
#11 0x7fbb2b3c3e4a in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3075
#12 0x7fbb2b3a5e78 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:422:10
#13 0x7fbb2b3dc3a6 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:562:13
#14 0x7fbb2b3ddff2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:605:8
#15 0x7fbb2bfc3619 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2621:10
#16 0x7fbb23268f39 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:52:8
#17 0x7fbb244d97b2 in HandleEvent<mozilla::dom::EventTarget > /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
#18 0x7fbb244d97b2 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener
, mozilla::dom::Event*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1038
#19 0x7fbb244dbde3 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /src/dom/events/EventListenerManager.cpp:1239:17
#20 0x7fbb244bbea0 in HandleEvent /src/obj-firefox/dist/include/mozilla/EventListenerManager.h:355:5
#21 0x7fbb244bbea0 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:351
#22 0x7fbb244ba0c8 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:553:16
#23 0x7fbb244c0d13 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:1048:11

previously allocated by thread T0 (file:// Content) here:
#0 0x55bc21392d63 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
#1 0x55bc213c75fd in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:68:15
#2 0x7fbb25f1ba8b in operator new /src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
#3 0x7fbb25f1ba8b in mozilla::dom::IDBOpenDBRequest::CreateForWindow(JSContext*, mozilla::dom::IDBFactory*, nsPIDOMWindowInner*, JS::Handle<JSObject*>) /src/dom/indexedDB/IDBRequest.cpp:400
#4 0x7fbb25f18b59 in mozilla::dom::IDBFactory::OpenInternal(JSContext*, nsIPrincipal*, nsTSubstring<char16_t> const&, mozilla::dom::Optional<unsigned long> const&, mozilla::dom::Optional<mozilla::dom::StorageType> const&, bool, mozilla::dom::CallerType, mozilla::ErrorResult&) /src/dom/indexedDB/IDBFactory.cpp:735:9
#5 0x7fbb25f1aad1 in mozilla::dom::IDBFactory::Open(JSContext*, nsTSubstring<char16_t> const&, mozilla::dom::IDBOpenDBOptions const&, mozilla::dom::CallerType, mozilla::ErrorResult&) /src/dom/indexedDB/IDBFactory.cpp:476:10
#6 0x7fbb23a70d5a in mozilla::dom::IDBFactory_Binding::open(JSContext*, JS::Handle<JSObject*>, mozilla::dom::IDBFactory*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/IDBFactoryBinding.cpp:282:76
#7 0x7fbb23c602d1 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3144:13
#8 0x7fbb2b3dba37 in CallJSNative /src/js/src/vm/Interpreter.cpp:442:13
#9 0x7fbb2b3dba37 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:534
#10 0x7fbb2c53f912 in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /src/js/src/jit/BaselineIC.cpp:3775:10
#11 0x7fbad2585887 (<unknown module>)
#12 0x63100109f537 (<unknown module>)
#13 0x7fbad25834de (<unknown module>)
#14 0x7fbb2c72e5cb in EnterBaseline /src/js/src/jit/BaselineJIT.cpp:111:5
#15 0x7fbb2c72e5cb in js::jit::EnterBaselineAtBranch(JSContext*, js::InterpreterFrame*, unsigned char*) /src/js/src/jit/BaselineJIT.cpp:189
#16 0x7fbb2b3cbddd in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:1982:24
#17 0x7fbb2b3a5e78 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:422:10
#18 0x7fbb2b3dc3a6 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:562:13
#19 0x7fbb2b3ddff2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:605:8
#20 0x7fbb2bfc3619 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2621:10
#21 0x7fbb23268f39 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:52:8
#22 0x7fbb244d97b2 in HandleEvent<mozilla::dom::EventTarget > /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
#23 0x7fbb244d97b2 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener
, mozilla::dom::Event*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1038

SUMMARY: AddressSanitizer: heap-use-after-free /src/xpcom/base/nsISupportsImpl.cpp:45:10 in IsCurrentThread
Shadow bytes around the buggy address:
0x0c2280088310: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2280088320: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2280088330: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
0x0c2280088340: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2280088350: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
=>0x0c2280088360: fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd
0x0c2280088370: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c2280088380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2280088390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c22800883a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c22800883b0: 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
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
==8994==ABORTING

Flags: in-testsuite?
Group: core-security → dom-core-security

Jan, can you take a look, please? It looks like mOpenDBRequest is a weak pointer.

Keywords: sec-high

Yaron, can you take a look ?

Flags: needinfo?(ytausky)
Attached file testcase.html —

The attached testcase is not fully reduced. However, further reductions significantly reduce the reliability of triggering the issue. Additionally, the testcase may take a minute or two to reproduce.

I can't pick it up right now, I'm already pretty occupied...

Flags: needinfo?(ytausky)

Jason, does one have to host the testcase somewhere to reproduce? There's some sort of redirection/cookies problem on the bmoattachments link.

Flags: needinfo?(jkratzer)

Andrew, yes it appears so. I've had the best luck reproducing this issue by starting a local webserver in the testcase directory. In my tests, it takes just over a minute of reloads to trigger.

Flags: needinfo?(jkratzer)

Tom, please take this.

Assignee: nobody → shes050117
Priority: -- → P1

I couldn't reproduce that on either rev "539451:57dab2bd30df" or rev "532859:4572f6055a6a" on my Linux machine.

Jason, would you mind elaborating more about how to reproduce that by the attachment? Thanks!

Or could you point out the difference between step from me and you? I wonder if I did something wrong.
What I did is:

  1. Build an ASAN debug build by following the instruction here [1] (basically only the mozconfig file).
  2. Launch a python HTTP server by "python -m SimpleHTTPServer"
  3. Run the build and go to the localhost:8000/testcase.html

One thing might be worth to mention is that I keep seeing these warning messages while running the testcase:

"WARNING: CheckInnerWindowCorrectness()" (only on rev 4572f6055a6a).

"WARNING: Suboptimal indexes for the SQL statement 0x612000b008c0 (http://mzl.la/1FuID0j).: file /home/shes050117/Work/mozilla-central/storage/mozStoragePrivateHelpers.cpp, line 108" (On both rev)

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Testing/Firefox_and_Address_Sanitizer#Building_Firefox

Flags: needinfo?(jkratzer)
Attached file prefs-default-e10s.js —
Flags: needinfo?(jkratzer)

Tom, I'm still able to reproduce the issue using the following process. Please note that a build with --enable-fuzzing is also required.

Testcase takes 30-60 seconds to reproduce.

Please NI me if you still can't reproduce it.

(In reply to Jason Kratzer [:jkratzer] from comment #10)

Tom, I'm still able to reproduce the issue using the following process. Please note that a build with --enable-fuzzing is also required.

"--enable-fuzzing" should be the thing which I missed. I'll let you know if I still cannot. Thanks!

(In reply to Jason Kratzer [:jkratzer] from comment #10)
I can reproduce by your comment. Thanks!

So, on the child process, VersionChangeTransaction received a "complete" message. Then, a delete message to FactoryRequest received. However, the VersionChangeTransaction was blocked by [1]. So, SetTransaction() was called after that. (Then, OpenDBRequest is UAF)

[1] https://searchfox.org/mozilla-central/rev/b9da45f63cb567244933c77b2c7e827a057d3f9b/dom/indexedDB/IDBTransaction.cpp#737

(In reply to Tom Tung [:tt, :ttung] from comment #13)

[1] https://searchfox.org/mozilla-central/rev/b9da45f63cb567244933c77b2c7e827a057d3f9b/dom/indexedDB/IDBTransaction.cpp#737

That was a wrong address. Below one is correct [2].

[2] https://searchfox.org/mozilla-central/rev/b9da45f63cb567244933c77b2c7e827a057d3f9b/dom/indexedDB/IDBTransaction.cpp#773

So, the test is doing sync XHR send in the event listener of the complete event. And, that breaks the sequence of IDBVersionChangeTransaction and IDBFactoryOpenDBRequest stuff.

I'm working on a temporary fix.
For that, I'm thinking of doing:

  • Extend the lifetime of the mOpenDBRequest of the IDBVersionChangeTransactionChild by holding that during dispatching the event.
    However, I got another issue and it looks like a double free:

AddressSanitizer: SEGV on unknown address 0x000000000000

#0 0x7f09c97ea0f6 in mozilla::DOMEventTargetHelper::Release() /home/shes050117/Work/mozilla-central/objdir-ff-asan/dist/include/mozilla/Assertions.h
#1 0x7f09cb9c737d in Release /home/shes050117/Work/mozilla-central/dom/indexedDB/IDBRequest.cpp:363:1
#2 0x7f09cb9c737d in mozilla::dom::IDBOpenDBRequest::Release() /home/shes050117/Work/mozilla-central/dom/indexedDB/IDBRequest.cpp:509
#3 0x7f09c2292143 in ~nsCOMPtr /home/shes050117/Work/mozilla-central/objdir-ff-asan/dist/include/nsCOMPtr.h:441:7
#4 0x7f09c2292143 in mozilla::WidgetEvent::~WidgetEvent() /home/shes050117/Work/mozilla-central/objdir-ff-asan/dist/include/mozilla/BasicEvents.h:500
#5 0x7f09cc9cd008 in mozilla::WidgetEvent::~WidgetEvent() /home/shes050117/Work/mozilla-central/objdir-ff-asan/dist/include/mozilla/BasicEvents.h:500:26
#6 0x7f09c9913539 in mozilla::dom::Event::~Event() /home/shes050117/Work/mozilla-central/dom/events/Event.cpp:113:5
#7 0x7f09cb9faed8 in ~IDBVersionChangeEvent /home/shes050117/Work/mozilla-central/dom/indexedDB/IDBEvents.h:94:29
#8 0x7f09cb9faed8 in mozilla::dom::IDBVersionChangeEvent::~IDBVersionChangeEvent() /home/shes050117/Work/mozilla-central/dom/indexedDB/IDBEvents.h:94
#9 0x7f09bf0cfec5 in SnowWhiteKiller::MaybeKillObject(SnowWhiteKiller::SnowWhiteObject&) /home/shes050117/Work/mozilla-central/xpcom/base/nsCycleCollector.cpp:2428:29
#10 0x7f09bf0c1fea in SnowWhiteKiller::~SnowWhiteKiller() /home/shes050117/Work/mozilla-central/xpcom/base/nsCycleCollector.cpp:2416:7
#11 0x7f09bf093744 in nsCycleCollector::FreeSnowWhite(bool) /home/shes050117/Work/mozilla-central/xpcom/base/nsCycleCollector.cpp:2607:3
#12 0x7f09bf09d997 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) /home/shes050117/Work/mozilla-central/xpcom/base/nsCycleCollector.cpp:3582:3
#13 0x7f09bf09cbdf in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) /home/shes050117/Work/mozilla-central/xpcom/base/nsCycleCollector.cpp:3411:9
#14 0x7f09bf0a3d24 in nsCycleCollector_collect(nsICycleCollectorListener*) /home/shes050117/Work/mozilla-central/xpcom/base/nsCycleCollector.cpp:3947:21
#15 0x7f09c4f9326b in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) /home/shes050117/Work/mozilla-central/dom/base/nsJSEnvironment.cpp:1432:3
#16 0x7f09c84bef18 in mozilla::dom::FuzzingFunctions_Binding::cycleCollect(JSContext*, unsigned int, JS::Value*) /home/shes050117/Work/mozilla-central/objdir-ff-asan/dom/bindings/FuzzingFunctionsBinding.c
#17 0x7f09d233dc15 in CallJSNative(JSContext*, bool ()(JSContext, unsigned int, JS::Value*), JS::CallArgs const&) /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter.cpp:443:13
#18 0x7f09d230f3d9 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter.cpp:535:12
#19 0x7f09d22ed21e in CallFromStack /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter.cpp:594:10
#20 0x7f09d22ed21e in Interpret(JSContext*, js::RunState&) /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter.cpp:3082
#21 0x7f09d22cf244 in js::RunScript(JSContext*, js::RunState&) /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter.cpp:423:10
#22 0x7f09d230f383 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter.cpp:563:13
#23 0x7f09d2312089 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /home/shes050117/Work/mozilla-central/js/src/vm/Interpreter
#24 0x7f09d3582619 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /home/shes050117/Work/mozilla-central/js/src/jsapi.cpp:2│2396 MaybeCollectGarbageOnIPCMessage();
647:10
#25 0x7f09c810e610 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /home/shes050117/Work/mozilla-central/objdir-ff-asan/dom/bind
#26 0x7f09c9917997 in void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::C
#27 0x7f09c98a885d in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) /home/shes050117/Work/mozilla-central/do
#28 0x7f09c98aa890 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /home/shes050117/Wo│2404 IDBDatabase* database = mTransaction->Database();
#29 0x7f09c9915624 in HandleEvent /home/shes050117/Work/mozilla-central/objdir-ff-asan/dist/include/mozilla/EventListenerManager.h:355:5
#30 0x7f09c9915624 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /home/shes050117/Work/mozilla-central/dom/events/EventDispatcher.cpp:349
#31 0x7f09c9881ac4 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreat
#32 0x7f09c98893e8 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::do│2410 if (NS_FAILED(aResult)) {
m::EventTarget*>) /home/shes050117/Work/mozilla-central/dom/events/EventDispatcher.cpp:1046:11
#33 0x7f09c9893e76 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports
, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) /home/shes050117/Work/mozilla-central/dom/events│2412 }
/EventDispatcher.cpp
#34 0x7f09c9822a50 in mozilla::DOMEventTargetHelper::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) /home/shes050117/Work/mozilla-central/dom/events/DOMEventTargetHel│2414 MOZ_ASSERT(mOpenDBRequest);
per.cpp:166:17
#35 0x7f09c98bffa2 in mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&, mozilla::ErrorResult&) /home/shes050117/Work/mozilla-central/dom/events/EventTarget.cpp:184:13
#36 0x7f09cb90052f in mozilla::dom::indexedDB::(anonymous namespace)::DispatchSuccessEvent(mozilla::dom::indexedDB::(anonymous namespace)::ResultHelper*, mozilla::dom::Event*) /home/shes050117/Work/mozill
#37 0x7f09cb8fe8d6 in mozilla::dom::indexedDB::BackgroundFactoryRequestChild::HandleResponse(mozilla::dom::indexedDB::OpenDatabaseRequestResponse const&) /home/shes050117/Work/mozilla-central/dom/indexedD
#38 0x7f09cb90251e in mozilla::dom::indexedDB::BackgroundFactoryRequestChild::Recv__delete__(mozilla::dom::indexedDB::FactoryRequestResponse const&) /home/shes050117/Work/mozilla-central/dom/indexedDB/Act
#39 0x7f09c1d93ce0 in mozilla::dom::indexedDB::PBackgroundIDBFactoryRequestChild::OnMessageReceived(IPC::Message const&) /home/shes050117/Work/mozilla-central/objdir-ff-asan/ipc/ipdl/PBackgroundIDBFactoryRequestChild.cpp:115:20

  • Another approach: dispatch an abort error for the IDBFactoryRequestChild if the Recv_delete is processed before the compelete event.
    It seems work because the fuzzing test runs to the end, but I'm not sure if it's a good approach.

(In reply to Tom Tung [:tt, :ttung] from comment #15)
The problem was RefPtr<T>::swap(), but it wouldn't call T::AddRef(). Using the assignment operator directory avoid the issue.

The patch tries to use a RefPtr to extend the lifetime of the IDBOpenDBRequest.

After applying my patch, the fuzzy test won't crash, but won't finish at least in 10 min. Jason, is that expected? (I don't find a finish function, so I guess it's expected?)

Flags: needinfo?(jkratzer)

(In reply to Tom Tung [:tt, :ttung] from comment #16)

This would be a temporary fix. I guess we should have a follow-up bug to guarantee the order.

(In reply to Tom Tung [:tt, :ttung] from comment #17)

(In reply to Tom Tung [:tt, :ttung] from comment #15)
The problem was RefPtr<T>::swap(), but it wouldn't call T::AddRef(). Using the assignment operator directory avoid the issue.

The patch tries to use a RefPtr to extend the lifetime of the IDBOpenDBRequest.

After applying my patch, the fuzzy test won't crash, but won't finish at least in 10 min. Jason, is that expected? (I don't find a finish function, so I guess it's expected?)

Tom, the testcase is not supposed to close unless one of the following conditions is hit:

  • the onerror event handler is triggered on line 1147
  • the onblocked event handler is triggered on line 1296
  • the onsuccess event handler is triggered on line 1736

As I mentioned this testcase is not fully reduced and some of the event callbacks have been merged due to the reduction process.

Flags: needinfo?(jkratzer)

(In reply to Jason Kratzer [:jkratzer] from comment #19)

Tom, the testcase is not supposed to close unless one of the following conditions is hit:

  • the onerror event handler is triggered on line 1147
  • the onblocked event handler is triggered on line 1296
  • the onsuccess event handler is triggered on line 1736

As I mentioned this testcase is not fully reduced and some of the event callbacks have been merged due to the reduction process.

I see, so it completes when window.close() is called. Thanks!

(In reply to Tom Tung [:tt, :ttung] from comment #20)

(In reply to Jason Kratzer [:jkratzer] from comment #19)

Tom, the testcase is not supposed to close unless one of the following conditions is hit:

  • the onerror event handler is triggered on line 1147

I can end the test by running into this(dispatching an error event for the FactoryRequest)

  • the onblocked event handler is triggered on line 1296
  • the onsuccess event handler is triggered on line 1736

These two are event handlers of the o[432]. However, it seems that that was reduced so that it isn't defined in the testcase.html.

So, it seems it's reasonable to complete the test.

Attachment #9065387 - Attachment description: Bug 1539219 - Use RefPtr for IDBOpenDBRequest before using it; → Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;
Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(shes050117)
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

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

https://hg.mozilla.org/integration/autoland/rev/8a1f1939b4e3ede0a80155917244ab5a7c36024c
https://hg.mozilla.org/mozilla-central/rev/8a1f1939b4e3

Patch applies cleanly to beta. Do you want to uplift it to beta?

This should also go to beta. I will request it to be uplifted. Thanks!

Flags: needinfo?(shes050117)

Comment on attachment 9065387 [details]
Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;

Beta/Release Uplift Approval Request

  • User impact if declined: UAF crash if visiting script has an XHR sync send (or any sync API) on the handler of the IDB version change transaction complete event.
  • 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): Ensure a lifetime of IDB request for the version change transaction class on the child process until the transaction class cleans it up. So, I don't think it is risky.
  • String changes made/needed:
Attachment #9065387 - Flags: approval-mozilla-beta?

This is sec-high but landed without sec-approval :(

What other branches are affected?

Flags: needinfo?(shes050117)

(In reply to Julien Cristau [:jcristau] from comment #25)

This is sec-high but landed without sec-approval :(

What other branches are affected?

I'm really sorry about forgetting to request. It's only on the m-c. I'm going to request it.

Flags: needinfo?(shes050117)

Comment on attachment 9065387 [details]
Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: I'm really sorry that I pushed the patch without asking a sec-approval in advance.

The change is small and doesn't show how to reproduce the issue.

  • 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?: esr60
  • 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?: By only reading the patch, it's not easy to figure out the way to attack. The patch only ensure the lifetime of a pointer (mOpenDBRequest) be long enough.
  • How likely is this patch to cause regressions; how much testing does it need?: It ensures the request lives until the request is null'd. The possibility of causing regression is low.
Attachment #9065387 - Flags: sec-approval?

Does this need an ESR60 request too? AFAICT, the affected code is present there too?

Flags: needinfo?(shes050117)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #29)

Does this need an ESR60 request too? AFAICT, the affected code is present there too?

Yes, I thought I need to get a security approval before requesting that. However, I can actually request that to ESR 60 and release now.

Flags: needinfo?(shes050117)

Comment on attachment 9065387 [details]
Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: If there is a sync XHR on the handler of an IDBVersionChangeTransaction onCompelete event, it would make it live longer than the opening IDB request. And, the mIDBOpenRequest inside the transaction object has a chance to be accessed (being null) after the real object has been free'd.
  • User impact if declined: Potentially hit the UAF if there is a script doing that.
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The patch ensures the IDBOpenDBRequest be held during dispatching the complete event so that there is no UAF. So, in general cases (the opening DB request usually lives longer than the version change transaction), that shouldn't have any risk.
  • String or UUID changes made by this patch:
Attachment #9065387 - Flags: approval-mozilla-esr60?

(In reply to Tom Tung [:tt, :ttung] from comment #30)

Yes, I thought I need to get a security approval before requesting that. However, I can actually request that to ESR 60 and release now.

Ryan just told/reminded me that the request for release is not needed.

Comment on attachment 9065387 [details]
Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;

uaf fix, approved for 68.0b7

Attachment #9065387 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9065387 [details]
Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;

After the fact sec-approval+ given. I'll let release management approve the ESR60 checkin but we should take it.

Attachment #9065387 - Flags: sec-approval? → sec-approval+
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
QA Whiteboard: [qa-triaged]

Hi Jason, can you please recheck this issue in our latest Beta and Nightly, I'm having some python issues and it's taking a bit long to sort it out, can you let us know if the issue still occurs in our latest builds ? Thank you

Flags: needinfo?(jkratzer)

Comment on attachment 9065387 [details]
Bug 1539219 - Get RefPtr for IDBOpenDBRequest before using it;

Fixes a s-s IndexedDB crash. Approved for 60.8esr.

Attachment #9065387 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

(In reply to Rares Doghi from comment #36)

Hi Jason, can you please recheck this issue in our latest Beta and Nightly, I'm having some python issues and it's taking a bit long to sort it out, can you let us know if the issue still occurs in our latest builds ? Thank you

Rares, I can no longer reproduce this issue using the latest nightly or beta.

Flags: needinfo?(jkratzer)

Well the Fix for esr60 came really fast, can you please check that one as well and I will mark this issue in the mean time as verified for beta and nightly. Thank you Jason

Flags: needinfo?(jkratzer)

(In reply to Rares Doghi from comment #40)

Well the Fix for esr60 came really fast, can you please check that one as well and I will mark this issue in the mean time as verified for beta and nightly. Thank you Jason

I can no longer reproduce this under ESR rev 409a5e270fa6.

Flags: needinfo?(jkratzer)
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main68+][adv-esr60.8+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: