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 0 users
Status: Fixed
Owner:
Closed: Sep 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
Use-of-uninitialized-value in SkFlatDictionary<SkPaint, SkPaint::FlatteningTraits>::findAndReturnMutableF
Project Member Reported by ClusterFuzz, Jul 2 2014 Back to list
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4806825801154560

Fuzzer: Miaubiz_svg_fuzzer
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  - crash stack -
  SkFlatDictionary<SkPaint, SkPaint::FlatteningTraits>::findAndReturnMutableF
  SkPictureRecord::drawPath
  SkBBoxRecord::drawPath
  

Minimized Testcase (0.87 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95gw1QQIQOOC7pFGMAbg4ew93ka-rsWj7eJkbhS9jv87dTnPA7AlXfXcDjz72ZNW9BVej6Y0CeX7X_xRfSUqce9KOulmLuaOSCN0FleHHl6bSE5mHSYnATGOV2P3kKSNRJX16Qh-1F8p2WtzeEFKaBX_w2giw
Filer: aarya@google.com
 
Labels: Security_Impact-Stable Security_Impact-Beta
Cc: reed@chromium.org senorblanco@chromium.org
Owner: sugoi@chromium.org
Status: Assigned
Sugoi@, we need someone to spearhead these MSAN bugs. Can someone from your team look into these. They should be easy to fix as it just needs intializing the vars.
Project Member Comment 3 by ClusterFuzz, Jul 2 2014
Labels: Pri-1
Comment 4 by sugoi@chromium.org, Jul 8 2014
Cc: sugoi@chromium.org
Owner: mtklein@chromium.org
Delegating to someone who knows about SkFlatDictionary.
Looks like the same as https://code.google.com/p/chromium/issues/detail?id=381156 ?

We haven't changed that code since then, so I guess my analysis and questions from over there still stand: I can't see how it's possible to read fArray without having first allocated fCapacity zero pointers with calloc.  Does MSAN understand calloced memory is zeroed?

Is there a way to set up suppressions for MSAN?  I'd set up a MSAN bot for Skia, but we call into a lot of system code, particularly font and GPU drivers.
Cc: euge...@chromium.org kcc@chromium.org
It is actually the *index* that is poisoned. It is derived from a hash of a SkDashPathEffect object, in which fPhase and fInitialDashIndex may be uninitialized, if this branch is taken during construction:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/src/utils/SkDashPath.cpp&rcl=1404816569&l=75

The issue is benign, but we should still initialize those members properly.

> Is there a way to set up suppressions for MSAN?  I'd set up a MSAN bot for Skia, but we call into a lot of system code, particularly font and GPU drivers.

We don't support suppressions. In Chromium one can set a flag to rebuild all DSO dependencies with MSan instrumentation (about 50 packages). This eliminates most false positives coming from system code. If you're interested, we could think about making this setup reusable across projects. For Skia you'll likely need only a subset of those packages, so ideally each project should be able to choose which ones it needs.
Oh interesting.  That makes a lot more sense.  Will dig into that SkDashPath code and make sure everything's initialized.


It's going to be tricky not supporting suppressions.  We deal pretty intimately with GPU drivers, which are pretty much all closed binary blobs.  E.g.:

UMR in __interceptor_strlen at offset 0 inside [0x60600000ee80, +37) 
==6670== WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7fa833eaa385 (/usr/lib/nvidia-331/libGL.so.1+0xc0385)

SUMMARY: MemorySanitizer: use-of-uninitialized-value ??:0 ??
Exiting

I guess we could restrict MSAN to testing the non-GPU parts of Skia on headless bots?  Even still, recompiling all dependencies is kind of a pain.  Is there any sort of --keep-going setting so we can run and filter away expected failures post-facto?
Comment 8 by euge...@google.com, Jul 9 2014
Well, there is a flag, but it's not well tested and completely unused. Try
-mllvm -msan-keep-going=1

> We deal pretty intimately with GPU drivers, which are pretty much all closed binary blobs.

In Chromium we run MSan builds with --use-gl=osmesa. I take it you don't have that option?
But I doubt it will help you run without rebuilding dependencies. There will be a huge number of reports, very bad performance and it will be hard to understand if a report is caused by an uninstrumented system library or not. For example, if a library call did not initialize some memory that's allocated in your code and used in your code, report will only mention your code.

Yeah, this does all sound tricky.  I think we'll just stick with Valgrind for now.
Project Member Comment 12 by ClusterFuzz, Jul 11 2014
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5989586578702336

Fuzzer: Inferno_twister
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  - crash stack -
  SkFlatDictionary<SkPaint, SkPaint::FlatteningTraits>::findAndReturnMutableF
  SkPictureRecord::drawPoints
  SkBBoxRecord::drawPoints
  

Minimized Testcase (0.09 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv97Zg5jKSQUSFdwjDjaj3QwZNDobW2VfdVGpAiOuHgxniCA00n8aQl3KsExguNGYi3PPSmj8JwVMA62K55U2Kcs_8UXg_xqngIZX6m5r2k7idR2E6fSEEswnKyOtgg9bzjTs6JkfjWQX9qxGPFpg9dyNjDMoMQ
<style>
* { background-position-y: -57%; outline: dashed 35px; outline-offset: 2147483552px;

Filer: earthdok@chromium.org
Project Member Comment 13 by ClusterFuzz, Jul 11 2014
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4999098132332544

Fuzzer: Inferno_twister
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  - crash stack -
  SkFlatDictionary<SkPaint, SkPaint::FlatteningTraits>::findAndReturnMutableF
  SkPictureRecord::drawRect
  SkBBoxRecord::drawRect
  

Minimized Testcase (0.50 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95ZqkyBWI5NXDUnouKYqEJHuecHlpE7-43tkEHTBL5bdFxZYlRFNzCAff8bt7k5JQK_zS2ExOIxxgolM8ddewyJwxWCgvr-5DM_pW0ArXWOO8RstbQe2hxtJe0L18wim0iuVqPxXWPvIdjUNhRTDDZGNXpUEQ
Filer: earthdok@chromium.org
Labels: M-37
Bulk edit of uninitialized value bugs without milestones to M-37.
Project Member Comment 15 by ClusterFuzz, Jul 11 2014
ClusterFuzz has detected this issue as fixed in range 282042:282480.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4806825801154560

Fuzzer: Miaubiz_svg_fuzzer
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  - crash stack -
  SkFlatDictionary<SkPaint, SkPaint::FlatteningTraits>::findAndReturnMutableF
  SkPictureRecord::drawPath
  SkBBoxRecord::drawPath
  
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=282042:282480

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95gw1QQIQOOC7pFGMAbg4ew93ka-rsWj7eJkbhS9jv87dTnPA7AlXfXcDjz72ZNW9BVej6Y0CeX7X_xRfSUqce9KOulmLuaOSCN0FleHHl6bSE5mHSYnATGOV2P3kKSNRJX16Qh-1F8p2WtzeEFKaBX_w2giw

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.

Project Member Comment 16 by ClusterFuzz, Jul 11 2014
ClusterFuzz has detected this issue as fixed in range 282042:282480.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5989586578702336

Fuzzer: Inferno_twister
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  - crash stack -
  SkFlatDictionary<SkPaint, SkPaint::FlatteningTraits>::findAndReturnMutableF
  SkPictureRecord::drawPoints
  SkBBoxRecord::drawPoints
  
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=282042:282480

Minimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv97Zg5jKSQUSFdwjDjaj3QwZNDobW2VfdVGpAiOuHgxniCA00n8aQl3KsExguNGYi3PPSmj8JwVMA62K55U2Kcs_8UXg_xqngIZX6m5r2k7idR2E6fSEEswnKyOtgg9bzjTs6JkfjWQX9qxGPFpg9dyNjDMoMQ

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.

Project Member Comment 17 by ClusterFuzz, Jul 20 2014
Labels: Nag
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Cc: bsalomon@chromium.org
Project Member Comment 19 by ClusterFuzz, Jul 28 2014
Labels: -Security_Impact-Beta
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 20 by ClusterFuzz, Jul 29 2014
Labels: Security_Impact-Beta
Project Member Comment 21 by ClusterFuzz, Jul 29 2014
Labels: -Security_Impact-Beta
Project Member Comment 22 by ClusterFuzz, Jul 29 2014
Labels: Security_Impact-Beta
Project Member Comment 23 by ClusterFuzz, Jul 29 2014
Labels: -Security_Impact-Beta
Project Member Comment 24 by ClusterFuzz, Jul 30 2014
Labels: -Security_Impact-Stable Security_Impact-Beta
Labels: -Security_Impact-Beta Security_Impact-Stable
Project Member Comment 26 by ClusterFuzz, Aug 9 2014
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 27 by ClusterFuzz, Aug 17 2014
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 28 by ClusterFuzz, Aug 24 2014
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Comment 29 by kenrb@chromium.org, Aug 26 2014
Cc: noel@chromium.org
Project Member Comment 30 by ClusterFuzz, Aug 31 2014
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 31 by ClusterFuzz, Sep 8 2014
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member Comment 32 by ClusterFuzz, Sep 15 2014
mtklein@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Are the fixed reports on this one correct? If so, please update the status to Fixed.
Status: Fixed
Yes, this code is both fixed and no longer running.
Project Member Comment 35 by ClusterFuzz, Sep 17 2014
Labels: -Restrict-View-SecurityTeam Merge-Triage M-38 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Labels: -Nag -Merge-Triage Merge-Requested
Matthew - Merge requested for M38 (Branch 2125)
FYI, the change to merge is https://skia.googlesource.com/skia/+/1c577cd3ee331944b9061ee0eec147b211ee563c which applies to third_party/skia

As mentioned above, this issue is pretty benign (it may rarely cause a relatively unimportant cache miss) so you might not want to bother.
Labels: -Merge-Requested
Per comment 37, punting to 39.  Removing Merge-Request label since 39 hasn't branched.
Labels: -M-37 -M-38 Release-0-M39 M-39
Targeting to M39 based on c#37
Cc: pdr@chromium.org
Cc: miau...@gmail.com
Labels: -reward-topanel reward-unpaid reward-500 CVE-2014-7909
Thanks again for the fuzzer contribution! This report qualified for a $500 reward.

Sorry for not ccing you on this one earlier.
Labels: -reward-unpaid reward-inprogress
Payment in progress.
Labels: -reward-inprogress reward-inprocess
Labels: -reward-inprocess
Processing via our e-payment system can take up to six weeks, but the reward should be on its way to you. Thanks again for your help!
Project Member Comment 45 by ClusterFuzz, Dec 24 2014
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 46 by sheriffbot@chromium.org, Oct 1 2016
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
Project Member Comment 47 by sheriffbot@chromium.org, Oct 2 2016
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
Labels: allpublic
Sign in to add a comment