1522987 - <area>s don't get their frame pointers properly cleared if inside shadow trees.

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: Testcase

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1522987 - Cleanup frames for areas properly.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9039241 [details]
Bug 1522987 - Cleanup frames for areas properly.

Security Approval Request

How easily could an exploit be constructed based on the patch?

Easily, with a bit of knowledge of browser internals.

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 older supported branches are affected by this flaw?

63+ presumably

If not all supported branches, which bug introduced the flaw?

Shadow DOM

Do you have backports for the affected branches?

Yes

If not, how different, hard to create, and risky will they be?

How likely is this patch to cause regressions; how much testing does it need?

Not likely.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Al, Daniel, mind giving this a sec rating? It's effectively frame poisoning but can end up in UAF really, if the iframe's shell goes away.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 9039241 [details]
Bug 1522987 - Cleanup frames for areas properly.

Sec-approval for checkin on February 12, two weeks into the new cycle.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/autoland/rev/bac61e8bf3a1

Please request Beta approval on this when you get a chance. It grafts cleanly as-landed.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9039241 [details]
Bug 1522987 - Cleanup frames for areas properly.

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Shadow DOM

User impact if declined

Security-sensitive crashes.

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

No

Needs manual test from QE?

No

If yes, steps to reproduce

Can try the crashtest I attached to this bug if needed.

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Very minor patch doing some cleanup more consistently.

String changes made/needed

none

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/bac61e8bf3a1

Comment 9

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

Follow-up:
https://hg.mozilla.org/integration/autoland/rev/3378fd5f4f22
https://hg.mozilla.org/mozilla-central/rev/3378fd5f4f22

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Do the follow up patches mean that you'd need something similar for beta uplift as well?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Yeah, though the followup patch is just reducing an expected assertion count in a test.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9039241 [details]
Bug 1522987 - Cleanup frames for areas properly.

Avoids a crash, let's uplift for beta 8.

Comment 13

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

https://hg.mozilla.org/releases/mozilla-beta/rev/18d342734441
https://hg.mozilla.org/releases/mozilla-beta/rev/f222bec68b26

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Need to remember to land the crashtest.

Comment 15

[Cristian Craciun]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:67.0) Gecko/20100101 Firefox/67.0
Build ID: 20190220040540

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:66.0) Gecko/20100101 Firefox/66.0
Build ID: 20190218131312

Verified as fixed on the latest Nightly build (v67) and on the latest Beta build (v66.0b9).

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Do you know if I can land the crashtest now that the fix is in all release channels?

Comment 17

[Al Billings [:abillings - ex-MoCo]]

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

Do you know if I can land the crashtest now that the fix is in all release channels?

Updates have barely started so most of our users are still affected. I'd suggest waiting another week and a half.