2026305 - Bitfield overflow in [@ CellData::SetRowSpanOffset] with rowspan=0 causes wild pointer dereference in border-collapse layout

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Bugmon [:jkratzer for issues]]

Summary

Integer overflow / bitfield truncation vulnerability in layout/tables/celldata.h line 296. When a <td rowspan=0> cell is placed in a rowgroup with more than 65535 rows, nsCellMap::GetRowSpanForNewCell returns an unclamped span value that CellData::SetRowSpanOffset stores without masking. The overflow corrupts adjacent bitfields and the truncated readback desynchronizes the cell map, causing border-collapse layout to dereference wild pointers.

Affected Code

File: layout/tables/celldata.h, lines 294-299

inline void CellData::SetRowSpanOffset(uint32_t aSpan) {
  mBits &= ~ROW_SPAN_OFFSET;
  mBits |= (aSpan << ROW_SPAN_SHIFT);   // BUG: no mask, overflows into bits 19+
  mBits |= SPAN;
  mBits |= ROW_SPAN;
}

ROW_SPAN_OFFSET is 0xFFFF << 3 (bits 3–18, 16 bits wide). When aSpan > 65535, aSpan << 3 overflows past bit 18 into bit 19 (unused), bit 20 (COL_SPAN), bit 21 (OVERLAP), and bits 22–31 (COL_SPAN_OFFSET). GetRowSpanOffset masks on readback and returns only the low 16 bits, so an offset of 65536 reads back as 0, desynchronizing the cell map.

File: layout/tables/nsCellMap.cpp, line 1846

if (0 == rowSpan) {
  // Use a min value of 2 for a zero rowspan...
  rowSpan = std::max(2, mContentRowCount - aRowIndex);  // BUG: no upper clamp
  aIsZeroRowSpan = true;
}

While HTMLTableCellElement::ParseAttribute clamps explicit rowspan values to MAX_ROWSPAN (65534), the special rowspan=0 path bypasses this clamp entirely.

Correct Pattern

// In celldata.h
inline void CellData::SetRowSpanOffset(uint32_t aSpan) {
  mBits &= ~ROW_SPAN_OFFSET;
  mBits |= ((aSpan << ROW_SPAN_SHIFT) & ROW_SPAN_OFFSET);  // mask to field
  mBits |= SPAN;
  mBits |= ROW_SPAN;
}

// In nsCellMap.cpp — clamp the zero-rowspan expansion
if (0 == rowSpan) {
  rowSpan = std::clamp(mContentRowCount - aRowIndex, 2, MAX_ROWSPAN);
  aIsZeroRowSpan = true;
}

Exploit Chain

1. Attacker serves HTML with <table style="border-collapse:collapse"> containing <td rowspan=0> in the first row.
2. JavaScript appends >65535 additional rows to the same <tbody>.
3. Layout flush (offsetHeight) triggers frame construction; nsCellMap::AppendCell calls GetRowSpanForNewCell, which returns ~65541 for the zero-rowspan cell.
4. The loop at nsCellMap.cpp:1395 calls SetRowSpanOffset(rowX - aRowIndex) with offsets up to 65540, overflowing the 16-bit bitfield.
5. nsTableFrame::CalcBCBorders iterates the damaged area. BCMapCellIterator::PeekIAt reads the truncated offset via GetRowSpanOffset() and computes a wrong origin row: rgRowIndex -= truncated_offset.
6. The misidentified origin cell is passed to BCMapCellInfo::SetInfo, which calls GetEffectiveRowSpan on confused cell-map state.
7. nsCellMap::GetRowSpan → GetDataAt → nsTArray::Length() dereferences a wild array header — crash.

Security Impact

Severity: High. Web content can corrupt the internal cell-map bitfields without any user interaction beyond visiting a page. The corrupted mBits overflow into COL_SPAN/OVERLAP/COL_SPAN_OFFSET flags, and the truncated row-span offset causes border-collapse code to follow invalid frame pointers. Register state at crash (r13 = 0x10005 = 65541) confirms attacker-controlled overflow value propagates to the crash site. While the observed crash is a wild read, the bitfield corruption affects pointer-union data (mBits aliases nsTableCellFrame* mOrigCell), giving potential for type confusion on adjacent cell entries.

Preconditions: None beyond rendering attacker-controlled HTML.

ASAN Report

==147611==ERROR: AddressSanitizer: ABRT on unknown address 0x000000000139 (pc 0x74641255a67f bp 0x7ffd7d0feb80 sp 0x7ffd7d0feb80 T0)
    #0 0x74641255a67f in nsTArray_base<nsTArray_RelocateUsingMemutils>::Length() const /firefox/obj-x86_64-pc-linux-gnu/dist/include/nsTArray.h:365:43
    #1 0x74641255a67f in nsTArray_Impl<CellData*, nsTArrayInfallibleAllocator>::SafeElementAt(unsigned long) const /firefox/obj-x86_64-pc-linux-gnu/dist/include/nsTArray.h:1094:18
    #2 0x74641255a67f in nsCellMap::GetDataAt(int, int) const /firefox/layout/tables/nsCellMap.cpp:2300:56
    #3 0x746412568ca5 in nsCellMap::GetRowSpan(int, int, bool) const /firefox/layout/tables/nsCellMap.cpp:1889:22
    #4 0x746412568ca5 in nsTableCellMap::GetEffectiveRowSpan(int, int) const /firefox/layout/tables/nsCellMap.cpp:304:19
    #5 0x746412568ca5 in nsTableFrame::GetEffectiveRowSpan(nsTableCellFrame const&, nsCellMap*) /firefox/layout/tables/nsTableFrame.cpp:407:24
    #6 0x74641259f8d5 in BCMapCellInfo::SetInfo(nsTableRowFrame*, int, BCCellData*, BCMapCellIterator*, nsCellMap*) /firefox/layout/tables/nsTableFrame.cpp:3790:31
    #7 0x7464125a1f29 in BCMapCellIterator::PeekIAt(BCMapCellInfo const&, int, int, BCMapCellInfo&) /firefox/layout/tables/nsTableFrame.cpp:4057:12
    #8 0x746412587d40 in BCMapCellIterator::PeekIStart(BCMapCellInfo const&, int, BCMapCellInfo&) /firefox/layout/tables/nsTableFrame.cpp:4030:3
    #9 0x746412587d40 in nsTableFrame::CalcBCBorders() /firefox/layout/tables/nsTableFrame.cpp:4956:14
    #10 0x746412597f22 in nsTableFrame::GetOuterBCBorder(mozilla::WritingMode) const /firefox/layout/tables/nsTableFrame.cpp:2157:38
    ...
    #42 0x74640a2496d9 in mozilla::dom::Element::OffsetHeight() /firefox/obj-x86_64-pc-linux-gnu/dist/include/mozilla/dom/Element.h:1820:5

==147611==Register values:
rax = 0x0000000000000000  rbx = 0x0000000000000000  rcx = 0x000000000000a3b3  rdx = 0x0000000000000000
rdi = 0x00007463623b55a0  rsi = 0x000074a4306ef0d0  rbp = 0x00007ffd7d0feb80  rsp = 0x00007ffd7d0feb80
 r8 = 0x0000000000000000   r9 = 0x0000000000000000  r10 = 0x0000000000000000  r11 = 0x00000e8c6c46c701
r12 = 0x000074a43045c3d0  r13 = 0x0000000000010005  r14 = 0x0000000000000000  r15 = 0x000000000000a3b3
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT (/firefox/obj-x86_64-pc-linux-gnu/dist/bin/libxul.so+0x2071e67f)
==147611==ABORTING

Note r13 = 0x10005 = 65541 — the unclamped row count. The ABRT signal with no assert frames indicates ASAN's shadow-memory check faulted on a wild pointer; the 0x139 is siginfo noise, not a fault address.

Suggested Fix

Apply defense in depth — clamp at the source and mask at the sink:

// layout/tables/nsCellMap.cpp — GetRowSpanForNewCell
if (0 == rowSpan) {
  rowSpan = std::clamp(mContentRowCount - aRowIndex, 2, MAX_ROWSPAN);
  aIsZeroRowSpan = true;
}

// layout/tables/celldata.h — SetRowSpanOffset
inline void CellData::SetRowSpanOffset(uint32_t aSpan) {
  MOZ_ASSERT(aSpan <= (ROW_SPAN_OFFSET >> ROW_SPAN_SHIFT), "rowspan offset overflow");
  mBits &= ~ROW_SPAN_OFFSET;
  mBits |= ((uintptr_t(aSpan) << ROW_SPAN_SHIFT) & ROW_SPAN_OFFSET);
  mBits |= SPAN;
  mBits |= ROW_SPAN;
}

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: Testcase: test.html

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: Crash stack trace

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Bugmon [:jkratzer for issues] from comment #0)

While HTMLTableCellElement::ParseAttribute clamps explicit rowspan values to MAX_ROWSPAN (65534), the special rowspan=0 path bypasses this clamp entirely.

For anyone else who's wondering (I at least had forgotten) -- per https://developer.mozilla.org/en-US/docs/Web/API/HTMLTableCellElement/rowSpan#value , rowspan=0 means to span all remaining rows.

(And so, if you have more than MAX_ROWSPAN rows that follow an element with rowspan=0, that presents a problem as noted in comment 0.)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: (secure)

Comment 6

[Daniel Holbert [:dholbert]]

Comment on attachment 9558437 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Probably pretty easily, unfortunately.
• 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?: All supported branches (including esr115)
• 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?: Table code doesn't change that much these days, so this patch should apply cleanly to all old branches.
• How likely is this patch to cause regressions; how much testing does it need?: Low risk of regressions, no need for manual testing; if it passes tests on treeherder (and I anticipate it will), it should be fine.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 7

[Bugmon [:jkratzer for issues]]

Unable to reproduce bug 2026305 using build mozilla-central 20260325212151-b431043d8af2. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 9558437 [details]
(secure)

sec-approval+ to land now and request uplifts

Comment 9

[Daniel Holbert [:dholbert]]

Thanks - triggered lando. I'll request uplifts in the morning, assuming this sticks. (or in the hopefully-unlikely event that this gets backed out for test failures, I'll tidy those up and reland before requesting uplift.)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/12540fc982c9
https://hg.mozilla.org/integration/autoland/rev/935f9a0ad992
Add additional checks that table rowspan/colspan are in acceptable ranges. r=layout-reviewers,emilio

Comment 11

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined/Reason for urgency: Security vulnerability (pointers can get overwritten with attacker-controlled values, perhaps granting attackers remote code execution or the ability to read data from other memory locations).
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Just adding bounds checks to clamp extremely large row/colspan sizes (which we already were doing at parse-time, and now we'll do a bit more robustly).
• String changes made/needed?: None
• Is Android affected?: yes

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: (secure)

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

Comment 13

[Phabricator Automation]

firefox-release Uplift Approval Request

• User impact if declined/Reason for urgency: Security vulnerability (pointers can get overwritten with attacker-controlled values, perhaps granting attackers remote code execution or the ability to read data from other memory locations).
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Just adding bounds checks to clamp extremely large row/colspan sizes (which we already were doing at parse-time, and now we'll do a bit more robustly).
• String changes made/needed?: None
• Is Android affected?: yes

Comment 14

[Daniel Holbert [:dholbert]]

Attachment: (secure)

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

Comment 15

[Daniel Holbert [:dholbert]]

Looks like ESR branches need some (likely very minor) rebasing; I'll take a look at that soon (after a meeting).

Comment 16

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined/Reason for urgency: Security vulnerability (pointers can get overwritten with attacker-controlled values, perhaps granting attackers remote code execution or the ability to read data from other memory locations).
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Just adding bounds checks to clamp extremely large row/colspan sizes (which we already were doing at parse-time, and now we'll do a bit more robustly).
• String changes made/needed?: None
• Is Android affected?: yes

Comment 17

[Daniel Holbert [:dholbert]]

Attachment: (secure)

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

Comment 18

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined/Reason for urgency: Security vulnerability (pointers can get overwritten with attacker-controlled values, perhaps granting attackers remote code execution or the ability to read data from other memory locations).
• Code covered by automated testing?: yes
• Fix verified in Nightly?: no
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Just adding bounds checks to clamp extremely large row/colspan sizes (which we already were doing at parse-time, and now we'll do a bit more robustly).
• String changes made/needed?: None
• Is Android affected?: yes

Comment 19

[Daniel Holbert [:dholbert]]

Attachment: (secure)

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

Comment 20

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

https://hg.mozilla.org/mozilla-central/rev/935f9a0ad992

Comment 21

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/48ba718fecdc
https://hg.mozilla.org/releases/mozilla-beta/rev/d369b0a7ce70

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/2729e9c2a271

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/9c94e2c49373

Comment 24

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/bd2278cd2f01
https://hg.mozilla.org/releases/mozilla-release/rev/becd4498b547

Comment 25

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

https://hg-edge.mozilla.org/releases/mozilla-esr140/rev/4ba6147a2672bd000f37a046a70d130ae4d30db4

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/d4b0736648b9