Closed Bug 1880405 Opened 1 year ago Closed 1 year ago

crash in [@ SetColumn]

Categories

(Core :: Layout: Tables, defect)

defect

Tracking

()

VERIFIED FIXED
125 Branch
Tracking Status
firefox-esr115 124+ verified
firefox123 --- wontfix
firefox124 + verified
firefox125 + verified

People

(Reporter: tsmith, Assigned: dshin)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [bugmon:bisected,confirmed][adv-main124+r][adv-esr115.9+r])

Attachments

(5 files, 2 obsolete files)

Attached file testcase.html

Found while fuzzing m-c 20240214-02d94c298a2b (--enable-address-sanitizer --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>
==12523==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x7f634670ad9c bp 0x7ffe4132ff50 sp 0x7ffe4132f2e0 T0)
==12523==The signal is caused by a READ memory access.
==12523==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x7f634670ad9c in GetParent /builds/worker/checkouts/gecko/layout/generic/nsIFrame.h:941:48
    #1 0x7f634670ad9c in SetColumn /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:4902:60
    #2 0x7f634670ad9c in nsTableFrame::CalcBCBorders() /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:5088:14
    #3 0x7f6346715406 in nsTableFrame::GetIncludedOuterBCBorder(mozilla::WritingMode) const /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:2428:38
    #4 0x7f6346724c1a in nsTableFrame::GetCollapsedBorderPadding(mozilla::Maybe<mozilla::LogicalMargin>&, mozilla::Maybe<mozilla::LogicalMargin>&) const /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:2454:21
    #5 0x7f634676ac39 in nsTableWrapperFrame::InnerTableShrinkWrapSize(gfxContext*, nsTableFrame*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::StyleSizeOverrides const&, mozilla::EnumSet<mozilla::ComputeSizeFlag, unsigned char>) const /builds/worker/checkouts/gecko/layout/tables/nsTableWrapperFrame.cpp:284:16
    #6 0x7f634676d3c8 in nsTableWrapperFrame::ComputeAutoSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::StyleSizeOverrides const&, mozilla::EnumSet<mozilla::ComputeSizeFlag, unsigned char>) /builds/worker/checkouts/gecko/layout/tables/nsTableWrapperFrame.cpp:439:38
    #7 0x7f6346300c7a in nsIFrame::ComputeSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::StyleSizeOverrides const&, mozilla::EnumSet<mozilla::ComputeSizeFlag, unsigned char>) /builds/worker/checkouts/gecko/layout/generic/nsIFrame.cpp:6324:7
    #8 0x7f634676d179 in nsTableWrapperFrame::ComputeSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::StyleSizeOverrides const&, mozilla::EnumSet<mozilla::ComputeSizeFlag, unsigned char>) /builds/worker/checkouts/gecko/layout/tables/nsTableWrapperFrame.cpp:400:35
    #9 0x7f634620b105 in mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::Maybe<mozilla::LogicalSize> const&, mozilla::Maybe<mozilla::LogicalMargin> const&, mozilla::Maybe<mozilla::LogicalMargin> const&, mozilla::LayoutFrameType) /builds/worker/checkouts/gecko/layout/generic/ReflowInput.cpp:2406:19
    #10 0x7f6346203d8e in mozilla::ReflowInput::Init(nsPresContext*, mozilla::Maybe<mozilla::LogicalSize> const&, mozilla::Maybe<mozilla::LogicalMargin> const&, mozilla::Maybe<mozilla::LogicalMargin> const&) /builds/worker/checkouts/gecko/layout/generic/ReflowInput.cpp:393:3
    #11 0x7f6346205466 in mozilla::ReflowInput::ReflowInput(nsPresContext*, mozilla::ReflowInput const&, nsIFrame*, mozilla::LogicalSize const&, mozilla::Maybe<mozilla::LogicalSize> const&, mozilla::EnumSet<mozilla::ReflowInput::InitFlag, unsigned char>, mozilla::StyleSizeOverrides const&, mozilla::EnumSet<mozilla::ComputeSizeFlag, unsigned char>) /builds/worker/checkouts/gecko/layout/generic/ReflowInput.cpp:254:5
    #12 0x7f6346578e09 in void mozilla::Maybe<mozilla::ReflowInput>::emplace<nsPresContext*&, mozilla::ReflowInput const&, nsIFrame*&, mozilla::LogicalSize&>(nsPresContext*&, mozilla::ReflowInput const&, nsIFrame*&, mozilla::LogicalSize&) /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:845:39
    #13 0x7f6346565873 in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:839:23
    #14 0x7f63465642a6 in nsInlineFrame::ReflowInlineFrame(nsPresContext*, mozilla::ReflowInput const&, nsInlineFrame::InlineReflowInput&, nsIFrame*, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsInlineFrame.cpp:667:15
    #15 0x7f63465627d2 in nsInlineFrame::ReflowFrames(nsPresContext*, mozilla::ReflowInput const&, nsInlineFrame::InlineReflowInput&, mozilla::ReflowOutput&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsInlineFrame.cpp:541:7
    #16 0x7f6346561a73 in nsInlineFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsInlineFrame.cpp:357:3
    #17 0x7f6346566b57 in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) /builds/worker/checkouts/gecko/layout/generic/nsLineLayout.cpp:894:13
    #18 0x7f6346295453 in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowState&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:5014:15
    #19 0x7f634629319a in nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4816:5
    #20 0x7f634628b7f2 in nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4688:9
    #21 0x7f63462840f2 in nsBlockFrame::ReflowLine(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3656:24
    #22 0x7f6346278a42 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowState&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3162:29
    #23 0x7f6346271392 in nsBlockFrame::TrialReflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsBlockFrame::TrialReflowState&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1911:35
    #24 0x7f634626d898 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1556:9
    #25 0x7f6346291114 in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowState&) /builds/worker/checkouts/gecko/layout/generic/nsBlockReflowContext.cpp:290:11
    #26 0x7f6346287602 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:4323:11
    #27 0x7f634628413c in nsBlockFrame::ReflowLine(mozilla::BlockReflowState&, nsLineList_iterator, bool*) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3653:5
    #28 0x7f6346278a42 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowState&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:3162:29
    #29 0x7f6346271392 in nsBlockFrame::TrialReflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsBlockFrame::TrialReflowState&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1911:35
    #30 0x7f634626d898 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1556:9
    #31 0x7f63462e4cb0 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:886:14
    #32 0x7f63462b8006 in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsCanvasFrame.cpp:729:7
    #33 0x7f634638c3f4 in ReflowChild /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:886:14
    #34 0x7f634638c3f4 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput&, bool, bool, mozilla::ReflowOutput*) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:915:3
    #35 0x7f634638f217 in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput&, mozilla::ReflowOutput const&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:1048:3
    #36 0x7f634639a1ff in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsGfxScrollFrame.cpp:1523:3
    #37 0x7f63462e5c60 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) /builds/worker/checkouts/gecko/layout/generic/nsContainerFrame.cpp:927:14
    #38 0x7f6346259f46 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/ViewportFrame.cpp:368:7
    #39 0x7f6345ff51f8 in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9777:11
    #40 0x7f6346044a37 in mozilla::PresShell::ProcessReflowCommands(bool) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9950:22
    #41 0x7f634600c233 in DoFlushLayout /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9997:10
    #42 0x7f634600c233 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4361:11
    #43 0x7f6345f83f17 in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1474:5
    #44 0x7f6345f83f17 in nsRefreshDriver::TickObserverArray(unsigned int, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2531:18
    #45 0x7f6345f79f06 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2735:28
    #46 0x7f6345f90f56 in TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:367:13
    #47 0x7f6345f90f56 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver>>&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:345:7
    #48 0x7f6345f90c2e in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:361:5
    #49 0x7f6345f90881 in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:951:5
    #50 0x7f6345f8f734 in mozilla::VsyncRefreshDriverTimer::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:861:5
    #51 0x7f6345f8e274 in mozilla::VsyncRefreshDriverTimer::NotifyVsyncOnMainThread(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:758:5
    #52 0x7f6345f8d872 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:592:14
    #53 0x7f6345f8d425 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:549:9
    #54 0x7f63442ccc2b in mozilla::dom::VsyncMainChild::RecvNotify(mozilla::VsyncEvent const&, float const&) /builds/worker/checkouts/gecko/dom/ipc/VsyncMainChild.cpp:66:15
    #55 0x7f63448e4f5d in mozilla::dom::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:237:78
    #56 0x7f633bf60ae4 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundChild.cpp:5555:32
    #57 0x7f633be99ba5 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1813:25
    #58 0x7f633be955ab in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1732:9
    #59 0x7f633be96959 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1525:3
    #60 0x7f633be97ed3 in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1623:14
    #61 0x7f633a1b9bca in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:578:16
    #62 0x7f633a19fa4b in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:905:26
    #63 0x7f633a19c628 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:728:15
    #64 0x7f633a19cd29 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:514:36
    #65 0x7f633a1c1cf4 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:235:37
    #66 0x7f633a1c1cf4 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_1>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
    #67 0x7f633a1e9c3f in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1199:16
    #68 0x7f633a1f797a in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
    #69 0x7f633bea31a3 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:107:5
    #70 0x7f633bccb92a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:370:10
    #71 0x7f633bccb92a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
    #72 0x7f633bccb92a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
    #73 0x7f634568e0a9 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
    #74 0x7f63458948b2 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:470:33
    #75 0x7f634a66c96e in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:721:20
    #76 0x7f633bccb92a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:370:10
    #77 0x7f633bccb92a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
    #78 0x7f633bccb92a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
    #79 0x7f634a66bf13 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:656:34
    #80 0x5579b285853c in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #81 0x5579b285853c in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
    #82 0x7f6362829d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #83 0x7f6362829e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #84 0x5579b277c848 in _start (/home/user/workspace/browsers/m-c-20240213093751-fuzzing-asan-opt/firefox+0xdc848) (BuildId: 7e944872bf5f84a8d37ab4382ed400de5312bca6)
Flags: in-testsuite?
Group: layout-core-security

Verified bug as reproducible on mozilla-central 20240215035312-981afc9a696a.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: b2d97b936206d8168d56322cc387fbbc7ad9c515 (20230216043537)
End: 02d94c298a2bcd03cc382be935a3edca72511945 (20240214155234)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)

Whiteboard: [bugmon:bisected,confirmed]
Keywords: sec-high

Successfully recorded a pernosco session. A link to the pernosco session will be added here shortly.

A pernosco session for this bug can be found here.

Damage area gets set here with colspan value, which is 63, which is nonsensical for this 1x1 table.

Seems we do guard against rowspan being nonsensical, funnily enough.

Changing the test case to use plain tables, there is no crash. It looks like we match up the number of column cells to span through this trace:

#0  nsTableFrame::InsertCol(nsTableColFrame&, int) (this=0x7f250bc71be8, aColFrame=..., aColIndex=0) at /home/dshin/mozilla-unified/layout/tables/nsTableFrame.cpp:543
#1  0x00007f25245a0016 in nsTableColGroupFrame::AddColsToTable(int, bool, nsFrameList::Slice const&) (this=0x7f250bc72040, aFirstColIndex=0, aResetSubsequentColIndices=true, aCols=...) at /home/dshin/mozilla-unified/layout/tables/nsTableColGroupFrame.cpp:82
#2  0x00007f25245a0bb0 in nsTableFrame::AppendAnonymousColFrames(nsTableColGroupFrame*, int, nsTableColType, bool) (this=0x7f250bc71be8, aColGroupFrame=0x7f250bc72040, aNumColsToAdd=2, aColType=eColAnonymousCell, aAddToTable=true)
    at /home/dshin/mozilla-unified/layout/tables/nsTableFrame.cpp:716
#3  0x00007f25245a4e04 in nsTableFrame::AppendAnonymousColFrames(int) (this=0x7f250bc71be8, aNumColsToAdd=2) at /home/dshin/mozilla-unified/layout/tables/nsTableFrame.cpp:664
#4  0x00007f25245a4e77 in nsTableFrame::MatchCellMapToColCache(nsTableCellMap*) (this=0x7f250bc71be8, aCellMap=0x7f250f1c1890) at /home/dshin/mozilla-unified/layout/tables/nsTableFrame.cpp:726
#5  0x00007f25245a424c in nsTableFrame::InsertCells(nsTArray<nsTableCellFrame*>&, int, int) (this=0x7f250bc71be8, aCellFrames=nsTArray<nsTableCellFrame*> & = {...}, aRowIndex=0, aColIndexBefore=-1) at /home/dshin/mozilla-unified/layout/tables/nsTableFrame.cpp:765
#6  0x00007f25245a406d in nsTableFrame::RowOrColSpanChanged(nsTableCellFrame*) (this=0x7f250bc71be8, aCellFrame=0x7f250bc71e98) at /home/dshin/mozilla-unified/layout/tables/nsTableFrame.cpp:365
#7  0x00007f252421af7f in nsCSSFrameConstructor::UpdateTableCellSpans(nsIContent*) (this=0x7f2511e1c310, aContent=0x7f250bd04d30) at /home/dshin/mozilla-unified/layout/base/nsCSSFrameConstructor.cpp:8401

... Which triggers here, which doesn't check for the MathML table specific attribute, columnspan (Not colspan)

nsMathMLmtdFrame would handle this, but it's flat-out not used for border-collapsed MathML tables...

Assignee: nobody → dshin
Status: NEW → ASSIGNED

Comment on attachment 9382058 [details]
Bug 1880405: Post cell span restyle event for border-collapsed MathML table cells. r=#layout-reviewers

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Despite being a small patch, reverse-engineering the condition for this issue is not straightforward:
    • Prerequisite (Border-collapsed MathML table): Involvement of border-collapse is hidden & not directly referenced in the patch.
    • Trigger (Column span that exceeds the overall size of the table gets restyled): Not directly referenced in the patch - Need to know about existence of "ghost cells" being inserted for invalid spans like this.

However, once the conditions are figured out, the exploit is not hard to create, especially intentionally.
That said, this is a null pointer dereference rather than OOB access, given the guard here) .

  • 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?: Highly localized patch, should be easy & low risk.
  • How likely is this patch to cause regressions; how much testing does it need?: As above - highly localized patch, that opens a code path for a very specific (i.e. MathML) case, so low risk.
    Attached test patch + existing border-collapsed table tests should suffice.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: Yes
Attachment #9382058 - Flags: sec-approval?
Attachment #9382059 - Flags: sec-approval?

Comment on attachment 9382058 [details]
Bug 1880405: Post cell span restyle event for border-collapsed MathML table cells. r=#layout-reviewers

Approved to land and uplift

Attachment #9382058 - Flags: sec-approval? → sec-approval+

Comment on attachment 9382059 [details]
Bug 1880405: Add test. r=#layout-reviewers

Will add a reminder date for the test; do not land the test until that date.

Attachment #9382059 - Flags: sec-approval? → sec-approval-
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed][reminder-test 2024-04-30]
Attachment #9382107 - Flags: approval-mozilla-beta?
Attachment #9382108 - Flags: approval-mozilla-release?

We're not going to need to uplift to Release, but an ESR patch & approval request would be appreciated!

Attachment #9382108 - Attachment is obsolete: true
Attachment #9382108 - Flags: approval-mozilla-release?
Attachment #9382123 - Flags: approval-mozilla-esr115?
Pushed by dshin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/50b3fac498fa Post cell span restyle event for border-collapsed MathML table cells. r=layout-reviewers,emilio
Group: layout-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Flags: needinfo?(dshin)
Resolution: --- → FIXED
Target Milestone: --- → 125 Branch

Verified bug as fixed on rev mozilla-central 20240227043210-8ea0c0ea3c7c.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
Keywords: bugmon
Attachment #9387900 - Attachment is obsolete: true

Uplift Approval Request

  • Steps to reproduce for manual QE testing: Load test case from associated bug, verify no crash
  • User impact if declined: Border-collapsed MathML table may cause a crash
  • Fix verified in Nightly: yes
  • Is Android affected?: yes
  • Code covered by automated testing: no
  • Risk associated with taking this patch: Low
  • Explanation of risk level: Highly localized patch, that opens a code path for a very specific (i.e. MathML) case, so low risk.
  • Needs manual QE test: yes
  • String changes made/needed: Low
Flags: qe-verify+

Uplift Approval Request

  • Steps to reproduce for manual QE testing: Load test case from associated bug, verify no crash
  • User impact if declined: Border-collapsed MathML table may cause a crash
  • Fix verified in Nightly: yes
  • Is Android affected?: yes
  • Code covered by automated testing: no
  • Risk associated with taking this patch: Low
  • Explanation of risk level: Highly localized patch, that opens a code path for a very specific (i.e. MathML) case
  • Needs manual QE test: yes
  • String changes made/needed: None
Attachment #9382107 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Reproduced the crash using an old Nightly build from 2024-02-02 (https://crash-stats.mozilla.org/report/index/4d838ac9-3649-4381-958a-441290240228), verified that the latest Beta 124 build from treeherder does not crash anymore using the testcase attached across platforms (Windows 11, macOS 13.6 and Ubuntu 22.04).

Flags: qe-verify+
Attachment #9382123 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+

(In reply to Bogdan Maris, Desktop QA from comment #23)

Reproduced the crash using an old Nightly build from 2024-02-02 (https://crash-stats.mozilla.org/report/index/4d838ac9-3649-4381-958a-441290240228), verified that the latest Beta 124 build from treeherder does not crash anymore using the testcase attached across platforms (Windows 11, macOS 13.6 and Ubuntu 22.04).

Same on the latest ESR 115 build from treeherder on the same OSs.

Whiteboard: [bugmon:bisected,confirmed][reminder-test 2024-04-30] → [bugmon:bisected,confirmed][reminder-test 2024-04-30][adv-main124+r]
Whiteboard: [bugmon:bisected,confirmed][reminder-test 2024-04-30][adv-main124+r] → [bugmon:bisected,confirmed][reminder-test 2024-04-30][adv-main124+r][adv-esr115.9+r]

2 months ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2024-04-30] .

dshin, please refer to the original comment to better understand the reason for the reminder.

Flags: needinfo?(dshin)
Whiteboard: [bugmon:bisected,confirmed][reminder-test 2024-04-30][adv-main124+r][adv-esr115.9+r] → [bugmon:bisected,confirmed][adv-main124+r][adv-esr115.9+r]

Comment on attachment 9382059 [details]
Bug 1880405: Add test. r=#layout-reviewers

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: N/A - This is for testcase for a fix that is already landed. The test review was given sec-approval- then to prevent landing it too early. Rebased today & confirmed still passing.
    Request for the fix itself is in bug 1880405 comment 9.
  • 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?: None
  • 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?: Since a test, can be trivially uplifted, though likely not required
  • How likely is this patch to cause regressions; how much testing does it need?: None - This is a testcase for the fixed bug.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: Yes
Flags: needinfo?(dshin)
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: