1512567 - heap-use-after-free in [@ mozilla::a11y::DocAccessibleChildBase::ActorDestroy]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Tyson Smith [:tsmith]]

I am currently reducing a testcase and will attach it when complete.

==19309==ERROR: AddressSanitizer: heap-use-after-free on address 0x6140001f7bc0 at pc 0x7fef52793fa3 bp 0x7ffc3cb0a080 sp 0x7ffc3cb0a078
WRITE of size 8 at 0x6140001f7bc0 thread T0 (file:// Content)
    #0 0x7fef52793fa2 in SetIPCDoc src/accessible/generic/DocAccessible.h:556:57
    #1 0x7fef52793fa2 in mozilla::a11y::DocAccessibleChildBase::ActorDestroy(mozilla::ipc::IProtocol::ActorDestroyReason) src/obj-firefox/dist/include/mozilla/a11y/DocAccessibleChildBase.h:55
    #2 0x7fef465bb093 in mozilla::dom::PBrowserChild::DestroySubtree(mozilla::ipc::IProtocol::ActorDestroyReason) src/obj-firefox/ipc/ipdl/PBrowserChild.cpp:5615:24
    #3 0x7fef465ba8ac in mozilla::dom::PBrowserChild::Send__delete__(mozilla::dom::PBrowserChild*) src/obj-firefox/ipc/ipdl/PBrowserChild.cpp:1758:14
    #4 0x7fef4dee73df in mozilla::dom::TabChild::DelayedDeleteRunnable::Run() src/dom/ipc/TabChild.cpp:318:17
    #5 0x7fef445460d8 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1157:14
    #6 0x7fef4454ee8d in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:468:10
    #7 0x7fef457d31bf in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
    #8 0x7fef456c5fee in RunInternal src/ipc/chromium/src/base/message_loop.cc:314:10
    #9 0x7fef456c5fee in RunHandler src/ipc/chromium/src/base/message_loop.cc:307
    #10 0x7fef456c5fee in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:289
    #11 0x7fef4e88f313 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #12 0x7fef533034ae in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:915:20
    #13 0x7fef456c5fee in RunInternal src/ipc/chromium/src/base/message_loop.cc:314:10
    #14 0x7fef456c5fee in RunHandler src/ipc/chromium/src/base/message_loop.cc:307
    #15 0x7fef456c5fee in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:289
    #16 0x7fef533024fe in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:753:34
    #17 0x56467dfcf864 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:49:28
    #18 0x56467dfcf864 in main src/browser/app/nsBrowserApp.cpp:265
    #19 0x7fef6847982f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #20 0x56467def4eec in _start (/home/ubuntu/firefox/firefox+0x2deec)

0x6140001f7bc0 is located 384 bytes inside of 400-byte region [0x6140001f7a40,0x6140001f7bd0)
freed by thread T0 (file:// Content) here:
    #0 0x56467df9ca12 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7fef44344b11 in SnowWhiteKiller::~SnowWhiteKiller() src/xpcom/base/nsCycleCollector.cpp:2416:7
    #2 0x7fef443431c3 in nsCycleCollector::FreeSnowWhite(bool) src/xpcom/base/nsCycleCollector.cpp:2607:3
    #3 0x7fef4434fa92 in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) src/xpcom/base/nsCycleCollector.cpp:3578:3
    #4 0x7fef4434ed25 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) src/xpcom/base/nsCycleCollector.cpp:3407:9
    #5 0x7fef44353ca6 in nsCycleCollector_collect(nsICycleCollectorListener*) src/xpcom/base/nsCycleCollector.cpp:3942:21
    #6 0x7fef48a440ba in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) src/dom/base/nsJSEnvironment.cpp:1413:3
    #7 0x7fef4b3cb428 in mozilla::dom::FuzzingFunctions_Binding::cycleCollect(JSContext*, unsigned int, JS::Value*) src/obj-firefox/dom/bindings/FuzzingFunctionsBinding.cpp:66:3
    #8 0x7fef535d284d in CallJSNative src/js/src/vm/Interpreter.cpp:443:13
    #9 0x7fef535d284d in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:535
    #10 0x7fef535bbfa3 in CallFromStack src/js/src/vm/Interpreter.cpp:594:10
    #11 0x7fef535bbfa3 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3320
    #12 0x7fef5359eec6 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:423:10
    #13 0x7fef535d31f1 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:563:13
    #14 0x7fef535d4e72 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:606:8
    #15 0x7fef541461c6 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/jsapi.cpp:2651:10
    #16 0x7fef4b0ccf79 in mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:265:37
    #17 0x7fef4c3672d9 in void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:363:12
    #18 0x7fef4c364569 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) src/dom/events/JSEventHandler.cpp:205:12
    #19 0x7fef4c317dca in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1044:51
    #20 0x7fef4c31a3a3 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1239:17
    #21 0x7fef4c2fad76 in HandleEvent src/obj-firefox/dist/include/mozilla/EventListenerManager.h:350:5
    #22 0x7fef4c2fad76 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:346

previously allocated by thread T0 (file:// Content) here:
    #0 0x56467df9cd93 in malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x56467dfd079d in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:68:15
    #2 0x7fef5263d5c0 in operator new src/obj-firefox/dist/include/mozilla/mozalloc.h:131:10
    #3 0x7fef5263d5c0 in mozilla::a11y::DocManager::CreateDocOrRootAccessible(nsIDocument*) src/accessible/base/DocManager.cpp:453
    #4 0x7fef52640b16 in HandleDOMDocumentLoad src/accessible/base/DocManager.cpp:370:14
    #5 0x7fef52640b16 in mozilla::a11y::DocManager::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/accessible/base/DocManager.cpp:249
    #6 0x7fef46f18005 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1235:3
    #7 0x7fef46f16bec in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:794:14
    #8 0x7fef46f12538 in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:693:9
    #9 0x7fef46f14e8e in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:589:5
    #10 0x7fef46f16714 in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp
    #11 0x7fef447d0ef7 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:586:22
    #12 0x7fef447cc2b6 in mozilla::net::nsLoadGroup::Cancel(nsresult) src/netwerk/base/nsLoadGroup.cpp:231:11
    #13 0x7fef46f11a93 in nsDocLoader::Stop() src/uriloader/base/nsDocLoader.cpp:220:36
    #14 0x7fef46f118d4 in nsDocLoader::Stop() src/uriloader/base/nsDocLoader.cpp:218:3
    #15 0x7fef5246a6a7 in Stop src/docshell/base/nsDocShell.h:214:25
    #16 0x7fef5246a6a7 in nsDocShell::Stop(unsigned int) src/docshell/base/nsDocShell.cpp:4739
    #17 0x7fef524968a6 in nsDocShell::InternalLoad(nsIURI*, nsIURI*, mozilla::Maybe<nsCOMPtr<nsIURI> > const&, bool, bool, bool, nsIURI*, unsigned int, nsIPrincipal*, nsIPrincipal*, unsigned int, nsTSubstring<char16_t> const&, nsTSubstring<char> const&, nsTSubstring<char16_t> const&, nsIInputStream*, nsIInputStream*, unsigned int, nsISHEntry*, bool, nsTSubstring<char16_t> const&, nsIDocShell*, nsIURI*, nsIDocShell**, nsIRequest**) src/docshell/base/nsDocShell.cpp
    #18 0x7fef52527afb in nsDocShell::OnLinkClickSync(nsIContent*, nsIURI*, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsIInputStream*, nsIInputStream*, bool, nsIDocShell**, nsIRequest**, bool, nsIPrincipal*) src/docshell/base/nsDocShell.cpp:12786:7
    #19 0x7fef4c5ef497 in mozilla::dom::HTMLFormElement::SubmitSubmission(mozilla::dom::HTMLFormSubmission*) src/dom/html/HTMLFormElement.cpp:712:23
    #20 0x7fef4c5ed863 in mozilla::dom::HTMLFormElement::DoSubmit(mozilla::WidgetEvent*) src/dom/html/HTMLFormElement.cpp:593:10
    #21 0x7fef4c5ea705 in DoSubmitOrReset src/dom/html/HTMLFormElement.cpp:515:12
    #22 0x7fef4c5ea705 in mozilla::dom::HTMLFormElement::Submit(mozilla::ErrorResult&) src/dom/html/HTMLFormElement.cpp:230
    #23 0x7fef4b5c54a6 in mozilla::dom::HTMLFormElement_Binding::submit(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLFormElement*, JSJitMethodCallArgs const&) src/obj-firefox/dom/bindings/HTMLFormElementBinding.cpp:853:9

Comment 1

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Comment 2

[Tyson Smith [:tsmith]]

Attachment: unreduced.html

Comment 3

[Tyson Smith [:tsmith]]

For the record the most reliable method I've found to reproduce this is:
1) Serve testcase.html from a local web server
2) open multiple instances of the testcase (4 or so)
3) let them run for a few seconds and close all but one tab
4) go to 2) until crash is triggered

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1512567 - Check that we didn't already create an IPCDoc for the DocAccessible. r?Jamie!

Comment 5

[Eitan Isaacson [:eeejay]]

Comment on attachment 9045482 [details]
Bug 1512567 - Check that we didn't already create an IPCDoc for the DocAccessible. r?Jamie!

Security Approval Request

How easily could an exploit be constructed based on the patch?

It would be hard. I had a very hard time reproducing this. Had to repeat the steps numerous times.

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?

beta, release

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?

I think this patch will apply to all branches.

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

I think it needs a moderate amount of time in nightly before uplifting.

Comment 6

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

This doesn't affect ESR60?

sec-approval+ for trunk. We'll want a beta patch nominated after bake time.

Comment 7

[Eitan Isaacson [:eeejay]]

(In reply to Al Billings [:abillings] from comment #6)

This doesn't affect ESR60?

According to bug 1518960, I think this surfaced in 65. I can test with ESR.

sec-approval+ for trunk. We'll want a beta patch nominated after bake time.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Has this baked enough?

Comment 9

[Eitan Isaacson [:eeejay]]

Just landed it.. sorry for the delay.

Comment 10

[Eitan Isaacson [:eeejay]]

Attachment: Check that we didn't already create an IPCDoc for the DocAccessible. r?Jamie!

Beta patch

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/8e6506978820

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8e6506978820

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 14

[Eitan Isaacson [:eeejay]]

Comment on attachment 9046750 [details] [diff] [review]
Check that we didn't already create an IPCDoc for the DocAccessible. r?Jamie!

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: None
• User impact if declined: Possible UAF
• Is this code covered by automated tests?: Yes
• 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: Medium
• Why is the change risky/not risky? (and alternatives if risky): We have good test coverage for this. On the other hand the code path that leads to this UAF is not easily reproducible, so its testing in Nightly is limited.
• String changes made/needed:

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9046750 [details] [diff] [review]
Check that we didn't already create an IPCDoc for the DocAccessible. r?Jamie!

Fix for potential UAF, OK for beta 13 uplift.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/6982cfd315d0

Comment 17

[Cornel Ionce [:noni] [Hubs QA]]

(In reply to Eitan Isaacson [:eeejay] from comment #14)

• Is this code covered by automated tests?: Yes
• Needs manual test from QE?: No

Per comment 14, this is covered by tests and don't require manual testing.