Bugzilla

Assignee

Updated

•

3 years ago

Flags: needinfo?(emilio)

Assignee

Comment 1

•

3 years ago

This is a bad cast in Layout. In a debug build I get:

Assertion failure: IsSize(), at /home/emilio/src/moz/gecko-4/obj-debug/dist/include/mozilla/ServoStyleConsts.h:6835
#17 0x00007fb6308abc5a in MOZ_ReportAssertionFailure(char const*, char const*, int)
    (aStr=0x74 <error: Cannot access memory at address 0x74>, aFilename=0xffffffffffffffff <error: Cannot access memory at address 0xffffffffffffffff>, aLine=0)
    at /home/emilio/src/moz/gecko-4/obj-debug/dist/include/mozilla/Assertions.h:106
#18 0x00007fb630939b22 in mozilla::StyleGenericFlexBasis<mozilla::StyleGenericSize<mozilla::StyleLengthPercentageUnion> >::AsSize() const (this=this@entry=0x7fb615fa3a58)
    at /home/emilio/src/moz/gecko-4/obj-debug/dist/include/mozilla/ServoStyleConsts.h:6835
#19 0x00007fb6308b9d8d in nsIFrame::ComputeSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::EnumSet<mozilla::ComputeSizeFlag, unsigned char>)
    (this=<optimized out>, aRenderingContext=0x7fb615f97e00, aWM=..., aCBSize=<optimized out>, aAvailableISize=368720264, aMargin=<optimized out>, aBorderPadding=..., aFlags=mozilla::ComputeSizeFlags = {...})
    at /home/emilio/src/moz/gecko-4/layout/generic/nsIFrame.cpp:6193
#20 0x00007fb630868372 in mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::Maybe<mozilla::LogicalSize> const&, mozilla::Maybe<mozilla::LogicalMargin> const&, mozilla::Maybe<mozilla::LogicalMargin> const&, mozilla::LayoutFrameType) (this=<optimized out>, this@entry=0x7ffca90f8fd8, aPresContext=aPresContext@entry=0x7fb617c71c00, aContainingBlockSize=<optimized out>, aBorder=..., aPadding=..., aFrameType=mozilla::LayoutFrameType::None, 
    aFrameType@entry=mozilla::LayoutFrameType::TableWrapper) at /home/emilio/src/moz/gecko-4/layout/generic/ReflowInput.cpp:2430
#21 0x00007fb630865b8f in mozilla::ReflowInput::Init(nsPresContext*, mozilla::Maybe<mozilla::LogicalSize> const&, mozilla::Maybe<mozilla::LogicalMargin> const&, mozilla::Maybe<mozilla::LogicalMargin> const&) (
    this=this@entry=0x7ffca90f8fd8, aPresContext=0x7fb617c71c00, aContainingBlockSize=..., aBorder=..., aPadding=...) at /home/emilio/src/moz/gecko-4/layout/generic/ReflowInput.cpp:357
#22 0x00007fb630866286 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::EnumSet<mozilla::ComputeSizeFlag, unsigned char>)
    (this=0x7ffca90f8fd8, aPresContext=0x7ffca90f6950, aParentReflowInput=<optimized out>, aFrame=0x7fb615fa1c48, aAvailableSpace=..., aContainingBlockSize=..., aFlags=mozilla::ReflowInput::InitFlags = {...}, aComputeSizeFlags=mozilla::ComputeSizeFlags = {...}) at /home/emilio/src/moz/gecko-4/layout/generic/ReflowInput.cpp:216
#23 0x00007fb6308bcef5 in nsFlexContainerFrame::GenerateFlexItemForChild(nsFlexContainerFrame::FlexLine&, nsIFrame*, mozilla::ReflowInput const&, nsFlexContainerFrame::FlexboxAxisTracker const&, bool)
    (this=this@entry=0x7fb615fa1b98, aLine=..., aChildFrame=0x7fb615fa1c48, aParentReflowInput=..., aAxisTracker=..., aHasLineClampEllipsis=false) at /home/emilio/src/moz/gecko-4/layout/generic/nsFlexContainerFrame.cpp:1285
#24 0x00007fb6308c200a in nsFlexContainerFrame::GenerateFlexLines(mozilla::ReflowInput const&, int, nsTArray<nsFlexContainerFrame::StrutInfo> const&, nsFlexContainerFrame::FlexboxAxisTracker const&, int, bool, nsTArray<nsIFrame*>&, nsTArray<nsFlexContainerFrame::FlexLine>&)
    (this=0x7fb615fa1b98, aReflowInput=<optimized out>, aContentBoxMainSize=<optimized out>, aStruts=const nsTArray<nsFlexContainerFrame::StrutInfo> &, aAxisTracker=..., aMainGapSize=0, aHasLineClampEllipsis=<optimized out>, aPlaceholders=nsTArray<nsIFrame*> &, aLines=nsTArray<nsFlexContainerFrame::FlexLine> & = {...}) at /home/emilio/src/moz/gecko-4/layout/generic/nsFlexContainerFrame.cpp:4004
#25 0x00007fb6308c3fec in nsFlexContainerFrame::DoFlexLayout(mozilla::ReflowInput const&, int&, int&, int&, nsTArray<nsFlexContainerFrame::FlexLine>&, nsTArray<nsFlexContainerFrame::StrutInfo>&, nsTArray<nsIFrame*>&, nsFlexContainerFrame::FlexboxAxisTracker const&, int, int, int, bool, ComputedFlexContainerInfo*)
    (this=this@entry=0x7fb615fa1b98, aReflowInput=..., aContentBoxMainSize=@0x7ffca90f9688: 1073741823, aContentBoxCrossSize=@0x7ffca90f9684: -1431655766, aFlexContainerAscent=@0x7ffca90f9680: -1431655766, aLines=nsTArray<nsFlexContainerFrame::FlexLine> & = {...}, aStruts=nsTArray<nsFlexContainerFrame::StrutInfo> &, aPlaceholders=nsTArray<nsIFrame*> &, aAxisTracker=..., aMainGapSize=0, aCrossGapSize=0, aConsumedBSize=0, aHasLineClampEllipsis=<optimized out>, aContainerInfo=0x0) at /home/emilio/src/moz/gecko-4/layout/generic/nsFlexContainerFrame.cpp:4920
#26 0x00007fb6308c3178 in nsFlexContainerFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)
    (this=0x7fb615fa1b98, aPresContext=0x7fb617c71c00, aReflowOutput=..., aReflowInput=<optimized out>, aStatus=<optimized out>) at /home/emilio/src/moz/gecko-4/layout/generic/nsFlexContainerFrame.cpp:4430
#27 0x00007fb630968a30 in nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) (this=0x7ffca90f9c18, aFrame=<optimized out>, aReflowStatus=..., aMetrics=0x0, aPushedFrame=@0x7ffca90f9a1b: false)
    at /home/emilio/src/moz/gecko-4/layout/generic/nsLineLayout.cpp:875
#28 0x00007fb630890375 in nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*)
    (this=this@entry=0x7fb615fa1ad0, aState=..., aLineLayout=..., aLine=..., aFrame=aFrame@entry=0x7fb615fa1b98, aLineReflowStatus=0x7ffca90f9afc) at /home/emilio/src/moz/gecko-4/layout/generic/nsBlockFrame.cpp

Status: UNCONFIRMED → NEW

Component: CSS Parsing and Computation → Layout

Ever confirmed: true

Assignee

Comment 2

•

3 years ago

The logic has been wrong here all the way since bug 1455976, but it probably didn't cause issues until bug 1527410, which changed how we represent flex-basis values.

Quite scary that our fuzzers haven't found this before...

Component: Layout → Layout: Flexbox

Regressed by: 1527410

Assignee

Updated

•

3 years ago

Assignee: nobody → emilio

Flags: needinfo?(emilio)

Assignee

Comment 3

•

3 years ago

Attached file Bug 1681022 - Ignore flex-basis: content on a table wrapper. r=dholbert — Details

Assignee

Updated

•

3 years ago

Regressed by: 1455976

Assignee

Comment 4

•

3 years ago

Comment on attachment 9191593 [details]
Bug 1681022 - Ignore flex-basis: content on a table wrapper. r=dholbert

Beta/Release Uplift Approval Request

User impact if declined: sec-high/crit, probably
Is this code covered by automated tests?: Yes
Has the fix been verified in Nightly?: No
Needs manual test from QE?: Yes
If yes, steps to reproduce: open test-case
List of other uplifts needed: none
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): Simple fix to a bad type cast when some CSS is applied to a table.
String changes made/needed: none

ESR Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: see above
Fix Landed on Version:
Risk to taking this patch: Low
Why is the change risky/not risky? (and alternatives if risky): see above
String or UUID changes made by this patch: none

Attachment #9191593 - Flags: approval-mozilla-esr78?

Attachment #9191593 - Flags: approval-mozilla-beta?

Assignee

Updated

•

3 years ago

Flags: qe-verify+

Assignee

Comment 5

•

3 years ago

Comment on attachment 9191593 [details]
Bug 1681022 - Ignore flex-basis: content on a table wrapper. r=dholbert

Security Approval Request

How easily could an exploit be constructed based on the patch?: somewhat easily I suspect.
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 older supported branches are affected by this flaw?: all
If not all supported branches, which bug introduced the flaw?: None
Do you have backports for the affected branches?: Yes
If not, how different, hard to create, and risky will they be?: Should apply cleanly modulo file moves or what not.
How likely is this patch to cause regressions; how much testing does it need?: Fix is trivial.

Attachment #9191593 - Flags: sec-approval?

Updated

•

3 years ago

Keywords: sec-high

Comment 6

•

3 years ago

Comment on attachment 9191593 [details]
Bug 1681022 - Ignore flex-basis: content on a table wrapper. r=dholbert

approved to land if we can take it in the RC, otherwise we should probably wait until January to land.

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

Updated

•

3 years ago

status-firefox83: --- → wontfix

status-firefox84: --- → affected

status-firefox85: --- → affected

status-firefox-esr78: --- → affected

tracking-firefox84: --- → +

tracking-firefox85: --- → +

tracking-firefox-esr78: --- → 84+

https://hg.mozilla.org/integration/autoland/rev/9ecb4b649c1f3b350bdba81d4bef8ad3bc3e8065
https://hg.mozilla.org/integration/autoland/rev/4bb72f41a1dddf3679c065957cc60f38a96c4aae

Comment 7

•

3 years ago

•

Edited

Comment 8

•

3 years ago

Comment on attachment 9191593 [details]
Bug 1681022 - Ignore flex-basis: content on a table wrapper. r=dholbert

Approved for 84.0rc1 and 78.6esr.

Attachment #9191593 - Flags: approval-mozilla-release+

Attachment #9191593 - Flags: approval-mozilla-esr78?

Attachment #9191593 - Flags: approval-mozilla-esr78+

Attachment #9191593 - Flags: approval-mozilla-beta?

https://hg.mozilla.org/releases/mozilla-release/rev/1d16ad126146

Comment 9

•

3 years ago

uplift

status-firefox84: affected → fixed

Assignee

Comment 10

•

3 years ago

Attached patch esr.patch — Details — Splinter Review

https://hg.mozilla.org/releases/mozilla-esr78/rev/8e6813a535da

Comment 11

•

3 years ago

uplift

status-firefox-esr78: affected → fixed

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

Comment 12

•

3 years ago

https://hg.mozilla.org/mozilla-central/rev/9ecb4b649c1f
https://hg.mozilla.org/mozilla-central/rev/4bb72f41a1dd

Group: layout-core-security → core-security-release

Status: NEW → RESOLVED

Closed: 3 years ago

status-firefox85: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → 85 Branch

Cornel Ionce [:noni] [Hubs QA]

Updated

•

3 years ago

Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main84+]

Daniel Veditz [:dveditz]

Updated

•

3 years ago

Flags: sec-bounty? → sec-bounty+

Updated

•

3 years ago

QA Whiteboard: [qa-triaged]

Ciprian Georgiu, Desktop QA

Comment 13

•

3 years ago

I've reproduced this bug using the testcase from comment 0, on an affected Nightly build 85.0a1 (2020-12-06).

The crash is not reproducing anymore on the latest asan builds: ESR 78.6, RC1 84.0 and latest Nightly 85.0a1. This was tested on Ubuntu 18.04 x84.

Status: RESOLVED → VERIFIED

status-firefox84: fixed → verified

status-firefox85: fixed → verified

status-firefox-esr78: fixed → verified

Flags: qe-verify+

BugBot [:suhaib / :marco/ :calixte]

Comment 14

•

3 years ago

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Flags: needinfo?(emilio)

Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main84+] → [reporter-external] [client-bounty-form] [verif?][adv-main84+][sec-survey]

Assignee

Comment 15

•

3 years ago

Done

Flags: needinfo?(emilio)

Assignee

Comment 16

•

3 years ago

Ok, Jason and I did a bit more digging to see why our fuzzers hadn't triggered this.

Turns out that even though the logic bug is there since forever, this is a relatively recent regression, from bug 1673006, after all, which made this codepath possible.

Now, release builds have a different regression range where it bisects to bug 1680172. So it looks like:

Logic bug is ancient, but...
Only bug 1673006 allows it to happen (content can't access the selector tweaked in that bug), and...
Before bug 1680172, Rust was initializing the values differently, somehow, in a way that it didn't trigger an exploitable crash...

Regressed by: 1673006
No longer regressed by: 1455976, 1527410

BMO Automation

Updated

•

3 years ago

Has Regression Range: --- → yes

Mike Hommey [:glandium]

Comment 17

•

3 years ago

Note that 1.48 changed things wrt uninitialized memory.

Comment 18

•

3 years ago

Attached file advisory.txt (obsolete) — Details

Comment 19

•

3 years ago

Attached file advisory.txt — Details

Attachment #9192285 - Attachment is obsolete: true

Updated

•

3 years ago

Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main84+][sec-survey] → [reporter-external] [client-bounty-form] [verif?][adv-main84+][sec-survey][adv-esr78.6+]