New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: heap overflow write in filter_fuzz_stub(x86)

Reported by look.wan...@gmail.com, Sep 11 2017

Issue description

This is variant of https://bugs.chromium.org/p/chromium/issues/detail?id=749147.
Just found out the patch for  issue 749147  is not enough



VERSION

Chrome Version: latest build of filter_fuzz_stub(x86-asan)
(args.gn:
target_cpu = "x86"
is_asan=true
) ;
asan-v8-arm-linux-release-496287(latest v8-arm-asan version) 


Operating System: Ubuntu 16.04.2 LTS




REPRODUCTION CASE

1)asan-v8-arm-linux-release-496287
Run "./filter_fuzz_stub poc": 
0912/003744.389552:INFO:filter_fuzz_stub.cc(60)] Test case: /tmp/poc
[0912/003744.510459:INFO:filter_fuzz_stub.cc(37)] Valid stream detected.
=================================================================
==32078==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf4b1162f at pc 0x0911bb2d bp 0xffc58f48 sp 0xffc58f40
WRITE of size 1 at 0xf4b1162f thread T0
    #0 0x911bb2c in SkMaskBlurFilter::blurOneScanGauss(SkMaskBlurFilter::FilterInfo, unsigned char const*, unsigned int, unsigned char const*, unsigned char*, unsigned int, unsigned char*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:313:14
    #1 0x911a6b9 in SkMaskBlurFilter::blurOneScan(SkMaskBlurFilter::FilterInfo, unsigned char const*, unsigned int, unsigned char const*, unsigned char*, unsigned int, unsigned char*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:211:15
    #2 0x9119d11 in SkMaskBlurFilter::blur(SkMask const&, SkMask*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:160:19
    #3 0x90d9aac in SkBlurMask::BoxBlur(SkMask*, SkMask const&, float, SkBlurStyle, SkBlurQuality, SkIPoint*, bool) third_party/skia/src/effects/SkBlurMask.cpp:596:25
    #4 0x8d058ab in SkEmbossMaskFilter::filterMask(SkMask*, SkMask const&, SkMatrix const&, SkIPoint*) const third_party/skia/src/effects/SkEmbossMaskFilter.cpp:64:10

2)latest build of filter_fuzz_stub(x86-asan)
Run "./filter_fuzz_stub poc": 
[0912/003744.389552:INFO:filter_fuzz_stub.cc(60)] Test case: /tmp/poc
[0912/003744.510459:INFO:filter_fuzz_stub.cc(37)] Valid stream detected.
=================================================================
==32078==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf4b1162f at pc 0x0911bb2d bp 0xffc58f48 sp 0xffc58f40
WRITE of size 1 at 0xf4b1162f thread T0
    #0 0x911bb2c in SkMaskBlurFilter::blurOneScanGauss(SkMaskBlurFilter::FilterInfo, unsigned char const*, unsigned int, unsigned char const*, unsigned char*, unsigned int, unsigned char*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:313:14
    #1 0x911a6b9 in SkMaskBlurFilter::blurOneScan(SkMaskBlurFilter::FilterInfo, unsigned char const*, unsigned int, unsigned char const*, unsigned char*, unsigned int, unsigned char*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:211:15
    #2 0x9119d11 in SkMaskBlurFilter::blur(SkMask const&, SkMask*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:160:19
    #3 0x90d9aac in SkBlurMask::BoxBlur(SkMask*, SkMask const&, float, SkBlurStyle, SkBlurQuality, SkIPoint*, bool) third_party/skia/src/effects/SkBlurMask.cpp:596:25
    #4 0x8d058ab in SkEmbossMaskFilter::filterMask(SkMask*, SkMask const&, SkMatrix const&, SkIPoint*) const third_party/skia/src/effects/SkEmbossMaskFilter.cpp:64:10



Root Cause:

-----------------------------------------------------
define SkAlign4(x)     (((x) + 3) >> 2 << 2)
-----------------------------------------------------
int8_t* SkMask::AllocImage(size_t size) {
#ifdef TRACK_SKMASK_LIFETIME
    SkDebugf("SkMask::AllocImage %d\n", gCounter++);
#endif
    return (uint8_t*)sk_malloc_throw(SkAlign4(size));
}


-----------------------------------------------------
If size is close 0xffffffff, int overflow will happen in "SkAlign4(size)" 


About fix:
As I said last time:
Use function "computeImageSize" to calculate the size;
Or limit the value of "size" to be between  "0 - int max" not "0 - unsigned int max" 
 
poc
520 bytes View Download


2)latest build of filter_fuzz_stub(x86-asan)
Run "./filter_fuzz_stub poc": 

[0912/003711.635448:INFO:filter_fuzz_stub.cc(60)] Test case: /tmp/poc
[0912/003711.643166:INFO:filter_fuzz_stub.cc(37)] Valid stream detected.
=================================================================
==32068==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xef71168f at pc 0xf4f3d029 bp 0xfff6ae68 sp 0xfff6ae60
WRITE of size 1 at 0xef71168f thread T0
    #0 0xf4f3d028 in PlanGauss::Gauss::blur(unsigned char const*, unsigned int, unsigned char const*, unsigned char*, unsigned int, unsigned char*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:429:22
    #1 0xf4f35ab5 in SkMaskBlurFilter::blur(SkMask const&, SkMask*) const third_party/skia/src/core/SkMaskBlurFilter.cpp:619:20
    #2 0xf57376e8 in SkBlurMask::BoxBlur(SkMask*, SkMask const&, float, SkBlurStyle, SkBlurQuality, SkIPoint*, bool) third_party/skia/src/effects/SkBlurMask.cpp:596:25
    #3 0xf5782437 in SkEmbossMaskFilter::filterMask(SkMask*, SkMask const&, SkMatrix const&, SkIPoint*) const third_party/skia/src/effects/SkEmbossMaskFilter.cpp:64:10
    #4 0xf4f43606 in SkMaskFilter::filterPath(SkPath const&, SkMatrix const&, SkRasterClip const&, SkBlitter*, SkStrokeRec::InitStyle) const third_party/skia/src/core/SkMaskFilter.cpp:269:16
    #5 0xf4df9d38 in SkDraw::drawDevPath(SkPath const&, SkPaint const&, bool, SkBlitter*, bool) const third_party/skia/src/core/SkDraw.cpp:983:36


Sorry for the mistake
Cc: hcm@chromium.org herb@chromium.org reed@google.com
Components: Internals>Skia
Labels: Security_Severity-High Security_Impact-Stable Pri-1
Owner: herb@chromium.org
Copying some labels from  https://crbug.com/749147 , since this is a new fix for it.


Project Member

Comment 4 by sheriffbot@chromium.org, Sep 12 2017

Labels: M-61
Project Member

Comment 5 by sheriffbot@chromium.org, Sep 12 2017

Status: Assigned (was: Unconfirmed)

Comment 6 by herb@chromium.org, Sep 21 2017

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/2917e0705e16e722e2ca82ff312ad0b88e0b89ec

commit 2917e0705e16e722e2ca82ff312ad0b88e0b89ec
Author: Herb Derby <herb@google.com>
Date: Thu Sep 21 20:52:06 2017

Remove unsafe align4 call

Align by 4 safely before calling malloc.

BUG= chromium:763972 

Change-Id: I9278ae0df516adf3eb96fc390ad7318a47cbefa4
Reviewed-on: https://skia-review.googlesource.com/49760
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Herb Derby <herb@google.com>

[modify] https://crrev.com/2917e0705e16e722e2ca82ff312ad0b88e0b89ec/src/core/SkMask.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/514364c76cf143604cb3184c13cee4dc404ebfa3

commit 514364c76cf143604cb3184c13cee4dc404ebfa3
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Fri Sep 22 03:47:04 2017

Roll src/third_party/skia/ cfbbcbe52..a83b903b6 (4 commits)

https://skia.googlesource.com/skia.git/+log/cfbbcbe52624..a83b903b601a

$ git log cfbbcbe52..a83b903b6 --date=short --no-merges --format='%ad %ae %s'
2017-09-21 angle-deps-roller Roll skia/third_party/externals/angle2/ d48261595..f32cd0b78 (1 commit)
2017-09-21 mtklein ok, add via_skp
2017-09-21 angle-deps-roller Roll skia/third_party/externals/angle2/ 1f9d68437..d48261595 (1 commit)
2017-09-21 herb Remove unsafe align4 call

Created with:
  roll-dep src/third_party/skia
BUG= 763972 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=fmalita@chromium.org

Change-Id: I68df7307985eac3dd532e134701f0ef14ab3b63f
Reviewed-on: https://chromium-review.googlesource.com/677856
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503640}
[modify] https://crrev.com/514364c76cf143604cb3184c13cee4dc404ebfa3/DEPS

Status: Fixed (was: Started)
Never understand why people forget to mark stuff as fixed.

Comment 10 by herb@chromium.org, Sep 26 2017

Thanks for marking it fixed for me.
Labels: reward-topanel
Project Member

Comment 12 by sheriffbot@chromium.org, Sep 27 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -reward-topanel reward-unpaid reward-5000
*** 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.
*********************************
Nice! $5,000 for this report :-)
Labels: -reward-unpaid reward-inprocess
Labels: -M-61 Release-0-M63 M-63
Labels: CVE-2017-15409
Project Member

Comment 18 by sheriffbot@chromium.org, Dec 15

Labels: Merge-Request-64
Project Member

Comment 19 by sheriffbot@chromium.org, Dec 15

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
please add impacted OS's
Labels: -Merge-Review-64
Project Member

Comment 22 by sheriffbot@chromium.org, Jan 3

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
Cc: kjlubick@chromium.org kjlubick@google.com
Project Member

Comment 24 by sheriffbot@chromium.org, Mar 27

Labels: -M-63 M-65
Labels: CVE_description-missing

Sign in to add a comment