Closed Bug 1347262 (CVE-2017-5467) Opened 3 years ago Closed 3 years ago

Potential Skia overflow due to round_asymmetric_to_int bug

Categories

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

51 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox52 --- wontfix
firefox-esr52 53+ fixed
firefox53 + fixed
firefox54 + fixed
firefox55 + fixed

People

(Reporter: lsalzman, Assigned: lsalzman)

References

Details

(Keywords: crash, csectype-bounds, sec-moderate, Whiteboard: [gfx-noted][adv-main53+][adv-esr52.1+])

Attachments

(1 file)

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/
Attachment #8847233 - Flags: review?(jmuizelaar) → review+
Duplicate of this bug: 1347261
Group: core-security → gfx-core-security
https://hg.mozilla.org/mozilla-central/rev/7c3b71a3cadc
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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
Attachment #8847233 - Flags: approval-mozilla-release?
Attachment #8847233 - Flags: approval-mozilla-esr52?
Attachment #8847233 - Flags: approval-mozilla-beta?
Attachment #8847233 - Flags: approval-mozilla-aurora?
Comment on attachment 8847233 [details] [diff] [review]
fix Skia's round_asymmetric_to_int to bias all sides

Fix a security issue. Aurora54+ & Beta53+.
Attachment #8847233 - Flags: approval-mozilla-beta?
Attachment #8847233 - Flags: approval-mozilla-beta+
Attachment #8847233 - Flags: approval-mozilla-aurora?
Attachment #8847233 - Flags: approval-mozilla-aurora+
Comment on attachment 8847233 [details] [diff] [review]
fix Skia's round_asymmetric_to_int to bias all sides

skia security fix for esr52
Attachment #8847233 - Flags: approval-mozilla-release?
Attachment #8847233 - Flags: approval-mozilla-release-
Attachment #8847233 - Flags: approval-mozilla-esr52?
Attachment #8847233 - Flags: approval-mozilla-esr52+
Group: gfx-core-security → core-security-release
Whiteboard: [gfx-noted] → [gfx-noted][adv-main53+][adv-esr52.1+]
Setting qe-verify- based on Lee's assessment on manual testing needs and the fact that this fix has automated coverage (see Comment 4).
Flags: qe-verify-
Alias: CVE-2017-5467
Blocks: 1372421
You need to log in before you can comment on or make changes to this bug.