1507696 - ANGLE crash in copyTexSubImage from a 2d_array

Status: RESOLVED FIXED

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

Description

[Kelsey Gilbert [:jgilbert]]

Crash bug, passing -1 where we expect a small positive number. Hopefully just sec-dos.

ANGLE is *full* of bugs in this code, it seems like.

Also WebRender wants to use this codepath. Oops.

Comment 1

[Kelsey Gilbert [:jgilbert]]

https://crash-stats.mozilla.com/report/index/08bd388c-2d7d-4365-8874-859180181116

Comment 2

[Daniel Veditz [:dveditz]]

r12 has the UAF marker in it -- interesting. Does a UAF from passing a -1 make sense?

Comment 3

[Kelsey Gilbert [:jgilbert]]

Quite possibly, yeah.

Comment 4

[Jessie [:jbonisteel] pls NI]

Jeff - is there anything we should be doing to address this bug or is it stalled?

Comment 5

[Kelsey Gilbert [:jgilbert]]

Stalled for now. It's not critical.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Picking this back up.

Comment 7

[Kelsey Gilbert [:jgilbert]]

This works in Chrome Canary now, so maybe it's fixed in ANGLE now?

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: testcase

Comment 9

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1507696 - Cherry-pick CopyTexImage3D fixes.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 9074364 [details]
Bug 1507696 - Cherry-pick CopyTexImage3D fixes.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Hard. There's a ton of moving parts, and you're in ANGLE, so you're like three layers deep in abstractions.
• 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?: Likely fairly easy. Only esr60 might be more difficult.
• How likely is this patch to cause regressions; how much testing does it need?: We have a ton of tests.

Comment 11

[Kelsey Gilbert [:jgilbert]]

[Tracking Requested - why for this release]:
sec-high UAF

Comment 12

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

sec-approval+ for trunk. We'll want patches on beta and ESR60 made and nominated as well.

Comment 13

[Kelsey Gilbert [:jgilbert]]

Attachment: beta68 backport

Beta/Release Uplift Approval Request

• User impact if declined: sec-high
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• 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 risk: This was a clean cherry-pick of a targetting fix taken upstream.
• String changes made/needed: none

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/mozilla-central/rev/69b25d1fba9a

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9074637 [details] [diff] [review]
beta68 backport

Fixes a webgl sec issue by cherry-picking an upstream fix. Approved for 68rc1.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f8c40b71fcb1

Comment 17

[BugBot [:suhaib / :marco]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.