Closed Bug 2029699 Opened 7 months ago Closed 7 months ago

Use-after-free (use-after-poison) on InlineBackgroundData::mLineContainer in [@ InlineBackgroundData::GetContinuousRect]

Categories

(Core :: Web Painting, defect)

defect

Tracking

()

RESOLVED FIXED
151 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox-esr140 --- wontfix
firefox149 --- wontfix
firefox150 + fixed
firefox151 + fixed

People

(Reporter: bugmon, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(6 keywords, Whiteboard: [prefs-checked][adv-main150+r])

Attachments

(4 files, 2 external files, 1 obsolete file)

nsCSSRendering keeps a process-global cache, gInlineBGData (an InlineBackgroundData), which stores raw nsIFrame* pointers mFrame and mLineContainer. The cache is only reset when gFrameTreeLockCount drops to 0 in EndFrameTreesLocked(). When an SVG image is used as a CSS background, VectorImage rasterization re-enters nsLayoutUtils::PaintFrame for the SVG document while the outer document's PaintFrame is still on the stack, so the lock count goes 1→2→1 and the cache is not cleared between background layers.

By supplying the same SVG twice as two background layers with different background-size values, the second rasterization calls SVGDocumentWrapper::UpdateViewportBounds → FlushLayout. A media query in the SVG (triggered by the smaller viewport) flips display values and forces a reframe, destroying the nsBlockFrame previously cached as mLineContainer and the nsInlineFrame cached as mFrame. A decoy <span> toggled from display:none to inline causes the PresArena LIFO freelist to hand the freshly created primary nsInlineFrame the exact address of the previously cached mFrame.

When the inner display list paints the border of that span, JoinBoxesForSlice → GetBorderContinuousRect → GetContinuousRect → SetFrame sees aFrame == mFrame and returns early without re-running Init(), leaving the stale mBidiEnabled=true and stale mLineContainer in place. Line 111 then dereferences mLineContainer->StyleVisibility() on a freed (poisoned) frame. In a non-ASan build the arena slot can be reused by an object of a different layout, yielding a type-confused read of mComputedStyle and downstream wild-pointer dereferences.

Build Info

Affected Code

File: layout/painting/nsCSSRendering.cpp, line 264-271

  void SetFrame(nsIFrame* aFrame) {
    MOZ_ASSERT(aFrame, "Need a frame");
    NS_ASSERTION(gFrameTreeLockCount > 0,
                 "Can't call this when frame tree is not locked");

    if (aFrame == mFrame) {
      return;   // <-- early return: arena address reuse makes a brand-new frame equal to the stale cached pointer, skipping Reset()/Init()
    }

File: layout/painting/nsCSSRendering.cpp, line 98-114

  nsRect GetContinuousRect(nsIFrame* aFrame) {
    MOZ_ASSERT(static_cast<nsInlineFrame*>(do_QueryFrame(aFrame)));

    SetFrame(aFrame);

    nscoord pos;
    if (mBidiEnabled) {
      pos = mLineContinuationPoint;

      // ...
      bool isRtlBlock = (mLineContainer->StyleVisibility()->mDirection ==   // <-- UAF: mLineContainer was destroyed by the mid-paint reframe
                         StyleDirection::Rtl);
      nscoord curOffset = mVertical ? aFrame->GetOffsetTo(mLineContainer).y
                                    : aFrame->GetOffsetTo(mLineContainer).x;

File: layout/painting/nsCSSRendering.cpp, line 422

static StaticAutoPtr<InlineBackgroundData> gInlineBGData;

File: layout/painting/nsCSSRendering.cpp, line 1336-1344

void nsCSSRendering::BeginFrameTreesLocked() { ++gFrameTreeLockCount; }

void nsCSSRendering::EndFrameTreesLocked() {
  NS_ASSERTION(gFrameTreeLockCount > 0, "Unbalanced EndFrameTreeLocked");
  --gFrameTreeLockCount;
  if (gFrameTreeLockCount == 0) {
    gInlineBGData->Reset();   // <-- never reached between nested SVG-image background layers
  }
}

The InlineBackgroundData cache assumes the frame tree cannot mutate while gFrameTreeLockCount > 0, and uses raw-pointer equality as a validity check. Nested SVG-as-image painting violates the immutability assumption (the inner SVG document is reframed while the outer lock is still held), and PresArena LIFO address reuse defeats the pointer-equality check, so stale raw frame pointers are dereferenced.

Exploit Chain

  1. Attacker serves a page with an element whose background-image is the same data: SVG twice, with two different background-size values (e.g. 100px and 50px).
  2. The SVG contains a foreignObject with dir='rtl' (enables the bidi path so mBidiEnabled=true and mLineContainer is consulted), a <span> with a border whose text wraps (so it has continuations and SkipSides is non-empty), and an @media (max-width:75px) rule that flips an ancestor display:block→flex and a sibling decoy <span> display:none→inline.
  3. Outer nsLayoutUtils::PaintFrame increments gFrameTreeLockCount to 1 and walks background layers. The first layer (100px) calls VectorImage::GetImageProvider → SVGDrawingCallback → PresShell::RenderDocument → nested PaintFrame (lock=2). The bordered span's nsDisplayBorder paints, populating gInlineBGData with mFrame=<span inline frame> and mLineContainer=<inner block frame>. Inner PaintFrame ends (lock=1, no Reset).
  4. The second layer (50px) calls VectorImage::GetImageProvider, which first calls SVGDocumentWrapper::UpdateViewportBounds(50,50) → FlushLayout. The media query now matches: the outer div becomes flex and the decoy span becomes inline, forcing frame reconstruction. The cached mLineContainer block frame and mFrame inline frame are destroyed and their arena slots poisoned/freed.
  5. During reconstruction the new primary nsInlineFrame for the bordered span is allocated; due to the decoy span consuming one freelist slot, the new frame lands at exactly the old mFrame address.
  6. The nested PaintFrame for the second layer paints the bordered span's border. JoinBoxesForSlice → GetBorderContinuousRect → GetContinuousRect → SetFrame(aFrame): aFrame == stale mFrame → early return. mBidiEnabled is still true, so line 111 dereferences the freed mLineContainer (StyleVisibility() reads mComputedStyle from poisoned memory).
  7. In a release build the freed line-container slot can be refilled with attacker-influenced layout objects, turning the stale read into a type-confused pointer dereference and a primitive toward renderer code execution.

Steps to Reproduce

  1. Build Firefox with AddressSanitizer (no source modifications required).
  2. (Optional, matches default) set pref image.svg.blob-image = false.
  3. Load test.html in the ASan build.
  4. Observe AddressSanitizer: use-after-poison at InlineBackgroundData::GetContinuousRect, nsCSSRendering.cpp:111.

Security Impact

  • Severity: High
  • Attacker capability: Web content can trigger a use-after-free read of an nsIFrame in the PresShell arena during painting. The freed arena slot can be refilled with a different object, so the read of mComputedStyle and subsequent virtual/style dereferences operate on attacker-influenced memory, providing a primitive toward arbitrary read and renderer (content-process) code execution.
  • Preconditions: None beyond visiting an attacker-controlled web page. No user interaction, no non-default prefs (image.svg.blob-image already defaults to false), no source patches.

ASAN Report

==16199==ERROR: AddressSanitizer: use-after-poison on address 0x79a0203cbfe8 at pc 0x775002586771 bp 0x7ffc6836ddf0 sp 0x7ffc6836dde8
READ of size 8 at 0x79a0203cbfe8 thread T0 (Isolated Web Co)
    #0 0x775002586770 in RefPtr<mozilla::ComputedStyle>::get() const /firefox/obj-firefox-asan/dist/include/mozilla/RefPtr.h:312:27
    #1 0x775002586770 in RefPtr<mozilla::ComputedStyle>::operator->() const /firefox/obj-firefox-asan/dist/include/mozilla/RefPtr.h:342:12
    #2 0x775002586770 in nsIFrame::StyleVisibility() const /firefox/layout/generic/nsIFrame.h:1005:3
    #3 0x775002586770 in InlineBackgroundData::GetContinuousRect(nsIFrame*) /firefox/layout/painting/nsCSSRendering.cpp:111:42
    #4 0x775002585641 in InlineBackgroundData::GetBorderContinuousRect(nsIFrame*, nsRect) /firefox/layout/painting/nsCSSRendering.cpp:178:31
    #5 0x7750024af57f in JoinBoxesForSlice(nsIFrame*, nsRect const&, InlineBoxOrder) /firefox/layout/painting/nsCSSRendering.cpp:547:34
    #6 0x7750024b5ef0 in nsCSSRendering::BoxDecorationRectForBorder(nsIFrame*, nsRect const&, mozilla::Sides, nsStyleBorder const*) /firefox/layout/painting/nsCSSRendering.cpp:569:16
    #7 0x7750024b5ef0 in ConstructBorderRenderer(...) /firefox/layout/painting/nsCSSRendering.cpp:744:29
    #8 0x7750024b0974 in nsCSSRendering::PaintBorderWithStyleBorder(...) /firefox/layout/painting/nsCSSRendering.cpp:866:28
    #9 0x7750024b001f in nsCSSRendering::PaintBorder(...) /firefox/layout/painting/nsCSSRendering.cpp:632:10
    #10 0x77500252ffc9 in mozilla::nsDisplayBorder::Paint(...) /firefox/layout/painting/nsDisplayList.cpp:4446:9
    #11 0x7750024ae683 in mozilla::nsDisplayList::Paint(...) /firefox/layout/painting/nsDisplayList.cpp:2333:11
    #18 0x77500251238a in mozilla::nsDisplayList::PaintRoot(...) /firefox/layout/painting/nsDisplayList.cpp:2399:5
    #19 0x775001d99109 in nsLayoutUtils::PaintFrame(...) /firefox/layout/base/nsLayoutUtils.cpp:3252:9
    #20 0x775001c8802c in mozilla::PresShell::RenderDocument(...) /firefox/layout/base/PresShell.cpp:4883:3
    #21 0x774ff901985a in mozilla::image::SVGDrawingCallback::operator()(...) /firefox/image/VectorImage.cpp:290:14
    #24 0x774ff9021d5f in mozilla::image::VectorImage::GetImageProvider(...) /firefox/image/VectorImage.cpp:867:26
    #25 0x77500256657b in mozilla::nsImageRenderer::BuildWebRenderDisplayItems(...) /firefox/layout/painting/nsImageRenderer.cpp:689:37
    #29 0x775002524280 in mozilla::nsDisplayBackgroundImage::CreateWebRenderCommands(...) /firefox/layout/painting/nsDisplayList.cpp:3604:7
    #41 0x775001d99109 in nsLayoutUtils::PaintFrame(...) /firefox/layout/base/nsLayoutUtils.cpp:3252:9
    #42 0x775001c95f70 in mozilla::PresShell::PaintInternal(...) /firefox/layout/base/PresShell.cpp:6592:5

0x79a0203cbfe8 is located 3816 bytes inside of 8192-byte region [0x79a0203cb100,0x79a0203cd100)
allocated by thread T0 (Isolated Web Co) here:
    #0 0x576679b6da04 in __interceptor_malloc _asan_rtl_:3
    #1 0x774ff483cf5f in mozilla::ArenaAllocator<8192ul, 8ul>::AllocateChunk(unsigned long) /firefox/obj-firefox-asan/dist/include/mozilla/ArenaAllocator.h:168:15
    #5 0x775001ec797e in mozilla::PresShell::AllocateByObjectID(mozilla::ArenaObjectID, unsigned long) /firefox/obj-firefox-asan/dist/include/mozilla/PresShell.h:291:32
    #6 0x775001ec797e in mozilla::PresShell::AllocateFrame(nsQueryFrame::FrameIID, unsigned long) /firefox/obj-firefox-asan/dist/include/mozilla/PresShell.h:283:12

SUMMARY: AddressSanitizer: use-after-poison
Shadow bytes around the buggy address:
=>0x79a0203cbf80: 00 00 00 00 00 00 00 00 00 f7 f7 f7 f7[f7]f7 f7
  0x79a0203cc000: f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7 f7
  Poisoned by user:        f7
==16199==ABORTING
Attached file test.html —
Attached file prefs.js —
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Group: core-security → gfx-core-security

Should this maybe go in Layout instead of Web Painting? I'm not sure exactly how the divide is here. Thanks.

Flags: needinfo?(emilio)

Web painting is probably correct.

Flags: needinfo?(tnikkel)

The only pref referenced (image.svg.blob-image=false) is the default on all channels (StaticPrefList.yaml), so this reproduces in a fully supported configuration. sec-high looks correct: web-content-triggered PresArena UAF with no user interaction, no patches, no non-default prefs.

Whiteboard: [prefs-checked]

Yeah web painting is correct. Tim, I have a WIP fix (making InlineBackgroundData local to nsPresContext). Lmk if you have any better ideas, but I think that's probably the most obvious one. It's also a use-after-poison, so probably not sec-high.

Flags: needinfo?(emilio) → needinfo?(choller)

Taking after discussing with Tim.

Assignee: nobody → emilio
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(tnikkel) → needinfo?(emilio)
Flags: needinfo?(choller)

(In reply to Emilio Cobos Álvarez [:emilio] from comment #9)

It's also a use-after-poison, so probably not sec-high.

I assume you mean frame poisoning, correct? because use-after-poison is not always guaranteed to be not sec-high. We use ASan's poisoning mechanism also in internal allocators in such a way that it isn't enabled in release.

Attached file (secure) —
Attached file (secure) (obsolete) —

I think this is probably a bit better over-all?

Flags: needinfo?(emilio)
Group: gfx-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 151 Branch

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

For more information, please visit BugBot documentation.

Flags: needinfo?(emilio)

firefox-beta Uplift Approval Request

  • User impact if declined/Reason for urgency: Sec bug
  • Code covered by automated testing?: no
  • Fix verified in Nightly?: yes
  • Needs manual QE testing?: no
  • Steps to reproduce for manual QE testing:
  • Risk associated with taking this patch: low
  • Explanation of risk level: trivial fix
  • String changes made/needed?: none
  • Is Android affected?: yes
Attachment #9568278 - Flags: approval-mozilla-beta?
Flags: needinfo?(emilio)
Attachment #9568278 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [sec] [uplift] [qa-triage-done-c151/b150]
Blocks: 2031670

Comment on attachment 9566916 [details]
(secure)

Revision D292387 was moved to bug 2031670. Setting attachment 9566916 [details] to obsolete.

Attachment #9566916 - Attachment is obsolete: true
Whiteboard: [prefs-checked] → [prefs-checked][adv-main150+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: