New issue
Advanced search Search tips

Issue 787718 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: Bug



Sign in to add a comment

NULL Deref (read) in SkColorSpace::gammaIsLinear

Project Member Reported by metzman@chromium.org, Nov 22 2017

Issue description

This is another bug found by my filter_proto_fuzzer
Although it is only a NULL deref, fixing it would be really helpful, since the fuzzer's ability to find more important bugs is hampered by this bug, because the fuzzer runs into this bug very frequently.

REPRODUCTION CASE

1. Build filter_fuzz_stub with these arguments:
enable_nacl = false
ffmpeg_branding = "ChromeOS"
pdf_enable_xfa = true
proprietary_codecs = true
use_goma = true
is_debug = false
is_asan = true
optimize_for_fuzzing=true

2. Run it on the attached input (skia-null-read)

I've found that a crash will happen if filter_fuzz_stub is built with ASAN, MSAN or no sanitizers at all.

Here is the stack trace from running an ASAN instrumented filter_fuzz_stub on skia-null-read:

[1121/231011.537621:INFO:filter_fuzz_stub.cc(61)] Test case: skia-null-read
[1121/231011.593922:INFO:filter_fuzz_stub.cc(38)] Valid stream detected.
AddressSanitizer:DEADLYSIGNAL
=================================================================
==185673==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000a377ed bp 0x7ffc0df57c70 sp 0x7ffc0df57c60 T0)
==185673==The signal is caused by a READ memory access.
==185673==Hint: address points to the zero page.
    #0 0xa377ec in SkColorSpace::gammaIsLinear() const third_party/skia/src/core/SkColorSpace.cpp:250:26
    #1 0xdf2824 in SkToSRGBColorFilter::onAppendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*, bool) const third_party/skia/src/effects/SkToSRGBColorFilter.cpp:27:25
    #2 0xb85b05 in SkRasterPipelineBlitter::Create(SkPixmap const&, SkPaint const&, SkArenaAlloc*, SkRasterPipeline const&, SkShaderBase::Context*, bool, bool) third_party/skia/src/core/SkRasterPipelineBlitter.cpp:167:22
    #3 0xb8534d in SkCreateRasterPipelineBlitter(SkPixmap const&, SkPaint const&, SkMatrix const&, SkArenaAlloc*) third_party/skia/src/core/SkRasterPipelineBlitter.cpp:101:16
    #4 0xa927d8 in SkBlitter::Choose(SkPixmap const&, SkMatrix const&, SkPaint const&, SkArenaAlloc*, bool) third_party/skia/src/core/SkBlitter.cpp:1001:24
    #5 0xa62bb1 in SkAutoBlitterChoose::SkAutoBlitterChoose(SkPixmap const&, SkMatrix const&, SkPaint const&, bool) third_party/skia/src/core/SkAutoBlitterChoose.h:25:20
    #6 0xa66d20 in SkDraw::drawRect(SkRect const&, SkPaint const&, SkMatrix const*, SkRect const*) const third_party/skia/src/core/SkDraw.cpp:802:29
    #7 0xa2b527 in SkBitmapDevice::drawRect(SkRect const&, SkPaint const&) third_party/skia/src/core/SkBitmapDevice.cpp:195:18
    #8 0xa1b6d5 in SkCanvas::onDrawRect(SkRect const&, SkPaint const&) third_party/skia/src/core/SkCanvas.cpp:2029:27
    #9 0xa15dba in SkCanvas::drawRect(SkRect const&, SkPaint const&) third_party/skia/src/core/SkCanvas.cpp:1710:11
    #10 0xde0385 in SkPaintImageFilter::onFilterImage(SkSpecialImage*, SkImageFilter::Context const&, SkIPoint*) const third_party/skia/src/effects/SkPaintImageFilter.cpp:66:13
    #11 0xab6e94 in SkImageFilter::filterImage(SkSpecialImage*, SkImageFilter::Context const&, SkIPoint*) const third_party/skia/src/core/SkImageFilter.cpp:213:40
    #12 0xa2d80e in SkBitmapDevice::drawSpecial(SkSpecialImage*, int, int, SkPaint const&, SkImage*, SkMatrix const&) third_party/skia/src/core/SkBitmapDevice.cpp:421:33
    #13 0xa10115 in SkCanvas::internalDrawDevice(SkBaseDevice*, int, int, SkPaint const*, SkImage*, SkMatrix const&) third_party/skia/src/core/SkCanvas.cpp:1313:25
    #14 0xa0c050 in SkCanvas::internalRestore() third_party/skia/src/core/SkCanvas.cpp:1201:19
    #15 0xa1184c in AutoDrawLooper::~AutoDrawLooper() third_party/skia/src/core/SkCanvas.cpp:495:22
    #16 0xa1f12e in SkCanvas::onDrawBitmap(SkBitmap const&, float, float, SkPaint const*) third_party/skia/src/core/SkCanvas.cpp:2308:1
    #17 0xa189a8 in SkCanvas::drawBitmap(SkBitmap const&, float, float, SkPaint const*) third_party/skia/src/core/SkCanvas.cpp:1831:11
    #18 0x75c40f in (anonymous namespace)::RunTestCase(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, SkBitmap&, SkCanvas*) skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:48:13
    #19 0x75afcd in (anonymous namespace)::ReadAndRunTestCase(char const*, SkBitmap&, SkCanvas*) skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:67:3
    #20 0x75aac6 in main skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:87:10
    #21 0x7fd9b5a7df44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV third_party/skia/src/core/SkColorSpace.cpp:250:26 in SkColorSpace::gammaIsLinear() const
==185673==ABORTING



 
skia-null-read
280 bytes View Download
Project Member

Comment 1 by ClusterFuzz, Nov 22 2017

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4918000301113344.
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam Type-Bug
NULL dereference crashes are not considered as security issues: https://www.chromium.org/Home/chromium-security/reporting-security-bugs#TOC-Signs-A-Crash-Is-Not-A-Security-Bug


I think the job type in the clusterfuzz analysis was wrong, I'm running a new one with the correct job type.
Project Member

Comment 4 by ClusterFuzz, Nov 22 2017

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5267491722100736.
Project Member

Comment 5 by ClusterFuzz, Nov 23 2017

Labels: Security_Impact-Beta
Detailed report: https://clusterfuzz.com/testcase?key=5267491722100736

Job Type: linux_asan_filter_fuzz_stub
Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  SkColorSpace::gammaIsLinear
  SkToSRGBColorFilter::onAppendStages
  SkRasterPipelineBlitter::Create
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_filter_fuzz_stub&range=502976:503015

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5267491722100736

See https://github.com/google/clusterfuzz-tools for more information.
Project Member

Comment 6 by ClusterFuzz, Nov 23 2017

Labels: Test-Predator-Auto-Owner
Owner: mtklein@chromium.org
Status: Assigned (was: Unconfirmed)
Automatically assigning owner based on suspected regression changelist https://skia.googlesource.com/skia/+/6c4ea7e8801c5b2b758c67d01e236da29292799b (Fix 3 related races in SkColorSpace.cpp.).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Cc: brianosman@chromium.org
Thanks!  I'm a good owner.  Looks like the bug is in SkToSRGBColorFilter.
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 27 2017

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

commit c9bc81434adc0c0ea6012167fbda0e131548e683
Author: Mike Klein <mtklein@chromium.org>
Date: Mon Nov 27 18:00:56 2017

Handle null colorspace in SkToSRGBColorFilter.

This was uncovered by the linked fuzzer issue.

I haven't looked hard at it, but I'd guess it's fuzzed an ICC profile
into one that can't be deserialized, and we get a null in CreateProc().

We could probably restrict the null check to just CreateProc(), but
putting it in Make() and asserting in the constructor feels cozy.

BUG= chromium:787718 

Change-Id: Ic4b1dad28c00ee5870f22093eedbf34686c32120
Reviewed-on: https://skia-review.googlesource.com/76080
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>

[add] https://crrev.com/c9bc81434adc0c0ea6012167fbda0e131548e683/tests/ToSRGBColorFilter.cpp
[modify] https://crrev.com/c9bc81434adc0c0ea6012167fbda0e131548e683/src/effects/SkToSRGBColorFilter.cpp
[modify] https://crrev.com/c9bc81434adc0c0ea6012167fbda0e131548e683/gn/tests.gni

Project Member

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

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

commit 0e25e78ebabf7037ca8917af02521bdae5360e84
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Tue Nov 28 02:43:28 2017

Roll src/third_party/skia/ dfaa057c1..c9c97b7fd (20 commits)

https://skia.googlesource.com/skia.git/+log/dfaa057c1518..c9c97b7fd5f9

$ git log dfaa057c1..c9c97b7fd --date=short --no-merges --format='%ad %ae %s'
2017-11-27 csmartdalton CCPR: Transform path points before parsing
2017-11-27 mtklein remove SkDebugCanvas clip query overrides
2017-11-27 csmartdalton Cleanup yes/no enums in Ganesh
2017-11-27 mtklein update SkJumper docs to Clang 5
2017-11-27 bsalomon Batch ops together in SkAtasTextTarget
2017-11-27 mtklein clean up SkDeferredCanvas
2017-11-27 csmartdalton Include opList ids in GrOnFlushCallbackObject::postFlush
2017-11-27 egdaniel Fix caps for advanced blend extension with regards to layout qualifiers
2017-11-27 robertphillips Revert "Make sure to visit clips and dst proxies during gather"
2017-11-27 mtklein roll GN
2017-11-21 csmartdalton Make sure to visit clips and dst proxies during gather
2017-11-27 ethannicholas added SkSL support for all blend mode layouts
2017-11-27 angle-deps-roller Roll skia/third_party/externals/angle2/ 994ee3f23..ebee5b3b7 (2 commits)
2017-11-27 mtklein Handle null colorspace in SkToSRGBColorFilter.
2017-11-27 liyuqian Add forceDAA parameter to AntiFillPath
2017-11-17 robertphillips Add Fiddle DrawOptions for the backend objects
2017-11-20 csmartdalton Delete an opList's clips when it survives a flush
2017-11-27 bsalomon Make EGL test context access GL via GrGLInterface
2017-11-27 caryclark bookmaker refresh
2017-11-27 reed saturate when converting float to fixed

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


The AutoRoll server is located here: https://autoroll.skia.org

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


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
TBR=allanmac@chromium.org

Change-Id: Ibfc18a318dfc01efeb3baefcd41127fc139fbbe6
Reviewed-on: https://chromium-review.googlesource.com/791996
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@{#519534}
[modify] https://crrev.com/0e25e78ebabf7037ca8917af02521bdae5360e84/DEPS

Project Member

Comment 10 by ClusterFuzz, Nov 28 2017

ClusterFuzz has detected this issue as fixed in range 519533:519535.

Detailed report: https://clusterfuzz.com/testcase?key=5267491722100736

Job Type: linux_asan_filter_fuzz_stub
Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  SkColorSpace::gammaIsLinear
  SkToSRGBColorFilter::onAppendStages
  SkRasterPipelineBlitter::Create
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_filter_fuzz_stub&range=502976:503015
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_filter_fuzz_stub&range=519533:519535

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5267491722100736

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, Nov 28 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5267491722100736 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Cc: kjlubick@chromium.org kjlubick@google.com

Sign in to add a comment