Found with m-c: 20190802-37229cef2cc7

At the moment no reproducable test case is available.

==2769==ERROR: AddressSanitizer: heap-use-after-free on address 0x61a00016fd14 at pc 0x7f86c680ad96 bp 0x7ffe2df7f8f0 sp 0x7ffe2df7f8e8
WRITE of size 1 at 0x61a00016fd14 thread T0
    #0 0x7f86c680ad95 in ~AutoUseNewTab /src/dom/ipc/BrowserParent.h:975:30
    #1 0x7f86c680ad95 in mozilla::dom::ContentParent::RecvCreateWindow(mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserParent*, unsigned int const&, bool const&, bool const&, bool const&, mozilla::Maybe<mozilla::ipc::URIParams> const&, nsTString<char> const&, float const&, IPC::Principal const&, nsIContentSecurityPolicy*, nsIReferrerInfo*, std::function<void (mozilla::dom::CreatedWindowInfo const&)>&&) /src/dom/ipc/ContentParent.cpp:4975
    #2 0x7f86bf3858b7 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PContentParent.cpp:9312:57
    #3 0x7f86befb0326 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2184:25
    #4 0x7f86befab08b in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2108:9
    #5 0x7f86befad647 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1955:3
    #6 0x7f86befae4d7 in mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1986:13
    #7 0x7f86bddc94e0 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1224:14
    #8 0x7f86bddcf8f8 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #9 0x7f86befb970f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #10 0x7f86beeb67b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #11 0x7f86beeb67b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #12 0x7f86beeb67b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #13 0x7f86c711f099 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #14 0x7f86cad4efe0 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:276:30
    #15 0x7f86caff88a3 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4631:22
    #16 0x7f86caffa9c0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4766:8
    #17 0x7f86caffc3ce in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4847:21
    #18 0x5624af135dd4 in do_main /src/browser/app/nsBrowserApp.cpp:213:22
    #19 0x5624af135dd4 in main /src/browser/app/nsBrowserApp.cpp:295
    #20 0x7f86df5e1b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #21 0x5624af0576ac in _start (/home/worker/builds/m-c-20190802215241-fuzzing-asan-opt/firefox+0x456ac)

0x61a00016fd14 is located 1172 bytes inside of 1248-byte region [0x61a00016f880,0x61a00016fd60)
freed by thread T0 here:
    #0 0x5624af102d42 in __interceptor_free /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
    #1 0x7f86bdc27fc2 in MaybeKillObject /src/xpcom/base/nsCycleCollector.cpp:2429:29
    #2 0x7f86bdc27fc2 in SnowWhiteKiller::Visit(nsPurpleBuffer&, nsPurpleBufferEntry*) /src/xpcom/base/nsCycleCollector.cpp:2459
    #3 0x7f86bdc05c12 in void nsPurpleBuffer::VisitEntries<SnowWhiteKiller>(SnowWhiteKiller&) /src/xpcom/base/nsCycleCollector.cpp:941:23
    #4 0x7f86bdc06e59 in nsCycleCollector::FreeSnowWhiteWithBudget(js::SliceBudget&) /src/xpcom/base/nsCycleCollector.cpp:2624:14
    #5 0x7f86c00d2108 in AsyncFreeSnowWhite::Run() /src/js/xpconnect/src/XPCJSRuntime.cpp:146:9
    #6 0x7f86bdde415c in Run /src/xpcom/threads/nsThreadUtils.cpp:331:22
    #7 0x7f86bdde415c in IdleRunnableWrapper::TimedOut(nsITimer*, void*) /src/xpcom/threads/nsThreadUtils.cpp:337
    #8 0x7f86bddb688b in nsTimerImpl::Fire(int) /src/xpcom/threads/nsTimerImpl.cpp:561:7
    #9 0x7f86bddb6109 in nsTimerEvent::Run() /src/xpcom/threads/TimerThread.cpp:260:11
    #10 0x7f86bddc94e0 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1224:14
    #11 0x7f86bddcf8f8 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #12 0x7f86ca4e270c in SpinEventLoopUntil<mozilla::ProcessFailureBehavior::ReportToCaller, (lambda at /builds/worker/workspace/build/src/xpfe/appshell/nsXULWindow.cpp:2061:24)> /src/obj-firefox/dist/include/nsThreadUtils.h:348:25
    #13 0x7f86ca4e270c in nsXULWindow::CreateNewContentWindow(int, nsIRemoteTab*, mozIDOMWindowProxy*, unsigned long, nsIXULWindow**) /src/xpfe/appshell/nsXULWindow.cpp:2061
    #14 0x7f86cad51373 in nsAppStartup::CreateChromeWindow(nsIWebBrowserChrome*, unsigned int, nsIRemoteTab*, mozIDOMWindowProxy*, unsigned long, bool*, nsIWebBrowserChrome**) /src/toolkit/components/startup/nsAppStartup.cpp:618:18
    #15 0x7f86caf4c266 in nsWindowWatcher::CreateChromeWindow(nsTSubstring<char> const&, nsIWebBrowserChrome*, unsigned int, nsIRemoteTab*, mozIDOMWindowProxy*, unsigned long, nsIWebBrowserChrome**) /src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:412:33
    #16 0x7f86caf4cc9d in nsWindowWatcher::OpenWindowWithRemoteTab(nsIRemoteTab*, nsTSubstring<char> const&, bool, float, unsigned long, bool, nsIRemoteTab**) /src/toolkit/components/windowwatcher/nsWindowWatcher.cpp:526:3
    #17 0x7f86c6808910 in mozilla::dom::ContentParent::CommonCreateWindow(mozilla::dom::PBrowserParent*, bool, unsigned int const&, bool const&, bool const&, bool const&, nsIURI*, nsTString<char> const&, float const&, unsigned long, nsTString<char16_t> const&, nsresult&, nsCOMPtr<nsIRemoteTab>&, bool*, int&, nsIPrincipal*, nsIReferrerInfo*, bool, nsIContentSecurityPolicy*) /src/dom/ipc/ContentParent.cpp:4830:22
    #18 0x7f86c680a0a9 in mozilla::dom::ContentParent::RecvCreateWindow(mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserParent*, unsigned int const&, bool const&, bool const&, bool const&, mozilla::Maybe<mozilla::ipc::URIParams> const&, nsTString<char> const&, float const&, IPC::Principal const&, nsIContentSecurityPolicy*, nsIReferrerInfo*, std::function<void (mozilla::dom::CreatedWindowInfo const&)>&&) /src/dom/ipc/ContentParent.cpp:4943:39
    #19 0x7f86bf3858b7 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PContentParent.cpp:9312:57
    #20 0x7f86befb0326 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2184:25
    #21 0x7f86befab08b in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2108:9
    #22 0x7f86befad647 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1955:3

previously allocated by thread T0 here:
    #0 0x5624af1030c3 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
    #1 0x5624af137e2d in moz_xmalloc /src/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f86c67f2ac7 in operator new /src/obj-firefox/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7f86c67f2ac7 in mozilla::dom::ContentParent::RecvConstructPopupBrowser(mozilla::ipc::ManagedEndpoint<mozilla::dom::PBrowserParent>&&, mozilla::dom::IdType<mozilla::dom::BrowserParent> const&, mozilla::dom::IPCTabContext const&, mozilla::dom::BrowsingContext*, unsigned int const&) /src/dom/ipc/ContentParent.cpp:3336
    #4 0x7f86bf36fa11 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PContentParent.cpp:5810:57
    #5 0x7f86befb0326 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2184:25
    #6 0x7f86befab08b in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2108:9
    #7 0x7f86befad647 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1955:3
    #8 0x7f86befae4d7 in mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1986:13
    #9 0x7f86bddc94e0 in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1224:14
    #10 0x7f86bddcf8f8 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:486:10
    #11 0x7f86befb970f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:88:21
    #12 0x7f86beeb67b2 in RunInternal /src/ipc/chromium/src/base/message_loop.cc:315:10
    #13 0x7f86beeb67b2 in RunHandler /src/ipc/chromium/src/base/message_loop.cc:308
    #14 0x7f86beeb67b2 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:290
    #15 0x7f86c711f099 in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:137:27
    #16 0x7f86cad4efe0 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:276:30
    #17 0x7f86caff88a3 in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4631:22
    #18 0x7f86caffa9c0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4766:8
    #19 0x7f86caffc3ce in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4847:21
    #20 0x5624af135dd4 in do_main /src/browser/app/nsBrowserApp.cpp:213:22
    #21 0x5624af135dd4 in main /src/browser/app/nsBrowserApp.cpp:295
    #22 0x7f86df5e1b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

It looks like BrowserParent::AutoUseNewTab has a raw pointer to a browser parent, and somehow the browser parent has gone away before we get to the end of RecvCreateWindow. The browser parent was able to be destroyed because we spun the event loop in nsXULWindow::CreateNewContentWindow. Maybe the AutoUseNewTab reference should be strong? And/or the BrowserParent* local variable newTab. I'm not sure why UAFs from newTab don't show up first.

(In reply to Andrew McCreight [:mccr8] from comment #1)

Maybe the AutoUseNewTab reference should be strong? And/or the BrowserParent* local variable newTab.

Making the AutoUseNewTab reference strong is probably a good idea to prevent future footguns, but the reference in RecvCreateWindow really needs to be strong too. It's kept alive over the same event loop spinning call. Though perhaps if CommonCreateWindow always returns failure in this circumstance, it isn't used after it's freed. I wouldn't be happy depending on that, though.

I'm not sure why UAFs from newTab don't show up first.

Likely because we return early from a failure in CommonCreateWindow and don't hit a UAF directly in RecvCreateWindow, only in the AutoUseNewTab destructor.

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

gcp, could you take a look with Jed and see if we can find the right component / owner for this.

Neha, Nika is away this week. Anyone else on that team that can look at this?

Andrew has already started looked into this (comment 1).

Comment on attachment 9086837 [details]
Bug 1571223 - Sprinkle around some RefPtrs in ContentParent.cpp.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The basic issue here is pretty obvious, but it feels like it would be tricky to turn into an exploit. We don't even have a test case for it.
• 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?: Should be trivial.
• How likely is this patch to cause regressions; how much testing does it need?: Seems like a safe patch. It just adds some stack references.

Speaking with Release Management, this looks to be too risky for this release (69). I am approving for checkin on September 23.

Tracking to make sure we get this into 70.

Comment on attachment 9086837 [details]
Bug 1571223 - Sprinkle around some RefPtrs in ContentParent.cpp.

Beta/Release Uplift Approval Request

• User impact if declined: possible security issues
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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 just adds some strong references on the stack
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment on attachment 9086837 [details]
Bug 1571223 - Sprinkle around some RefPtrs in ContentParent.cpp.

Fix for sec-high crash issue, OK to land today and uplift for beta 9.

Landed in beta https://hg.mozilla.org/releases/mozilla-beta/rev/48e74f990086920697ec7b6d3a6ff95554de390f
Still needs to land on m-c though.

From discussion with mccr8, he will file a followup bug for the small cleanup suggested in phabricator (which won't need uplift)

The patch for central bitrotted (by bug 1579820).

I've rebased the patch (and addressed the minor review comment, which does not need to be backported to beta).

https://hg.mozilla.org/integration/autoland/rev/7627416761545e7898799f4d9be97e4f64f96749

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

Comment on attachment 9086837 [details]
Bug 1571223 - Sprinkle around some RefPtrs in ContentParent.cpp.

Sec fix, approved for 68.2esr.

Actually, this needs a rebased patch for ESR68.

I've attached a patch rebased for esr68.

https://hg.mozilla.org/releases/mozilla-esr68/rev/c21445cd531686b8f2708d28099b60c06f924cc9