New issue
Advanced search Search tips

Issue 794406 link

Starred by 0 users

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

Security: Use of Uninitialized Value in approx_log2 (msan build filter_fuzz_stub)

Project Member Reported by metzman@chromium.org, Dec 13 2017

Issue description

This was found by skia_image_filter_proto_fuzzer (filter_proto_fuzzer). 

The bug is reproducible using a MSAN build from HEAD of filter_fuzz_stub

REPRODUCTION CASE

1. Build filter_fuzz_stub using the following options:
is_msan = true
is_debug = false

2. Run it on the attached input (ffs-approx_log2):

$ ./out/ffs/filter_fuzz_stub ffs-approx_log2 
[1212/184144.487348:INFO:filter_fuzz_stub.cc(61)] Test case: ffs-approx_log2
[1212/184144.515041:INFO:filter_fuzz_stub.cc(38)] Valid stream detected.
==18892==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0xb8b679 in approx_log2 third_party/skia/src/jumper/SkJumper_vectors.h:643:27
    #1 0xb8b679 in approx_powf third_party/skia/src/jumper/SkJumper_vectors.h:654
    #2 0xb8b679 in parametric third_party/skia/src/jumper/SkJumper_stages.cpp:825
    #3 0xb8b679 in parametric_r_k third_party/skia/src/jumper/SkJumper_stages.cpp:828
    #4 0xb8b679 in sk_parametric_r third_party/skia/src/jumper/SkJumper_stages.cpp:828
    #5 0xb78229 in sk_start_pipeline third_party/skia/src/jumper/SkJumper_stages.cpp:79:13
    #6 0xb774cc in SkRasterPipeline::run(unsigned long, unsigned long, unsigned long, unsigned long) const third_party/skia/src/jumper/SkJumper.cpp:471:5
    #7 0x8ed1ec in SkColorFilter::filterColor4f(SkColor4f const&) const third_party/skia/src/core/SkColorFilter.cpp:77:14
    #8 0x8ecc3a in SkColorFilter::filterColor(unsigned int) const third_party/skia/src/core/SkColorFilter.cpp:54:26
    #9 0x11201d5 in affectsTransparentBlack third_party/skia/include/core/SkColorFilter.h:140:22
    #10 0x11201d5 in SkColorFilterImageFilter::affectsTransparentBlack() const third_party/skia/src/effects/SkColorFilterImageFilter.cpp:146
    #11 0x9644a7 in SkImageFilter::canComputeFastBounds() const third_party/skia/src/core/SkImageFilter.cpp:281:15
    #12 0x9e835a in SkPaint::canComputeFastBounds() const third_party/skia/src/core/SkPaint.cpp:1991:60
    #13 0x8d63fb in SkCanvas::onDrawBitmap(SkBitmap const&, float, float, SkPaint const*) third_party/skia/src/core/SkCanvas.cpp:2273:33
    #14 0x8c658a in SkCanvas::drawBitmap(SkBitmap const&, float, float, SkPaint const*) third_party/skia/src/core/SkCanvas.cpp:1831:11
    #15 0x49369d in RunTestCase skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:48:13
    #16 0x49369d in ReadAndRunTestCase skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:67
    #17 0x49369d in main skia/tools/filter_fuzz_stub/filter_fuzz_stub.cc:87
    #18 0x7f4de3809f44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287
    #19 0x423829 in _start (/usr/local/google/home/metzman/chromium1/src/out/ffs/filter_fuzz_stub+0x423829)

  Uninitialized value was created by an allocation of 'program' in the stack frame of function '_ZNK16SkRasterPipeline3runEmmmm'
    #0 0xb77170 in SkRasterPipeline::run(unsigned long, unsigned long, unsigned long, unsigned long) const third_party/skia/src/jumper/SkJumper.cpp:462

SUMMARY: MemorySanitizer: use-of-uninitialized-value third_party/skia/src/jumper/SkJumper_vectors.h:643:27 in approx_log2
Exiting
 
ffs-approx_log2
260 bytes View Download
Project Member

Comment 1 by ClusterFuzz, Dec 13 2017

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

Comment 2 by ClusterFuzz, Dec 13 2017

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

Job Type: linux_msan_filter_fuzz_stub
Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  sk_parametric_r
  sk_start_pipeline
  SkRasterPipeline::run
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_filter_fuzz_stub&range=489366:489404

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

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 3 by sheriffbot@chromium.org, Dec 13 2017

Labels: M-63
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 13 2017

Labels: Pri-1

Comment 5 by cthomp@chromium.org, Dec 14 2017

Cc: herb@google.com
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
@herb: Could you help find an owner for this? Thanks.

Comment 6 by herb@google.com, Dec 14 2017

Cc: mtkl...@google.com
Owner: mtklein@chromium.org

Comment 7 by cthomp@chromium.org, Dec 14 2017

Status: Assigned (was: Unconfirmed)
Looks like we're deserializing a 7-parameter transfer function wrong, and MSAN is deciding those parameters are first "used" when doing the divide in the expression

    return e
         - 124.225514990f
         -   1.498030302f * m
         -   1.725879990f / (0.3520887068f + m);

where both e and m are derived from pixel data (here, {0,0,0,0}) mixed with some of the uninitialized parameters.

Interesting that all the math up to that point is not "use", and if we'd used rcpps here it would also not be a "use".
Nooope.  The deserialization is fine.  I just made a subtle mistake implementing SKToSRGBColorFilter...
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 14 2017

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

commit dbd43481f1da3c9c87f5d913660f216ec531870a
Author: Mike Klein <mtklein@chromium.org>
Date: Thu Dec 14 21:00:57 2017

Don't pass a stack address as a pipeline context pointer.

We call isNumericalTransferFn() both to test if an SkColorSpace
is a 7-parameter numerical transfer function, and to get those
parameters if so.  They're passed to the stage functions that
apply that transfer function via a context pointer.

We can't use &srcFn as this pointer, as it's on the stack,
and won't be alive by the time we get around to running the
pipeline.  Instead, copy it to the SkArenaAlloc we thread
through just for this purpose.

This would be a beginner's mistake, except that I wrote
the API myself...

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

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

Project Member

Comment 11 by ClusterFuzz, Dec 15 2017

ClusterFuzz has detected this issue as fixed in range 524219:524244.

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

Job Type: linux_msan_filter_fuzz_stub
Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  sk_parametric_r
  sk_start_pipeline
  SkRasterPipeline::run
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_filter_fuzz_stub&range=489366:489404
Fixed: https://clusterfuzz.com/revisions?job=linux_msan_filter_fuzz_stub&range=524219:524244

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

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 12 by ClusterFuzz, Dec 15 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 6033872835051520 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 13 by sheriffbot@chromium.org, Dec 15 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
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 24 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