New issue
Advanced search Search tips
Starred by 3 users
Status: Fixed
Owner:
Closed: May 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux, Windows, Chrome, Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: Use-of-uninitialized-value on stack
Reported by look.wan...@gmail.com, May 25 Back to list
VERSION
Operating System: Ubuntu 16.04.2 LTS 


VULNERABILITY DETAILS

1) build latest code of filter_fuzz_stub with following gn flags:
is_msan = true
is_debug = false
(ninja -C buildir skia:filter_fuzz_stub)

2) Run filter_fuzz_stub with attached file:

./filter_fuzz_stub  crash1

[0525/105956.610359:INFO:filter_fuzz_stub.cc(60)] Test case: /tmp/crash1
==19188==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1076321  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x1076321)
    #1 0x10789b4  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x10789b4)
    #2 0x945337  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x945337)
    #3 0x77fa95  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x77fa95)
    #4 0xf5baab  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0xf5baab)
    #5 0x945337  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x945337)
......
Uninitialized value was created by an allocation of 'src' in the stack frame of function '_ZN17SkTileImageFilter10CreateProcER12SkReadBuffer'




./filter_fuzz_stub  crash2

[0525/110054.946202:INFO:filter_fuzz_stub.cc(60)] Test case: /tmp/crash2
==20304==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x101fdfb  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x101fdfb)
    #1 0x945337  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x945337)
    #2 0x77fa95  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x77fa95)
    #3 0xf5baab  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0xf5baab)
    #4 0x945337  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x945337)
    #5 0x77fa95  (/home/test/work/chromium/src/skia/out/filter_fuzz_stub+0x77fa95)
......
 Uninitialized value was created by an allocation of 'src' in the stack frame of function '_ZN22SkMagnifierImageFilter10CreateProcER12SkReadBuffer'



3)
https://cs.chromium.org/chromium/src/third_party/skia/src/effects/SkTileImageFilter.cpp?l=148

https://cs.chromium.org/chromium/src/third_party/skia/src/effects/SkMagnifierImageFilter.cpp?l=314


I think same problem is here in function "CreateProc": "src" is not initialized. 

 
crash1
1.0 KB View Download
crash2
1.0 KB View Download
Components: Internals>Skia
Cc: robertphillips@chromium.org
Labels: OS-Linux
Owner: reed@chromium.org
Status: Assigned
reed@: Can you please help triage this?
Owner: hcm@chromium.org
If our tree ever opens, I have a fix in https://skia-review.googlesource.com/c/17984/ (Initialize rects in SkValidatingReadBuffer readRect & readIRect on failure)
Cc: -robertphillips@chromium.org hcm@chromium.org
Labels: Security_Severity-Medium OS-Chrome OS-Mac OS-Windows Pri-1
Owner: robertphillips@chromium.org
Thanks! Do you know if this would affect Stable, or might it have regressed at some point?
We will definitely want to cherry pick this back to M60. It is a very safe fix though so we can cherry pick it as far back as everyone is comfortable with. 

The uninitialized memory reference will definitely exist in all extant Chrome versions.
Project Member Comment 7 by bugdroid1@chromium.org, May 26
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/10240e385f541f9a6dea30469bda84c92799fbd9

commit 10240e385f541f9a6dea30469bda84c92799fbd9
Author: Robert Phillips <robertphillips@google.com>
Date: Fri May 26 18:24:43 2017

Initialize rects in SkValidatingReadBuffer readRect & readIRect on failure

BUG:  726199 

Change-Id: I2afeebbd2240e1c2f3b7d7298cb11572eef18acd
Reviewed-on: https://skia-review.googlesource.com/17984
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Mike Reed <reed@google.com>

[modify] https://crrev.com/10240e385f541f9a6dea30469bda84c92799fbd9/src/core/SkValidatingReadBuffer.cpp

Project Member Comment 8 by bugdroid1@chromium.org, May 26
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/10240e385f541f9a6dea30469bda84c92799fbd9

commit 10240e385f541f9a6dea30469bda84c92799fbd9
Author: Robert Phillips <robertphillips@google.com>
Date: Fri May 26 18:24:43 2017

Initialize rects in SkValidatingReadBuffer readRect & readIRect on failure

BUG:  726199 

Change-Id: I2afeebbd2240e1c2f3b7d7298cb11572eef18acd
Reviewed-on: https://skia-review.googlesource.com/17984
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Mike Reed <reed@google.com>

[modify] https://crrev.com/10240e385f541f9a6dea30469bda84c92799fbd9/src/core/SkValidatingReadBuffer.cpp

Labels: reward-topanel M-60 Security_Impact-Stable
Thanks for the fast fix.
Labels: Merge-Request-60
The fix rolled into Chrome on 5/26 at 475219 in https://chromium-review.googlesource.com/c/517374/.

It did not make it into the M60 branch so we will need to cherry-pick it.
Project Member Comment 11 by sheriffbot@chromium.org, May 30
Status: Fixed
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 12 by sheriffbot@chromium.org, May 30
Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 13 by sheriffbot@chromium.org, May 31
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member Comment 14 by bugdroid1@chromium.org, May 31
Labels: merge-merged-m60
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/334781463a8d9221a7b7adf1137bedc9d7c8d4cb

commit 334781463a8d9221a7b7adf1137bedc9d7c8d4cb
Author: Robert Phillips <robertphillips@google.com>
Date: Wed May 31 14:02:57 2017

[M60 cherry pick] Initialize rects in SkValidatingReadBuffer readRect & readIRect on failure

TBR=reed@google.com

No-Tree-Checks: true
No-Try: true
No-Presubmit: true
Bug:  726199 
Change-Id: Ibaf5391d01565a522d3230cd6ac066cc522ad739
Reviewed-on: https://skia-review.googlesource.com/18226
Reviewed-by: Robert Phillips <robertphillips@google.com>

[modify] https://crrev.com/334781463a8d9221a7b7adf1137bedc9d7c8d4cb/src/core/SkValidatingReadBuffer.cpp

Labels: Merge-Rejected-59
Merged to M60 but this will also be present in M59.
Labels: -Merge-Rejected-59 Merge-Request-59
Project Member Comment 17 by sheriffbot@chromium.org, May 31
Labels: -Merge-Request-59 Merge-Review-59 Hotlist-Merge-Review
This bug requires manual review: We are only 5 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: awhalley@chromium.org
+ awhalley@ for security review
Labels: -Merge-Review-59 Merge-Rejected-59
No need to get this into M59.
Project Member Comment 20 by sheriffbot@chromium.org, Jun 5
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-60
Labels: -reward-topanel reward-unpaid reward-500
The panel decided to award $500 for this report - thanks!
Labels: -reward-unpaid reward-inprocess
Labels: Release-0-M60
Labels: CVE-2017-5103
Project Member Comment 27 by sheriffbot@chromium.org, Sep 6
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