Tom Ritter [:tjr] (OOTO until mid-August) Reporter
	Description • 7 years ago

This is a (semi-)automated bug making you aware that there is an available upgrade for an embedded third-party library. 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.

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

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

	Tom Ritter [:tjr] (OOTO until mid-August) Reporter
	Comment 1 • 7 years ago

This is a (semi-)automated bug making you aware that there is an available upgrade for an embedded third-party library. 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.

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

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

	Daniel Veditz [:dveditz]
	Comment 2 • 6 years ago

What can we do to get back on the Skia train? Our version is a year old and the library's been under active bug fixing.

	Lee Salzman [:lsalzman] Assignee
	Comment 3 • 6 years ago

(In reply to Daniel Veditz [:dveditz] from comment #2)
> What can we do to get back on the Skia train? Our version is a year old and
> the library's been under active bug fixing.

The update is in progress behind the scenes. Unfortunately, Skia updates are never simple drop-ins. It is a substantial engineering effort and is taking a couple of months to work through all of the upstream bugs we've turned up during the process. Patience is advised. We will get there soon.

	Daniel Veditz [:dveditz]
	Comment 4 • 6 years ago

There are a couple dozen published security vulnerabilities in our version of Skia
https://nvd.nist.gov/vuln/search/results?adv_search=false&form_type=basic&results_type=overview&search_type=all&query=Skia

Plus some that are not yet public like bug 1420738 and bug 1441941.

bug 1441941 has a May disclosure date from the reporter which means Fx60 needs to be the target.

	Lee Salzman [:lsalzman] Assignee
	Comment 5 • 6 years ago

The update to Skia milestone 66 is being dealt with in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1444506

Patches up for review. Estimated it will probably will land early in 61. Would need an uplift to 60 if desired once it is in.

It needs extensive testing so it wouldn't really be safe to uplift beyond 60, where we at least have time to let it simmer in beta.

	Tom Ritter [:tjr] (OOTO until mid-August) Reporter
	Comment 6 • 6 years ago

I know this is being worked on; I just wanted to add the data I got from running my script on the commits since our current version:

441 issues with a keyword were identified. Of those 209 were especially interesting, linking to a restricted bug or mentioning 'overflow'.

	Lee Salzman [:lsalzman] Assignee
	Comment 7 • 6 years ago

Approval Request Comment
[Feature/Bug causing the regression]: bug 1340627
[User impact if declined]: Many year old security vulnerabilities that have long since been disclosed by Google/Chrome lurking around in 60 ESR for the entire year, with it not being very tenable to fix them individually.
[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]: Bug 1444506
[Is the change risky?]: Somewhat. 
[Why is the change risky/not risky?]: We might experience some potential rendering bugs as a result of changes in Skia. However, after extensive discussion with Daniel Veditz and Milan Sreckovic, this is outweighed by the risk of shipping with up to 2 year old disclosed security vulnerabilities in the new 60 ESR. We don't have much choice but to do this uplift to protect our users. So long as we complete this uplift early we will have over a month left to test in beta to verify that there are no objectionable rendering regressions or other bugs.
[String changes made/needed]: None

	Julien Cristau [:jcristau]
	Comment 8 • 6 years ago

Comment on attachment 8960647 [details]
Bug 1444506 - Update Skia to milestone 66

ok let's do this :/

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 6 years ago
uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/6591b4cc0933a80c8c1f75b8fc562385b6518a9c
https://hg.mozilla.org/releases/mozilla-beta/rev/fed09e8e9884ea4b5b0cc6faddd568fdc76054e9
https://hg.mozilla.org/releases/mozilla-beta/rev/e4b6d0d4c44329564ad771a707ecf2847f556fe3
https://hg.mozilla.org/releases/mozilla-beta/rev/18cc24b781fb4ca01031677d22fdff10c5e41e80
https://hg.mozilla.org/releases/mozilla-beta/rev/91ea59ff7425cdc67aa77a3a0b46aadba165f10d
https://hg.mozilla.org/releases/mozilla-beta/rev/0c7ea7d9aede9a18cecc52c563d93f4dae449cca
https://hg.mozilla.org/releases/mozilla-beta/rev/2d7dff9cd84c9edfdcbf5b28c2d61058a46e0c26
https://hg.mozilla.org/releases/mozilla-beta/rev/86e67789dfa793bc51da46ce5eb1dfbe8611cdae
https://hg.mozilla.org/releases/mozilla-beta/rev/2bf3d2cdef340f1352ec2a0e91ded10b4e579236
https://hg.mozilla.org/releases/mozilla-beta/rev/23a13199eb6e47552be72347e96d5fab45378209

	Daniel Veditz [:dveditz]
	Comment 10 • 6 years ago

(In reply to Daniel Veditz [:dveditz] from comment #4)
> bug 1441941 has a May disclosure date from the reporter which means Fx60
> needs to be the target.

ESR-52 is supported until late summer (and many skia vulnerabilities are already public). We can't simply "wontfix" this for that branch without a plan. Options include
 * painful uplift of skia to ESR-52 (might be less painful than merging to trunk though)
 * even more painful cherry-picking of all skia vulns we can find announced
 * communication plan for explaining why "support" for ESR doesn't include published vulns in skia

	Lee Salzman [:lsalzman] Assignee
	Comment 11 • 6 years ago

(In reply to Daniel Veditz [:dveditz] from comment #10)
> (In reply to Daniel Veditz [:dveditz] from comment #4)
> > bug 1441941 has a May disclosure date from the reporter which means Fx60
> > needs to be the target.
> 
> ESR-52 is supported until late summer (and many skia vulnerabilities are
> already public). We can't simply "wontfix" this for that branch without a
> plan. Options include
>  * painful uplift of skia to ESR-52 (might be less painful than merging to
> trunk though)
>  * even more painful cherry-picking of all skia vulns we can find announced
>  * communication plan for explaining why "support" for ESR doesn't include
> published vulns in skia

Milan, please weigh in on this.

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 12 • 6 years ago

Valid points.  Dan, do we have precedent here?  Some of these vulnerabilities are "old", does that make a difference?  Is there a fourth (fifth) option? For example, a backwards choice of keeping these fixes out of 60 until 60 ESR takes over from 52?

	Daniel Veditz [:dveditz]
	Comment 13 • 6 years ago

Keeping the fixes out of 60 makes it worse! I'm not worried about private Skia security bugs _we_ know about, I'm worried about the public ones for which advisories and bugs have been published by the Chrome team over the past year. That's out there whatever we do.

https://bugs.chromium.org/p/chromium/issues/list?can=1&q=component%3AInternals%3ESkia+label%3ASecurity_Severity-critical%2CSecurity_Severity-high

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 6 years ago

Per discussion with Dan and Milan today, ESR52 is not going to get a wholesale update in favor of cherry-picking relevant fixes instead. Bug 1454692 has been filed for that.