1550655 - Cherry-pick fixes to angle-66

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P1)

Comment 1

[Kelsey Gilbert [:jgilbert]]

Before:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0dd27f1b3f0733d2c6e9e0756ca1b569a0d6ab2

After:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd5302d7198456f91f0e73062962e63f8479818e

Looks green.

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1550655 - Cherry-pick fixes to angle-66.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9063958 [details]
Bug 1550655 - Cherry-pick fixes to angle-66.

Beta/Release Uplift Approval Request

• User impact if declined: sec-high, sec-high, sec-moderate
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low, but not zero risk. We're taking these as cherry-picks, so while they seem to pass our tests, and they're the fixes taken upstream, non-trivial cherry-picks are inherently somewhat risky.
• String changes made/needed: none

Comment 4

[Pascal Chevrel:pascalc (PTO until April 26)]

I believe per https://wiki.mozilla.org/Security/Bug_Approval_Process that this patch should get sec-approval+ before uplifting so as that it gets on our security team radar. Jeff could you request it please? Thanks

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9063958 [details]
Bug 1550655 - Cherry-pick fixes to angle-66.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The main one that's obvious is the 2GB limit this adds, so exploiting it would just be a matter of creating a too-large buffer. That said, it's more of a vector than an exploit ready-to-go.
• 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?: esr60
• 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?: Should be fairly easy to create.
• How likely is this patch to cause regressions; how much testing does it need?: Low non-zero regression risk. We have quite good CI coverage, but we don't have an opportunity to bake these changes on Nightly first. (Nighly68 got an whole-ANGLE update. While that includes these patches, cherry-picks are always somewhat risky.

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9063958 [details]
Bug 1550655 - Cherry-pick fixes to angle-66.

This is coming way too late (we are building RC today) for 67 and it seems risky to me as we have zero bake time on Beta to make sure we are not breaking the web, so I am not going to take these uplifts, sorry.

Comment 7

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

Comment on attachment 9063958 [details]
Bug 1550655 - Cherry-pick fixes to angle-66.

sec-approval for trunk so we don't have to manually merge to beta after we fork shortly. These are known issues.

I agree that this isn't necessarily safe to shove into the release at the last moment.

Comment 9

[Andrew McCreight [:mccr8]]

Marking sec-high per comment 3.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Since 67 went WONTFIX, I don't think we should land this on ESR60 until 68 hits release. Does that sound right :dveditz?

Comment 11

[Kelsey Gilbert [:jgilbert]]

Offline discussion with dveditz concluded that we'd aim for ESR60 when we hit Release68, next cycle.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

I'm a bit confused about the status of this bug. Wouldn't we want to get this on ESR60 now so it ships in the 60.8esr release alongside Fx68? If so, we should proceed with an ESR60 approval request.

Comment 13

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Bug-1550655-Cherry-pick-fixes-for-angle-66.-r-lsalzm.patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-high
• User impact if declined: sec-high UAFs, fixed and published in Chrome.
• Fix Landed on Version: none, unaffected on 68
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): There's some risk the UAF fix for the VAO unbinding is insufficient. (cherry-pick was a little messy due to age of ANGLE in ESR60)
• String or UUID changes made by this patch: none

Comment 14

[Kelsey Gilbert [:jgilbert]]

I ran the provided test for "Fix OOB access for dynamic attribs with offsets" after cherry-pick conflicts and it passed.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9073727 [details] [diff] [review]
0001-Bug-1550655-Cherry-pick-fixes-for-angle-66.-r-lsalzm.patch

Fixes some known ANGLE security issues via upstream cherry-picks. Fx68 already has these fixes via a wholesale upstream update. Approved for 60.8esr so the fixes go out to all users at the same time.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/39861a66e665