New issue
Advanced search Search tips

Issue 789767 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

MSAN detects use-of-uninitialized-value in analyze_3x4_matrix() in filter_fuzz_stub

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

Issue description

Another bug found by my filter_proto_fuzzer which I have just put up for review (any feedback on it from skia folks would be greatly appreciated https://chromium-review.googlesource.com/c/chromium/src/+/792950)


VULNERABILITY DETAILS
An Image Filter containing an SkToSRGBColorFilter can cause uninitialized memory to be accessed. I'm not too familiar with skia internals, but I've minimized the testcase by hand and it seems like the kSRGBLinear_Named value inside of a ColorSpace in the ColorFilter is necessary for the crash to occur. The crash occurs in filter_fuzz_stub on trunk when built with MSAN, it will not occur with ASAN.  


REPRODUCTION CASE
1. Build filter_fuzz_stub using the following options:
enable_nacl = false
ffmpeg_branding = "ChromeOS"
is_msan = true
pdf_enable_xfa = true
proprietary_codecs = true
use_libfuzzer = true
use_goma = true
is_debug = false
optimize_for_fuzzing = true


2. Run it on the attached input (skia-msan-crash-str):

$ ./out/skia-msan/filter_fuzz_stub skia-msan-crash-str                                                                                                                                                                                                                                         
[1129/160708.865588:INFO:filter_fuzz_stub.cc(61)] Test case: skia-msan-crash-str
[1129/160708.890186:INFO:filter_fuzz_stub.cc(38)] Valid stream detected.
==27718==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x12c9b75 in analyze_3x4_matrix(float const*, bool*, bool*) third_party/skia/src/core/SkPM4fPriv.h:92:39
    #1 0x12c87f4 in SkToSRGBColorFilter::onAppendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*, bool) const third_party/skia/src/effects/SkToSRGBColorFilter.cpp:49:5
    #2 0x8fc8a1 in SkColorFilter::filterColor4f(SkColor4f const&) const third_party/skia/src/core/SkColorFilter.cpp:74:11
    #3 0x8fc2f7 in SkColorFilter::filterColor(unsigned int) const third_party/skia/src/core/SkColorFilter.cpp:54:26
    #4 0x9a2424 in SkColorFilter::affectsTransparentBlack() const third_party/skia/include/core/SkColorFilter.h:140:22
    #5 0x9a0139 in SkImageFilter::canComputeFastBounds() const third_party/skia/src/core/SkImageFilter.cpp:281:15
    #6 0xa3a6a1 in SkPaint::canComputeFastBounds() const third_party/skia/src/core/SkPaint.cpp:1991:60
    #7 0x8e537a in SkCanvas::onDrawBitmap(SkBitmap const&, float, float, SkPaint const*) third_party/skia/src/core/SkCanvas.cpp:2273:33
    #8 0x8d9939 in SkCanvas::drawBitmap(SkBitmap const&, float, float, SkPaint const*) third_party/skia/src/core/SkCanvas.cpp:1831:11
    #9 0x49611f 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
    #10 0x493c6c in (anonymous namespace)::ReadAndRunTestCase(char const*, SkBitmap&, SkCanvas*) skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:67:3
    #11 0x4934d6 in main skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:87:10
    #12 0x7fe544dd0f44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287
    #13 0x4240b9 in _start (/usr/local/google/home/metzman/chromium0/src/out/skia-msan/filter_fuzz_stub+0x4240b9)

  Uninitialized value was created by an allocation of 'alloc' in the stack frame of function '_ZNK13SkColorFilter13filterColor4fERK9SkColor4f'
    #0 0x8fc530 in SkColorFilter::filterColor4f(SkColor4f const&) const third_party/skia/src/core/SkColorFilter.cpp:67

SUMMARY: MemorySanitizer: use-of-uninitialized-value third_party/skia/src/core/SkPM4fPriv.h:92:39 in analyze_3x4_matrix(float const*, bool*, bool*)
Exiting
 
skia-msan-crash-str
184 bytes View Download
Project Member

Comment 1 by ClusterFuzz, Nov 30 2017

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5727333972705280.
Description: Show this description
Project Member

Comment 3 by ClusterFuzz, Nov 30 2017

Labels: Security_Impact-Stable Security_Severity-Medium
Detailed report: https://clusterfuzz.com/testcase?key=5727333972705280

Job Type: linux_msan_filter_fuzz_stub
Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  SkToSRGBColorFilter::onAppendStages
  SkColorFilter::filterColor4f
  SkColorFilter::filterColor
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_filter_fuzz_stub&range=489592:489609

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

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

A recommended severity was added to this bug. Please change the severity if it is inaccurate.

Project Member

Comment 4 by ClusterFuzz, Nov 30 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/+/159db0a6a1001b220b42fcae46ed324e0986b14a (rough clamped tracking in SkRasterPipeline).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 30 2017

Labels: M-63
Project Member

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

Labels: Pri-1
This is interesting and unexpected.  I'll be sure to check it out.

I'm not sold yet on what you're doing over in https://chromium-review.googlesource.com/c/chromium/src/+/792950.  Can you get me up to speed on that?  Maybe a VC?
Cc: brianosman@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 4 2017

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

commit 364c4c87c1321625ae961d8f2cf315096ccfd462
Author: Mike Klein <mtklein@chromium.org>
Date: Mon Dec 04 19:29:49 2017

Only look at gamut_transform if we wrote to it.

If we're already in sRGB gamut, we will not write to the 3x4 matrix,
but we still analyze it to see if we need clamping.  So we might
(harmlessly) re-clamp some already clamped colors unnecessarily.

Found by this Chromium MSAN bot.
Bug:  chromium:789767 
Change-Id: I5d76e59b541a03ee8efbd4352262b4f650e1ec01
Reviewed-on: https://skia-review.googlesource.com/79762
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

[modify] https://crrev.com/364c4c87c1321625ae961d8f2cf315096ccfd462/src/effects/SkToSRGBColorFilter.cpp

Project Member

Comment 10 by ClusterFuzz, Dec 5 2017

ClusterFuzz has detected this issue as fixed in range 521495:521546.

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

Job Type: linux_msan_filter_fuzz_stub
Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  SkToSRGBColorFilter::onAppendStages
  SkColorFilter::filterColor4f
  SkColorFilter::filterColor
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_filter_fuzz_stub&range=489592:489609
Fixed: https://clusterfuzz.com/revisions?job=linux_msan_filter_fuzz_stub&range=521495:521546

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

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, Dec 5 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

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

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Cc: kjlubick@chromium.org kjlubick@google.com
Labels: -M-63 M-65
Labels: Release-0-M65
Project Member

Comment 17 by sheriffbot@chromium.org, Mar 13 2018

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