1626152 - use-after-free in nsFlexContainerFrame::MeasureAscentAndBSizeForFlexItem

Status: VERIFIED FIXED

(Core :: Layout: Flexbox, defect, P2)

Description

[tarafans7]

Attachment: poc.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.149 Safari/537.36

Steps to reproduce:

A use-after-free bug is triggered when fuzzing Firefox Nightly ASAN build.
The PoC and the ASAN log reproduced by ffpuppet against the official build on Mar. 30 are attached.

Wen Xu from sslab, gatech

Comment 1

[tarafans7]

Attachment: ASAN log

Comment 2

[Emilio Cobos Álvarez [:emilio]]

So this is

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1626152 - Clear cached result instead of removing the property. r=dholbert,TYLin

This saves an allocation and prevents some ::first-line + svg text edge cases
from clearing the caches.

It's really weird to invalidate intrinsic sizes on ancestors while reflowing
them, but it can happen.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9137171 [details]
Bug 1626152 - Clear cached result instead of removing the property. r=dholbert,TYLin

Security Approval Request

• How easily could an exploit be constructed based on the patch?: not too easily.
• 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?: No
• If not, how different, hard to create, and risky will they be?: I think not too hard. Would need to backport bug 1624247 part 3 or such. But nothing too scary.
• How likely is this patch to cause regressions; how much testing does it need?: not likely at all.

Comment 5

[Andrew McCreight [:mccr8]]

Setting affected flags per comment 4.

Comment 6

[Daniel Holbert [:dholbert]]

Notably: the patch makes complete sense to do, as an optimization on its own (it's a logical followup to bug 1624247).

If it's judged to be worthwhile, I'm sure emilio could spin off a public version of this bug with the patch as a drive-by improvement, with no security-related classification/notes whatsoever on that bug. That would let us land the patch & fix the issue & ride the trains, without drawing attention to it from having it land with a security-sensitive bug number.

(Not sure that's worth it -- it's just more of an option here than it is in many other sec bugs, given that the patch is trivial & makes total sense on its own, regardless of the security edge-case that it happens to help with here.)

Comment 7

[Daniel Holbert [:dholbert]]

(I'll defer to the sec-approval-granting folks on whether comment 6 is worth it.)

Comment 8

[Daniel Veditz [:dveditz]]

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

Notably: the patch makes complete sense to do, as an optimization on its own (it's a logical followup to bug 1624247).
If it's judged to be worthwhile, I'm sure emilio could spin off a public version of this bug with the patch as a drive-by improvement,

The nice thing about landing now (before Monday) is we don't need to uplift to Beta to ship in Fx76 (Fx75 is a wrap--this doesn't warrant an RC respin). But it's the weekend now -- Sorry I didn't get to the sec-approvals with the chemspill stuff going on. I'm not sure when the merge actually takes place. Do folks in Europe-time get to land some stuff in their morning before it happens?

• if you can land this before the merge then you have sec-approval to land on mozilla-central under this bug number.
• if we miss that then a public bug might be best so the uplift-to-beta doesn't look suspicious.
• Don't land on ESR until much closer to shipping, the week of April 20 at the earliest. We'll need a combo-patch that has the bits from bug 1624247 that you need.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

if you can land this before the merge then you have sec-approval to land on mozilla-central under this bug number.

I asked the sheriffs and confirmed that it'd get in before the merge, so landed https://hg.mozilla.org/integration/autoland/rev/3cf71917b15056473e97d520e96f40b9a5edb09c

ESR probably needs a rebased patch, so I'll hold off on preparing that for now. Please ni? me when you want me to provide a patch :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Go ahead and land now.

Comment 11

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

https://hg.mozilla.org/integration/autoland/rev/3cf71917b15056473e97d520e96f40b9a5edb09c
https://hg.mozilla.org/mozilla-central/rev/3cf71917b150

Comment 12

[Emilio Cobos Álvarez [:emilio]]

On ESR? I assume that's not what you meant per comment 8... I had already pushed to autoland (see comment 9).

Comment 13

[Ryan VanderMeulen [:RyanVM]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)

On ESR? I assume that's not what you meant per comment 8... I had already pushed to autoland (see comment 9).

Yeah, I missed that it was already on autoland there. Thanks for double-checking!

Comment 14

[BugBot [:suhaib / :marco]]

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.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Done

Comment 16

[Ryan VanderMeulen [:RyanVM]]

We're ready for that ESR rebase and uplift request now :)

Comment 17

[Emilio Cobos Álvarez [:emilio]]

Ah, I lie, ESR turns out not to be affected. This bug was introduced in bug 1621989 (part 3). Which is in 76, so I think we're good!

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Thanks for checking!

Comment 19

[tarafans7]

Dear developers,

I wonder if this bug will be acknowledged by a CVE number and/or qualified for security bug bounty?
Thanks a lot.

Comment 20

[Bogdan Maris, Desktop Test Engineering]

Reproduced the initial issue using the poc file attacked using an old Nightly build. Verified that the this is not reproducible anymore using latest Nightly fuzz asan build and latest Beta 76.0b9 fuzz asan on Ubuntu 18.04 64bit.

Comment 21

[Emilio Cobos Álvarez [:emilio]]

(In reply to tarafans7 from comment #19)

I wonder if this bug will be acknowledged by a CVE number and/or qualified for security bug bounty?

I don't know about the CVE, specially given it didn't affect anything else than pre-release builds, but it probably qualifies for a bounty?

See https://www.mozilla.org/en-US/security/client-bug-bounty/#claiming-a-bounty, it seems you haven't filed the bug via the bounty form, so you should probably mail security@

Comment 22

[Daniel Veditz [:dveditz]]

Tagging for the bounty committee at the reporter's request.