New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: heap-buffer-overflow read in filter_fuzz_stub

Reported by look.wan...@gmail.com, Oct 23 2017

Issue description

This is variant of https://bugs.chromium.org/p/chromium/issues/detail?id=740789


VERSION
Chrome Version: asan-linux-release-510655
Operating System: Ubuntu 16.04.2 LTS
(When build filter_fuzz_test, add "use_allocator="none"" to file "args.gn", or malloc would fail)


REPRODUCTION CASE
(get "poc" through unziping "poc.zip" )
run ./filter_fuzz_stub poc
(May need to wait for one minute)

==27912==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f8840511d58 at pc 0x000000b2b0f1 bp 0x7ffef2b689b0 sp 0x7ffef2b689a8
READ of size 8 at 0x7f8840511d58 thread T0
    #0 0xb2b0f0 in SkPathRef::Iter::next(SkPoint*) third_party/skia/src/core/SkPathRef.cpp:718:20
    #1 0xafe2a1 in next third_party/skia/include/core/SkPath.h:1553:36
    #2 0xafe2a1 in SkPath::addPath(SkPath const&, SkMatrix const&, SkPath::AddPathMode) third_party/skia/src/core/SkPath.cpp:1532
    #3 0xafdf2f in SkPath::addPath(SkPath const&, float, float, SkPath::AddPathMode) third_party/skia/src/core/SkPath.cpp:1520:11
    #4 0xe16aaf in SkPath1DPathEffect::next(SkPath*, float, SkPathMeasure&) const third_party/skia/src/effects/Sk1DPathEffect.cpp:175:22
    #5 0xe1594e in filterPath third_party/skia/src/effects/Sk1DPathEffect.cpp:22:36


ROOT CAUSE
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkPathRef.cpp?l=203

    for (int i = 0; i < vCount; ++i) {
        switch (verbs[i]) {
            case SkPath::kMove_Verb:
            case SkPath::kLine_Verb:
                ptCount += 1;
                break;
            case SkPath::kConic_Verb:
                conicCount += 1;
                // fall-through
            case SkPath::kQuad_Verb:
                ptCount += 2;
                break;
            case SkPath::kCubic_Verb:
                ptCount += 3;
                break;
            case SkPath::kClose_Verb:
                break;
            default:
                return false;
        }
    }
"ptCount" could be easily overflowed, and we can read  out-bound memory with size up to about 1.3G*8 
 
poc.zip
1.3 MB Download
Components: Internals>Skia

Comment 2 by tsepez@chromium.org, Oct 23 2017

Labels: M-64 Security_Severity-Medium Security_Impact-Stable Pri-2
Owner: bunge...@chromium.org
Unclear if this can be hit other than with the fuzzer, hence sev-med, otherwise would be higher.
Cc: bunge...@chromium.org
Owner: reed@chromium.org
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 24 2017

Labels: -Pri-2 Pri-1
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 24 2017

Status: Assigned (was: Unconfirmed)
Project Member

Comment 6 by sheriffbot@chromium.org, Nov 6 2017

reed: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by sheriffbot@chromium.org, Nov 20 2017

reed: Uh oh! This issue still open and hasn't been updated in the last 28 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by reed@chromium.org, Nov 20 2017

Cc: herb@google.com
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 21 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/67b190bc3f6ee0035c6bac28974f53c53ec32d6f

commit 67b190bc3f6ee0035c6bac28974f53c53ec32d6f
Author: Mike Reed <reed@google.com>
Date: Tue Nov 21 18:53:39 2017

detect too many points

Bug:777318
Change-Id: Idb52688b6ee4ae020004400da995620c1f548559
Reviewed-on: https://skia-review.googlesource.com/73821
Commit-Queue: Mike Reed <mike@reedtribe.org>
Reviewed-by: Herb Derby <herb@google.com>

[modify] https://crrev.com/67b190bc3f6ee0035c6bac28974f53c53ec32d6f/src/core/SkPathRef.cpp
[modify] https://crrev.com/67b190bc3f6ee0035c6bac28974f53c53ec32d6f/src/core/SkSafeMath.h

Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Cc: kjlubick@chromium.org kjlubick@google.com
Friendly ping from security sheriff.

reed: Is this fixed by your patch in #9 or is there more work to do? 

Comment 13 by reed@chromium.org, Jan 31 2018

Owner: reed@google.com
Status: Fixed (was: Assigned)
Labels: reward-topanel
Project Member

Comment 15 by sheriffbot@chromium.org, Feb 8

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 16 by sheriffbot@chromium.org, Feb 8

Labels: Merge-Request-65
Project Member

Comment 17 by sheriffbot@chromium.org, Feb 9

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: M65 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: awhalley@chromium.org
[Bulk Edit]

+awhalley@ (Security TPM) for M65 merge review
govind@ - good for 65
Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comment #19. Please merge ASAP so we can pick it up for next week Beta release. Thank you.
Labels: -reward-topanel reward-unpaid reward-1000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
Groovy, the VRP panel decided to award $1,000 - Cheers!
Labels: -reward-unpaid reward-inprocess
fix already in chrome/m65
Labels: -Merge-Approved-65
Labels: -M-64 Release-0-M65 M-65
Labels: CVE-2018-6071
Labels: CVE_description-missing
Project Member

Comment 29 by sheriffbot@chromium.org, May 10

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment