New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 469247 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2015
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
Nag



Sign in to add a comment

Use-of-uninitialized-value in blink::TransformationMatrix::blend

Project Member Reported by ClusterFuzz, Mar 20 2015

Issue description

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

Fuzzer: Dstockwell-anim-gen
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  blink::TransformationMatrix::blend
  blink::InterpolatedTransformOperation::apply
  blink::LayoutStyle::applyTransform
  

Minimized Testcase (2.90 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97iVFwYeoWWDryokzHWUABT62CdLIZWnuPIEQSLIK9r0WyFavYUxFl4cgBFG3b_arShPHZZGTh5z5HhjVJubgCaUJrqYG83uhw32oE163L4MTtpURyrHtLgaVUp3oj5KljFil4cXyTEU6wgRgVjp8PMRRw9gg

Filer: inferno
 
Cc: dstockwell@chromium.org
Owner: alancutter@chromium.org
Status: Assigned
Project Member

Comment 2 by ClusterFuzz, Mar 20 2015

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

Fuzzer: Dstockwell-anim-gen
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  blink::TransformationMatrix::blend
  blink::Matrix3DTransformOperation::blend
  blink::TransformOperations::blendByMatchingOperations
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=319619:319765

Minimized Testcase (97.06 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97LvkeK21EgqRkRzGXyS_PRA7jST7fW30dGOpPi3WKbOiOnZ58nZqsvEVpw69_F9s5U8yrFBiAJXY86uMXUe6busZ3CW4Dgg849F5npy80sEpXalXFDEd4xoKKOk3glkuRhQWIqyqJmYYr-qfN08z7Fud7tu9pPWD7WbFOODxNPfc_NiXs

Filer: inferno
Cc: zac...@126.com
Author: kui.zheng@arm.com 
Component: blink
Changelist: https://chromium.googlesource.com/chromium/blink.git/+/7b03a53c2c7f24fb1dd79c0e7f4b7fad520875a5
Time: Mon Mar 09 17:06:11 2015
File TransformationMatrix.cpp is changed in this cl (and is part of stack frame #0, "blendFloat"; frame #1, "blink::TransformationMatrix::blend")
Minimum distance from crash line to modified line: 1181. (file: TransformationMatrix.cpp, crashed on: 1525, modified: 344).
Labels: M-43
Setting M-43 for now since I can't tell which value is uninitialized or if it could be read back, but feel free to set M-42 if this looks like a more serious issue.
Project Member

Comment 5 by ClusterFuzz, Mar 20 2015

Labels: Pri-1

Comment 6 by zac...@126.com, Mar 21 2015

I'm failed to access cluster-fuzz, What platform is it?
The Change(https://chromium.googlesource.com/chromium/blink.git/+/7b03a53c2c7f24fb1dd79c0e7f4b7fad520875a5) is only for ARM64.
Adding the repro. this is on x86.
fuzz-17.html
2.9 KB View Download
Cc: jbroman@chromium.org
The stack trace suggests that fromDecomp.perspectiveX in TransformationMatrix::blend() is initialised.
Haven't yet been able to repro the failure so far using both the minified and full test cases.
MSAN crashes immediately on launch for me on Ubuntu 14.04.
Manually RELEASE_ASSERTing that all the DecomposedTypes have been set after calling decompose with them doesn't find anything.
This looks like a different bug than the one I recently fixed. But I can speculate.

My best guess as to the problem would be that inverse(perspectiveMatrix, inversePerspectiveMatrix) is returning false.

The invertibility check that decompose does is

  if (determinant4x4(perspectiveMatrix) == 0)

but the check that inverse does is (with some simplification):

  if (fabs(determinant4x4(matrix)) < SMALL_NUMBER)

So maybe if the determinant is between 0 and 1e-8, it will use uninitialized values from inversePerspectiveMatrix. If that's so, TransformationMatrix::blend should probably check the result of inverse. (And maybe inverse should be WARN_UNUSED_RETURN.)
Cc: earthdok@chromium.org
Can you try rerunning with --use-gl=osmesa. I think regular gl has some startup crash with MSAN.
Project Member

Comment 11 by ClusterFuzz, Apr 15 2015

Labels: Nag
alancutter@: Uh oh! This issue is still open and hasn't been updated in the last 21 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 12 by ClusterFuzz, May 6 2015

alancutter@: Uh oh! This issue is still open and hasn't been updated in the last 42 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: timwillis@chromium.org
alancutter / earthdok - just want to check that this hasn't fallen off your todo lists. Any luck with reproducing with tip in #10?
Labels: findit-wrong
Cc: mikelawther@chromium.org
Cc: mbarbe...@chromium.org
Is anyone actively looking at this?
Sorry for the delay on this one.
Still haven't been able to run MSAN locally without hitting unrelated UOUVs:

Uninitialized bytes in __interceptor_getsockname at offset 0 inside [0x7ffedf3d7f3c, 4)
==5646== WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7f73110ab30e  (/usr/lib/x86_64-linux-gnu/libxcb.so.1+0xd30e)
    #1 0x7f73110ab46e  (/usr/lib/x86_64-linux-gnu/libxcb.so.1+0xd46e)
    #2 0x7f73110aafe3  (/usr/lib/x86_64-linux-gnu/libxcb.so.1+0xcfe3)
    #3 0x7f7317d05329  (/usr/lib/x86_64-linux-gnu/libX11.so.6+0x3b329)
    #4 0x7f7317cf688e  (/usr/lib/x86_64-linux-gnu/libX11.so.6+0x2c88e)
    #5 0x7f7328f3b0d8  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libgfx_x11.so+0x60d8)
    #6 0x7f7328f3ae12  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libgfx_x11.so+0x5e12)
    #7 0x7f73242c409c  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libcontent.so+0x71609c)
    #8 0x7f73242c7e79  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libcontent.so+0x719e79)
    #9 0x7f7330be0450  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/content_shell+0xe5450)
    #10 0x7f7330bbb441  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/content_shell+0xc0441)
    #11 0x7f732406d3c5  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libcontent.so+0x4bf3c5)
    #12 0x7f732406fc61  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libcontent.so+0x4c1c61)
    #13 0x7f732406a654  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libcontent.so+0x4bc654)
    #14 0x7f7330bb8adc  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/content_shell+0xbdadc)
    #15 0x7f7315bb8ec4  (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #16 0x7f7330b65d27  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/content_shell+0x6ad27)

  Uninitialized value was created by an allocation of 'ref.tmp40' in the stack frame of function '_ZN7content15BrowserMainLoop19EarlyInitializationEv'
    #0 0x7f73242a9880  (/usr/local/google/home/alancutter/repos/chromium/src/out/Release/lib/libcontent.so+0x6fb880)

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/usr/lib/x86_64-linux-gnu/libxcb.so.1+0xd30e) 
Exiting


Created speculative fix based on the analysis in #9: https://codereview.chromium.org/1131973003
If this is the cause of the bug then this has been around since 2009 (M3):
https://chromium.googlesource.com/chromium/blink/+/29cde4f990dfcbe22a898a23c3f6e4c2def43711%5E%21/WebCore/platform/graphics/transforms/TransformationMatrix.cpp

Comment 18 by euge...@google.com, May 19 2015

Could you be running without instrumented libraries?
libxcb.so should be loaded from out/Release/instrumented_libraries_prebuilt/msan/lib/libxcb.so and not from /usr/lib.
See http://dev.chromium.org/developers/testing/memorysanitizer

Comment 19 by euge...@google.com, May 19 2015

I can confirm that the report goes away with your change (re:#17).

Project Member

Comment 20 by bugdroid1@chromium.org, May 21 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=195670

------------------------------------------------------------------
r195670 | alancutter@chromium.org | 2015-05-21T03:21:28.621155Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/animations/invalid-decompose-invert.html?r1=195670&r2=195669&pathrev=195670
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/transforms/TransformationMatrix.cpp?r1=195670&r2=195669&pathrev=195670

Fix use of uninitialized memory in TransformationMatrix decompose()

The static decompose() method in TransformationMatrix.cpp called
inverse() without checking the return value for success. If the matrix
fails to invert the function proceeds to use the uninitialized return
parameter as if it had succeeded.

This patch ensures decompose() fails if inverse() fails avoiding the
use of uninitialized memory.

BUG= 469247 

Review URL: https://codereview.chromium.org/1131973003
-----------------------------------------------------------------
Status: Fixed
Project Member

Comment 22 by ClusterFuzz, May 21 2015

Labels: -Restrict-View-SecurityTeam Merge-Triage Restrict-View-SecurityNotify M-44
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
Project Member

Comment 23 by ClusterFuzz, May 23 2015

ClusterFuzz has detected this issue as fixed in range 330970:331027.

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

Fuzzer: Dstockwell-anim-gen
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  blink::TransformationMatrix::blend
  blink::Matrix3DTransformOperation::blend
  blink::TransformOperations::blendByMatchingOperations
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=319619:319765
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=330970:331027

Minimized Testcase (0.37 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95_lU6NmUwCBQauGhMHeS9UUTrGnzGBA0vziVfJ62j2fBWeIdXEi9TxN_OVuWHtsypa3j5fxdKVdFZcwrayauWz6GWZteQciHB8w5v5oxF1GHqr0YvRotXvDFgwdWxHyJzoe2hqd7CeomcLjLJwQZ-4OqAmIw

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

Comment 24 by ClusterFuzz, May 23 2015

ClusterFuzz has detected this issue as fixed in range 330970:331027.

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

Fuzzer: Dstockwell-anim-gen
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  blink::TransformationMatrix::blend
  blink::Matrix3DTransformOperation::blend
  blink::TransformOperations::blendByMatchingOperations
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=319619:319765
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=330970:331027

Minimized Testcase (0.37 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95_lU6NmUwCBQauGhMHeS9UUTrGnzGBA0vziVfJ62j2fBWeIdXEi9TxN_OVuWHtsypa3j5fxdKVdFZcwrayauWz6GWZteQciHB8w5v5oxF1GHqr0YvRotXvDFgwdWxHyJzoe2hqd7CeomcLjLJwQZ-4OqAmIw

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

Comment 25 by ClusterFuzz, May 23 2015

ClusterFuzz has detected this issue as fixed in range 330970:331027.

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

Fuzzer: Dstockwell-anim-gen
Job Type: Linux_msan_chrome

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  blink::TransformationMatrix::blend
  blink::InterpolatedTransformOperation::apply
  blink::LayoutStyle::applyTransform
  
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_msan_chrome&range=330970:331027

Minimized Testcase (2.90 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97iVFwYeoWWDryokzHWUABT62CdLIZWnuPIEQSLIK9r0WyFavYUxFl4cgBFG3b_arShPHZZGTh5z5HhjVJubgCaUJrqYG83uhw32oE163L4MTtpURyrHtLgaVUp3oj5KljFil4cXyTEU6wgRgVjp8PMRRw9gg

If you suspect that the result above is incorrect,try re-doing that job on the test case report page.
Cc: st...@chromium.org

Comment 27 by st...@chromium.org, May 26 2015

Cc: kateso...@chromium.org
 Issue 507019  has been merged into this issue.
Labels: -Merge-Triage Merge-Request-44
Merge-Requested for M44 (branch 2403)
Labels: -Merge-Request-44 Merge-Review-44 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M44, manual review required.
Labels: -Merge-Review-44 -Hotlist-Merge-Review Merge-Approved-44
Merge approved for m44 branch 2403.  Please get merge done by end of business PST Monday.
FYI: You only have ~4 hours to get this merge finished, or it'll miss stable.
And now you have 1 hour.
Sorry, saw the updates on this one too late. ):
Labels: -M-43 -M-44 -Merge-Approved-44 M-45 Merge-Request-45
Since this bug has been around since 2009 (M3), I think we're better off not merging to M44 at this late stage, and merging to M45 instead.

Labels: -Merge-Request-45 Merge-Review-45 Hotlist-Merge-Review
[Automated comment] Commit may have occurred before M45 branch point (7/10/2015), needs manual review.

Comment 37 by amin...@google.com, Jul 24 2015

Labels: -Merge-Review-45
M45 branched July 10, so I don't think a merge is required here.  Ping me if you feel otherwise.
Project Member

Comment 38 by ClusterFuzz, Aug 27 2015

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: Release-0-M45
Labels: -Hotlist-Merge-Review -ClusterFuzz Clusterfuzz
Project Member

Comment 41 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 42 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