Closed Bug 1338658 Opened 4 years ago Closed 4 years ago

Skia buffer overflow in analytic AA code

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr45 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected

People

(Reporter: tjr, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: sec-other, Whiteboard: gfx-noted [third-party-lib-audit])

This is a (semi-)automated bug making you aware that there is an available upgarde for an embedded third-party library.

skia is currently at version 55 in mozilla-central, and the latest version of the library released is 58.

I fetched the latest version of the library from https://skia.googlesource.com/skia/+/master/include/core/SkMilestone.h.

You can leave this bug open, and it will be updated if a newer version of the library becomes available. If you close it as WONTFIX, please indicate if you do not wish to receive any future bugs upon new releases of the library.
Priority: -- → P3
Whiteboard: gfx-noted
Skia m58 is not actually released yet. All that header means is they released 57 and updated the in-dev version to reflect that they are still working on 58.
Thanks lee, I will update the library-checking code to reflect that.
Summary: Update skia to 58 → Update skia to 57
I have a suspicion that Skia 57 fixes a security bug, based off the comments in this commit/changeset: https://skia-review.googlesource.com/c/5266/
Upgraded this bug to a security bug after confirming that that bug was a buffer overflow (https://bugs.chromium.org/p/chromium/issues/detail?id=668907) per Mike West at google.

Detailed info:
https://skia-review.googlesource.com/c/5266/ - fixed bug
https://skia.googlesource.com/skia.git/+/44be067730b9bff2027b41691587a1b0454966a1 - then reverted
https://skia.googlesource.com/skia.git/+/79252f7997f2f5b90a72d2c7bd5f6aa8a58ee640 - then relanded

And then the following two wre also tied to the chrome bug:
https://chromium.googlesource.com/chromium/src.git/+/b0622c15226e2c82d2ed5897167837bcb3130c3c
https://chromium.googlesource.com/chromium/src.git/+/25e7d9294677e6ebfc4add4d3a665d4cafa52e5c

The Skia Milestone was updated from 57 -> 58 on Jan 19, so it's my understanding that picking up M57 would include these fixes.
Group: core-security → gfx-core-security
We should file a spin-off bug for just the sec issue so we can track backporting that fix to 52/53 since I'm assuming it's highly unlikely we'd want to take a wholesale Skia update to our stable branches. Or repurpose this bug for it and file a new, non s-s bug for the Skia update.
It is not the policy of the graphics team to pull in entire Skia updates for isolated security fixes currently, as it can take weeks or months to actually orchestrate such an update. Please separate out the security bug from the update.

Probably best to rename this bug to ONLY deal with the security issue, and file a new bug about an update if you wish to keep the update as a bug, although, as noted, the graphics team has its own timeline for when and how we update Skia. We currently don't plan to update Skia till milestone 58 is official.
I looked through the bug, and this one does not affect us at all, so we're good, actually. We do not enable analytic AA in Skia, nor does the code that is patched there exist in the milestone we are using.
Given comments 6 and 7, let's change this bug back to updating to version 58 whenever the Graphics team does so. And I guess we'll want to keep this bug closed so as to not 0-day other skia consumers in the mean time.
Comment 3 is private: false
Summary: Update skia to 57 → Update skia to 58
Talked it over with Lee and he'd prefer this bug remain about the specific security issue and open a new one for the update, so I'll do that instead.
Summary: Update skia to 58 → Skia buffer overflow in analytic AA code
Depends on: 1340627
The chrome bug is public: we can unhide this one.

https://bugs.chromium.org/p/chromium/issues/detail?id=668907 is a regression from a check-in a week before. Even if we used this feature we almost certainly don't have the bad code. Nor is any other Skia consumer likely to have a mid-version pull.
Group: gfx-core-security
Status: NEW → RESOLVED
Closed: 4 years ago
Priority: P3 → --
Resolution: --- → WORKSFORME
Whiteboard: gfx-noted → gfx-noted [third-party-lib-audit]
You need to log in before you can comment on or make changes to this bug.