1347262 - (CVE-2017-5467) Potential Skia overflow due to round_asymmetric_to_int bug

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Lee Salzman [:lsalzman]]

Upstream Skia security bug (https://bugs.chromium.org/p/skia/issues/detail?id=6294) details a variant of bug 1330166 that was not handled by that fix. All relevant security details of this bug should basically be the same as in bug 133016, just that this is a new way to trigger it. I've fixed up that case now with this patch that ensures the rounding is properly biased for all sides of the bounds rect.

This patch was submitted upstream here: https://skia-review.googlesource.com/c/9700/

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: fix Skia's round_asymmetric_to_int to bias all sides

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7c3b71a3cadc

Comment 4

[Lee Salzman [:lsalzman]]

Comment on attachment 8847233 [details] [diff] [review]
fix Skia's round_asymmetric_to_int to bias all sides

Approval Request Comment
[Feature/Bug causing the regression]: 51+ when we enabled Skia content.
[User impact if declined]: Possible heap overruns/crashes when using Skia content.
[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?]: This extends a clipping region to ensure it properly bounds drawing inside it, which solves the crash/sec issue, but does not otherwise change drawing appearance-wise. Applied a similar fix in bug 1330166 that has as yet caused no problems. This just extends that fix to more cases. Change has also been reviewed/upstreamed to Skia as well.
[String changes made/needed]: None

Comment 5

[Gerry Chang [:gchang]]

Comment on attachment 8847233 [details] [diff] [review]
fix Skia's round_asymmetric_to_int to bias all sides

Fix a security issue. Aurora54+ & Beta53+.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bb3a786dfee3

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/759f2057584d

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8847233 [details] [diff] [review]
fix Skia's round_asymmetric_to_int to bias all sides

skia security fix for esr52

Comment 9

[Julien Cristau [:jcristau]]

Track all the things.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/6ed26e6c1a09

Comment 11

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Lee's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 4).