Closed Bug 1535518 Opened 2 years ago Closed 2 years ago

Possible out of bounds read in Skia

Categories

(Core :: Graphics, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr60 67+ fixed
firefox66 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected

People

(Reporter: pauljt, Assigned: lsalzman)

Details

(Keywords: csectype-bounds, sec-moderate, Whiteboard: [adv-esr60.7+])

Attachments

(1 file)

There is a skia bug mentioned in latest stable chrome release notes [1]:

$N/A][883596] Medium CVE-2019-5798: Out of bounds read in Skia. Reported by Tran Tien Hung (@hungtt28) of Viettel Cyber Security on 2018-09-13

That's https://bugs.chromium.org/p/chromium/issues/detail?id=883596 which is private but Dan has access to. From irc Dan thought the patch be:

https://skia.googlesource.com/skia/+/f026d896dce856dd3c757c4c341b2df6876e1d28

We already have this patch (bug 1502152)

Dan also thought this patch might be the test patch:
https://skia.googlesource.com/skia/+/fb5f43b09fc6194f097c5818158055ed9b74c498

Dveditz, need info for you to confirm since you have access. If its definately the first patch above, then we already have the fix. But need you to confirm.

[1] https://chromereleases.googleblog.com/2019/03/stable-channel-update-for-desktop_12.html

Flags: needinfo?(dveditz)
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED

This chrome bug was fixed by a series of fixes. We picked up two out of three pieces in bug 1498701 (referencing crbug 882423). This third bit was originally fixed based on a fuzz tests the Skia team runs as https://skia-review.googlesource.com/c/skia/+/154625 (the patch you link in comment 0) and then when crbug 883596 was reported they found they had a fix for it already. That's apparently why this bug got a CVE (external reporter) but did not get a chrome bounty (the "[$N/A]" bit) because it was an already known bug.

As Ryan points out we don't have this fix on ESR-60 because that branch is not on m71. We did land the bug 1498701 fixes there though, so we still need this one.

Flags: needinfo?(dveditz)

Comment on attachment 9051792 [details]
Bug 1535518 - Preserve fLastMoveToIndex in SkPath::transform r?rhunt

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Bounds vulnerability already disclosed by Skia/Chromium upstream.
  • User impact if declined:
  • Fix Landed on Version: 65
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Fix already in 65 release as of bug 1502152.
  • String or UUID changes made by this patch:
Attachment #9051792 - Flags: approval-mozilla-esr60?

Am I understanding this correctly from comment 4, that it's only esr60 affected and there's nothing left to do for other branches?

Flags: needinfo?(lsalzman)

Comment on attachment 9051792 [details]
Bug 1535518 - Preserve fLastMoveToIndex in SkPath::transform r?rhunt

While this is sec-moderate, let's take the fix on ESR since the vulnerability is already public.

Attachment #9051792 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #6)

Am I understanding this correctly from comment 4, that it's only esr60 affected and there's nothing left to do for other branches?

Correct.

Flags: needinfo?(lsalzman)
Group: gfx-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Whiteboard: [adv-esr60.7+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.