2053578 - Use-after-poison via stale nsSplittableFrame::mFirstInFlow/mFirstContinuation caches in [@ nsSplittableFrame::RemoveFromFlow]

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Bugmon [:jkratzer for issues]]

nsSplittableFrame caches raw mFirstContinuation/mFirstInFlow pointers on every frame of a continuation chain (introduced to avoid O(n^2) chain walks). nsSplittableFrame::UpdateFirstContinuationAndFirstInFlowCache() guards its purge loops with if (oldCachedFirstContinuation) / if (oldCachedFirstInFlow), i.e. it assumes that if the frame's own cache is null, all following frames' caches are also null ('suffix uniformity'). Ordinary bidi/line-wrap churn violates this invariant: (1) a bidi demotion via SetPrevContinuation() clears the fluid bit on a mid-chain frame S, making it a flow head, and the purge nulls mFirstInFlow across S's whole next-in-flow suffix (W, P, Q, R, T); (2) a later mid-chain frame removal calls RemoveFromFlow(), whose SetPrevInFlow() relink repopulates the cache only from the relinked frame onward (the update loop starts at this), so R and T get mFirstInFlow = S while W, P, Q legitimately keep null; (3) when S itself is destroyed, RemoveFromFlow(S) runs W->SetPrevContinuation(...); the purge guard sees W's null cache and skips the purge, leaving R and T holding pointers to the freed, arena-poisoned frame S.

FirstInFlow()/FirstContinuation() return the cached pointer without validation and essentially every caller dereferences the result (e.g. nsLayoutUtils::FirstContinuationOrIBSplitSibling during restyling, ReflowInput's FirstInFlow()->HasAnyStateBits() during reflow, frame-property lookups), yielding a use-after-free of a destroyed frame in the PresShell frame arena. The arena recycles freed frame slots, so in a release build the dangling pointer aliases a subsequently allocated frame of attacker-influenced type — a classic frame type-confusion primitive.

The corrupted state is created entirely by unpatched code driven by plain web content (nested inline elements with mixed Hebrew/Arabic/Latin text in narrow containers, plus DOM/style mutations). The submitted crash artifact uses an 11-line read-only diagnostic patch that performs, immediately after RemoveFromFlow(), exactly the FirstInFlow()->GetStateBits() read that shipping consumers perform, making the timing-dependent consumption of the dangling pointer deterministic under ASAN; I reproduced the identical crash stack with it (the unpatched build did not crash within a 4-minute run, as natural consumption timing is heuristic).

Build Info

• Branch: main
• Revision: d4a2cc2aa48ba5e56ce90680e90038843a7298c7
• Timestamp: 2026-07-01T00:30:50+00:00

Affected Code

File: layout/generic/nsSplittableFrame.cpp, line 236-258

  nsIFrame* oldCachedFirstInFlow = mFirstInFlow;
  if (nsIFrame* prevInFlow = GetPrevInFlow()) {
    nsIFrame* newFirstInFlow = prevInFlow->FirstInFlow();
    if (oldCachedFirstInFlow != newFirstInFlow) {
      // Update the first-in-flow cache for us and our next-in-flows.
      for (nsSplittableFrame* f = this; f;
           f = reinterpret_cast<nsSplittableFrame*>(f->GetNextInFlow())) {
        f->mFirstInFlow = newFirstInFlow;   // <-- repopulates ONLY the suffix
      }                                     //     starting at `this`, creating a
    }                                       //     mixed null/non-null suffix
  } else {
    // We become the new first-in-flow due to our prev-in-flow being removed.
    if (oldCachedFirstInFlow) {             // <-- BUG: guard assumes that if our
      // It's tempting to update the first-in-flow cache for our
      // next-in-flows here, but that would result in overall O(n^2)
      // behavior when a frame list is destroyed from the front. To avoid that
      // pathological behavior, we simply purge the cached values.
      for (nsSplittableFrame* f = this; f;
           f = reinterpret_cast<nsSplittableFrame*>(f->GetNextInFlow())) {
        f->mFirstInFlow = nullptr;          //     cache is null, all later caches
      }                                     //     are null too; the purge is
    }                                       //     skipped and stale pointers to
  }                                         //     the freed flow head survive

File: layout/generic/nsSplittableFrame.cpp, line 180-208

void nsSplittableFrame::RemoveFromFlow(nsIFrame* aFrame) {
  nsIFrame* prevContinuation = aFrame->GetPrevContinuation();
  nsIFrame* nextContinuation = aFrame->GetNextContinuation();

  // The new continuation is fluid only if the continuation on both sides
  // of the removed frame was fluid
  if (aFrame->GetPrevInFlow() && aFrame->GetNextInFlow()) {
    if (prevContinuation) {
      prevContinuation->SetNextInFlow(nextContinuation);
    }
    if (nextContinuation) {
      nextContinuation->SetPrevInFlow(prevContinuation);  // <-- partial suffix
    }                                                     //     repopulation (step 2)
  } else {
    if (prevContinuation) {
      prevContinuation->SetNextContinuation(nextContinuation);
    }
    if (nextContinuation) {
      nextContinuation->SetPrevContinuation(prevContinuation);  // <-- guard-skipped
    }                                                           //     purge when the
  }                                                             //     flow head dies (step 3)
  ...
}

File: layout/generic/nsSplittableFrame.cpp, line 156-169

nsIFrame* nsSplittableFrame::FirstInFlow() const {
  if (mFirstInFlow) {
    return mFirstInFlow;   // <-- returns the stale pointer to the freed frame
  }
  // We fall back to the slow path during the frame destruction where our
  // first-in-flow cache was purged.
  auto* firstInFlow = const_cast<nsSplittableFrame*>(this);
  while (nsIFrame* prev = firstInFlow->GetPrevInFlow()) {
    firstInFlow = static_cast<nsSplittableFrame*>(prev);
  }
  MOZ_ASSERT(firstInFlow);
  return firstInFlow;
}

File: layout/base/nsLayoutUtils.cpp, line 4066-4077

nsIFrame* nsLayoutUtils::FirstContinuationOrIBSplitSibling(
    const nsIFrame* aFrame) {
  nsIFrame* result = aFrame->FirstContinuation();

  if (result->HasAnyStateBits(NS_FRAME_PART_OF_IBSPLIT)) {  // <-- example shipping
    while (auto* f = result->GetProperty(nsIFrame::IBSplitPrevSibling())) {
      result = f;   //     consumer: dereferences the cached first-continuation
    }               //     (same GetStateBits read the diagnostic patch performs)
  }
  return result;
}

Root cause: the purge loops assume that a null cache on the first following frame implies null caches on the whole suffix; the partial repopulation performed by SetPrevInFlow() during mid-chain RemoveFromFlow() relinking breaks that assumption, and FirstInFlow() then hands out the stale pointer which callers dereference.

Exploit Chain

1. Attacker serves an HTML page with nested inline elements (span/b/bdi/bdo/em) containing mixed Hebrew/Arabic/Latin text inside narrow-width containers; line wrapping creates fluid continuations and bidi resolution creates/destroys non-fluid continuations of the inline frames.
2. Script toggles dir/unicode-bidi/direction, inserts/removes <br> and elements, and forces layout after each mutation, driving continuation-chain churn.
3. A bidi demotion (SetPrevContinuation clearing NS_FRAME_IS_FLUID_CONTINUATION) turns a mid-chain frame S into a flow head; UpdateFirstContinuationAndFirstInFlowCache() purges mFirstInFlow to null across S's entire next-in-flow suffix (W, P, Q, R, T).
4. A later removal of a frame in the middle of the chain calls nsSplittableFrame::RemoveFromFlow(); its SetPrevInFlow() relink repopulates the cache only from the relinked frame onward: R.mFirstInFlow = T.mFirstInFlow = S, while W, P, Q keep null (mixed suffix).
5. DOM removal of the element destroys S; RemoveFromFlow(S) runs W->SetPrevContinuation(...), and the purge guard if (oldCachedFirstInFlow) sees W's null cache and skips the purge — R and T retain raw pointers to the freed frame S, whose arena memory is poisoned/recycled.
6. Any subsequent FirstInFlow()/FirstContinuation() consumer on R or T (restyle via nsLayoutUtils::FirstContinuationOrIBSplitSibling, reflow via ReflowInput, frame-property lookups) dereferences the freed frame — use-after-free. The PresShell arena recycles freed frame slots, so the attacker can groom the arena to place a different frame type at S's address, converting the UAF into frame type confusion and memory corruption in the content process.

Steps to Reproduce

1. Check out mozilla-central at the tested revision and apply the provided 11-line read-only diagnostic patch source_patch.diff to layout/generic/nsSplittableFrame.cpp (it performs, at the end of RemoveFromFlow(), the same FirstInFlow()->GetStateBits()/FirstContinuation()->GetStateBits() reads that shipping consumers perform, making the stale-cache dereference deterministic).
2. Build Firefox with the Linux ASAN fuzzing configuration (mozconfig.linux.asan.fuzzing).
3. Serve testcase.html over HTTP and load it with pref fuzzing.enabled=true (standard fuzzing baseline; the testcase does not use FuzzingFunctions).
4. Wait ~1-2 minutes while the seeded deterministic testcase mutates bidi text, dir/unicode-bidi styles, <br> elements, and container widths; the content process crashes with AddressSanitizer: use-after-poison in nsIFrame::GetStateBits called from nsSplittableFrame::RemoveFromFlow (nsSplittableFrame.cpp:214). Verified: crashed=true with a stack identical to the submitted crashdata.

Security Impact

• Severity: High
• Attacker capability: A dangling nsIFrame* is left in the mFirstInFlow/mFirstContinuation fields of live frames after a frame in the same continuation chain is destroyed. FirstInFlow()/FirstContinuation() hand this pointer to ubiquitous layout/restyle consumers that read state bits, walk frame-property tables, and follow embedded pointers of the freed object. Because frames are allocated from the recycling PresShell arena, the freed slot can be reoccupied by an attacker-influenced frame of a different type, yielding frame type confusion — a well-understood primitive for information disclosure and memory corruption leading toward arbitrary code execution in the content process.
• Preconditions: None beyond the victim loading an attacker-controlled web page; triggered by ordinary bidi text, line wrapping, and DOM/style mutations with no user interaction. The submitted ASAN artifact uses a minimal read-only instrumentation patch to make the dereference of the corrupted cache deterministic; the corrupted (dangling) state itself is created entirely by unpatched, content-reachable code.

ASAN Report

==92238==ERROR: AddressSanitizer: use-after-poison on address 0x764e403ebc78 at pc 0x73fe21168b0d bp 0x7fff76c72b50 sp 0x7fff76c72b48
READ of size 8 at 0x764e403ebc78 thread T0 (WebCOOP+COEP Co)
    #0 0x73fe21168b0c in nsIFrame::GetStateBits() const /firefox/layout/generic/nsIFrame.h:2564:46
    #1 0x73fe21168b0c in nsSplittableFrame::RemoveFromFlow(nsIFrame*) /firefox/obj-firefox-asan/layout/generic/./../../../layout/generic/nsSplittableFrame.cpp:214:54
    #2 0x73fe2116877b in nsSplittableFrame::Destroy(mozilla::FrameDestroyContext&) /firefox/obj-firefox-asan/layout/generic/./../../../layout/generic/nsSplittableFrame.cpp:36:5
    #3 0x73fe20f5bac8 in nsContainerFrame::Destroy(mozilla::FrameDestroyContext&) /firefox/obj-firefox-asan/layout/generic/./../../../layout/generic/nsContainerFrame.cpp:252:22
    #4 0x73fe210b2113 in nsInlineFrame::Destroy(mozilla::FrameDestroyContext&) /firefox/obj-firefox-asan/layout/generic/./../../../layout/generic/nsInlineFrame.cpp:208:21
    #5 0x73fe20f16798 in nsBlockFrame::DoRemoveFrame(mozilla::FrameDestroyContext&, nsIFrame*, unsigned int) /firefox/obj-firefox-asan/layout/generic/./../../../layout/generic/nsBlockFrame.cpp:7388:20
    #6 0x73fe20f1593f in nsBlockFrame::RemoveFrame(mozilla::FrameDestroyContext&, mozilla::FrameChildListID, nsIFrame*) /firefox/obj-firefox-asan/layout/generic/./../../../layout/generic/nsBlockFrame.cpp:6553:5
    #7 0x73fe20d3d836 in nsFrameManager::RemoveFrame(mozilla::FrameDestroyContext&, mozilla::FrameChildListID, nsIFrame*) /firefox/obj-firefox-asan/layout/base/./../../../layout/base/nsFrameManager.cpp:118:18
    #8 0x73fe20d3d836 in nsCSSFrameConstructor::ContentWillBeRemoved(nsIContent*, nsCSSFrameConstructor::RemovalKind) /firefox/obj-firefox-asan/layout/base/./../../../layout/base/nsCSSFrameConstructor.cpp:7039:3
    #9 0x73fe20d36915 in nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /firefox/obj-firefox-asan/layout/base/./../../../layout/base/nsCSSFrameConstructor.cpp:7987:31
    #10 0x73fe20d3e8cb in nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame*) /firefox/xpcom/base/nsCOMPtr.h:0:48
    #11 0x73fe20d3c937 in nsCSSFrameConstructor::ContentWillBeRemoved(nsIContent*, nsCSSFrameConstructor::RemovalKind) /firefox/obj-firefox-asan/layout/base/./../../../layout/base/nsCSSFrameConstructor.cpp:6884:7
    #12 0x73fe20c8f659 in mozilla::PresShell::ContentWillBeRemoved(nsIContent*, ContentRemoveInfo const&) /firefox/obj-firefox-asan/layout/base/./../../../layout/base/PresShell.cpp:4819:22
    #13-#15 mozilla::dom::MutationObservers::NotifyContentWillBeRemoved(...) /firefox/dom/base/MutationObservers.cpp
    #16 0x73fe185d87ac in nsINode::RemoveChildNode(nsIContent*, bool, BatchRemovalState const*, nsINode*, MutationEffectOnScript) /firefox/obj-firefox-asan/dom/base/./../../../dom/base/nsINode.cpp:2816:5
    #17 0x73fe185ca9e0 in nsINode::RemoveChildInternal(nsINode&, MutationEffectOnScript, mozilla::ErrorResult&) /firefox/obj-firefox-asan/dom/base/./../../../dom/base/nsINode.cpp:1358:3
    #18 0x73fe185d46a1 in nsINode::Remove() /firefox/obj-firefox-asan/dom/base/./../../../dom/base/nsINode.cpp:2505:11
    #20 0x73fe1a30b89b in mozilla::dom::Element_Binding::remove(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /firefox/obj-firefox-asan/dom/bindings/./ElementBinding.cpp:12156:24
    #21 0x73fe1a5c2d26 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /firefox/obj-firefox-asan/dom/bindings/./../../../dom/bindings/BindingUtils.cpp:3215:13
    #22 0x231c1d4f2a84  (<unknown module>)

0x764e403ebc78 is located 888 bytes inside of 8192-byte region [0x764e403eb900,0x764e403ed900)
allocated by thread T0 (WebCOOP+COEP Co) here:
    #0 0x5a4f81261994 in __interceptor_malloc _asan_rtl_:3
    #1 0x73fe14923def in mozilla::ArenaAllocator<8192ul, 8ul>::AllocateChunk(unsigned long) /firefox/obj-firefox-asan/dist/include/mozilla/ArenaAllocator.h:168:15
    #2-#6 mozilla::ArenaAllocator Allocate / mozilla::PresShell::AllocateByObjectID / AllocateFrame (PresShell frame arena)
    #7-#34 frame construction under nsCSSFrameConstructor (arena chunk hosting the destroyed nsInlineFrame continuation; the poisoned bytes are the freed frame, shadow bytes f7 = poisoned by user)

SUMMARY: AddressSanitizer: use-after-poison (/firefox/obj-firefox-asan/dist/bin/libxul.so+0x213f8b0c) (BuildId: b5a37151dae7067c961d9cf6ac951dd5)
==92238==ABORTING

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: verifier-0.jsonl

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: summarizer.jsonl

Comment 3

[Bugmon [:jkratzer for issues]]

Attachment: patch.jsonl

Comment 4

[Bugmon [:jkratzer for issues]]

Attachment: analyzer.jsonl

Comment 5

[Bugmon [:jkratzer for issues]]

Attachment: testcase.html

Comment 6

[Bugmon [:jkratzer for issues]]

Attachment: source_patch.diff

Comment 7

[Bugmon [:jkratzer for issues]]

Attachment: prefs.json

Comment 8

[Bugmon [:jkratzer for issues]]

Attachment: fix.patch

Comment 9

[Bugmon [:jkratzer for issues]]

Attachment: crash_stack.txt

Comment 10

[Bugmon [:jkratzer for issues]]

Attachment: NOTES.md

Comment 11

[Bugmon [:jkratzer for issues]]

Attachment: prefs.js

Comment 13

[Emilio Cobos Álvarez [:emilio]]

I think the source patch might be introducing the bug? It's breaking the assumption that then it uses to break. It's also frame poisoning, so sec-moderate / S3 probably.

Ting-Yu, am I missing something / can you double-check, since you introduced that cache IIRC?

Comment 14

[BugBot [:suhaib / :marco]]

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:dholbert, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Comment 15

[Daniel Holbert [:dholbert]]

This doesn't make sense as a sec-high; based on the description in comment 0, it's mitigated by frame poisoning.

Comment 16

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

I can take a look at this bug.

Comment 17

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #15)

This doesn't make sense as a sec-high; based on the description in comment 0, it's mitigated by frame poisoning.

--> removing security-sensitive flag, too.

Comment 18

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 2053578 - Use static_cast in nsSplittableFrame::UpdateFirstContinuationAndFirstInFlowCache(). r?#layout

nsSplittableFrame is a subclass of nsIFrame, it is sufficient to use
static_cast.

Comment 19

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 2053578 - Update first-in-flow cache when a continuation is changing from fluid to non-fluid. r?#layout

The original case in bug 2053578 comment 5 can reproduce an ASAN
use-after-poison with the patch bug 2053578 comment 6 applied. However, with
unpatched code, the best we can do is using a DEBUG-only assertion to catch the
error condition that detect a stale first-in-flow cache in next-in-flow.

bidi-inline-continuation-first-in-flow.html is generated with the help of
Claude code, and it can trigger the assertion without other fix in this patch.

Comment 20

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: An explainer for the root casue and proposed fix (generated by Claude)

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/734eaa2b1a9e
https://hg.mozilla.org/integration/autoland/rev/3ec6e7dfb4e9
Use static_cast in nsSplittableFrame::UpdateFirstContinuationAndFirstInFlowCache(). r=layout-reviewers,jfkthame
https://github.com/mozilla-firefox/firefox/commit/f49e44a6db6e
https://hg.mozilla.org/integration/autoland/rev/c1c95a8d3b6e
Update first-in-flow cache when a continuation is changing from fluid to non-fluid. r=layout-reviewers,jfkthame

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/61643 for changes under testing/web-platform/tests

Comment 23

[rperta]

https://hg.mozilla.org/mozilla-central/rev/3ec6e7dfb4e9
https://hg.mozilla.org/mozilla-central/rev/c1c95a8d3b6e

Comment 24

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 25

[BugBot [:suhaib / :marco]]

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

• If yes, please nominate the patch for beta and ESR approvals.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox154 and the ESR status flag(s) to wontfix.

For more information, please visit BugBot documentation.