1454692 - (CVE-2018-5183) Backport relevant post-m55 Skia security fixes to ESR52

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Ryan VanderMeulen [:RyanVM]]

Per discussion with Milan and Dan today, we want to cherry-pick the relevant Skia security fixes which have landed upstream since the m55 release we're shipping in ESR52 rather than trying to do a wholesale uplift of m66.

Dan is going to work on compiling a spreadsheet of the fixes in question.

Comment 1

[Daniel Veditz [:dveditz]]

Working on a spreadsheet of Chrome-shipped skia security fixes:
https://docs.google.com/spreadsheets/d/1J6Ccvp2S3ftf1lzrzhiUUIGNIELG7-gj119q3pebPSI/edit#gid=0

Comment 2

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

We could still potentially uplift before the esr52 build next week. 
Should I hold back the 52.8esr build for these fixes (say, till Tuesday) or go ahead without them?

Comment 3

[Daniel Veditz [:dveditz]]

We're working on it, please give us the time.

Comment 4

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

OK, sounds fine, I will hold off on the esr52 build.

Comment 5

[Lee Salzman [:lsalzman]]

Attachment: use SkSafeMath for buffer sizes

Backport of https://skia.googlesource.com/skia/+/fe266c2bce2b8ac4ef953f16c8e1a7801da9c57d%5E%21/#F0

Comment 6

[Lee Salzman [:lsalzman]]

Attachment: limit vertex counts in GPU hairline renderer

https://skia.googlesource.com/skia/+/296de50b4c2e31f94b8c3fafae8fcd7bcfb00e0b%5E%21/#F0

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: conditionally align SkMask

https://skia.googlesource.com/skia/+/2917e0705e16e722e2ca82ff312ad0b88e0b89ec%5E%21/#F0

Comment 8

[Lee Salzman [:lsalzman]]

Okay, I went through the spreadsheet to determine which of the bugs affect us. It should only be these three, which I have put up patches for.

Comment 9

[Lee Salzman [:lsalzman]]

Comment on attachment 8971907 [details] [diff] [review]
use SkSafeMath for buffer sizes

Approval Request Comment
[Feature/Bug causing the regression]: bug 1340627
[User impact if declined]: sec-high vulnerability
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: 
[Is the change risky?]: no
[Why is the change risky/not risky?]: Upstream security fix
[String changes made/needed]: none

Comment 10

[Lee Salzman [:lsalzman]]

Comment on attachment 8971907 [details] [diff] [review]
use SkSafeMath for buffer sizes

Oops, wrong flag.

Comment 11

[Lee Salzman [:lsalzman]]

Comment on attachment 8971908 [details] [diff] [review]
limit vertex counts in GPU hairline renderer

Oops, wrong flag.

Comment 12

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

Comment on attachment 8971907 [details] [diff] [review]
use SkSafeMath for buffer sizes

sec-approval is only for trunk checkins.
Giving esr52 approval.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/f729bf78fb3a