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

Issue 468519 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Container-overflow in blink::FEColorMatrix::createImageFilter

Project Member Reported by ClusterFuzz, Mar 18 2015

Issue description

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

Fuzzer: Miaubiz_svg_fuzzer
Job Type: Linux_asan_chrome_mp

Crash Type: Container-overflow READ 4
Crash Address: 0x602000065750
Crash State:
  blink::FEColorMatrix::createImageFilter
  blink::FilterEffect::createImageFilterWithoutValidation
  blink::SkiaImageFilterBuilder::build
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=316793:316810

Minimized Testcase (5.82 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97pnz2ZhT8jbhH--6BisaAdiCglwJovyAYeifbmnKHi3mhI2BwRp1FssTPUBHEJh51UvtFGIL8nYoOgdJlY6GXIN1Zw81CdoKC_eEnegMabkGGSGcA-ArNA4Upme8O5uxDRAAOlfkYb6OeVjmi5EfYvYLTEog

Filer: ochang
 
Project Member

Comment 1 by ClusterFuzz, Mar 18 2015

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

Fuzzer: Miaubiz_svg_fuzzer
Job Type: Linux_asan_chrome_mp

Crash Type: Container-overflow READ {*}
Crash Address: 0x6080000314a0
Crash State:
  blink::FEColorMatrix::createImageFilter
  blink::FilterEffect::createImageFilterWithoutValidation
  blink::SkiaImageFilterBuilder::build
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=316591:316716

Minimized Testcase (4.54 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94BNSQc7v1fXcdoOa46P8ZACznWWSj64cyvu5EkKbDanazE4G3SLRPL_4oxiHrlPDwRd7Hfw7XxWIm40qk8BPr7RaJLV856hLlds7sBApNziIlWcHeNO1CNUXK9WdMAVqH5MFz97XBpnuUgXxQyPePVA0GOkA

Filer: ochang

Comment 2 by och...@chromium.org, Mar 18 2015

Cc: miau...@gmail.com senorblanco@chromium.org
Owner: schenney@chromium.org
It appears that the size of |values| isn't checked for createColorFilter in FEColorMatrix.cpp. After some initial investigation for one of these crashes (the first), it appears to be happening because an empty WTF::Vector being passed to FEColorMatrix::setValues, with createColorFilter being called later on on this |m_values|'s |.data()| afterwards. 

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/platform/graphics/filters/FEColorMatrix.cpp&sq=package:chromium&type=cs&l=128

Comment 3 by och...@chromium.org, Mar 18 2015

Cc: -ochang@google.com och...@chromium.org
Project Member

Comment 4 by ClusterFuzz, Mar 19 2015

Labels: Pri-1

Comment 6 by och...@chromium.org, Mar 19 2015

Here's the backtrace for what I believe is the offending setValues (which sets the zero length vector).

#0  shrink () at ../../third_party/WebKit/Source/wtf/Vector.h:991
#1  operator= () at ../../third_party/WebKit/Source/wtf/Vector.h:811
#2  0x000000000ef0d53b in setValues () at ../../third_party/WebKit/Source/platform/graphics/filters/FEColorMatrix.cpp:68
#3  0x000000000f562145 in setFilterEffectAttribute () at ../../third_party/WebKit/Source/core/svg/SVGFEColorMatrixElement.cpp:81
#4  0x00000000091eee18 in primitiveAttributeChanged () at ../../third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceFilter.cpp:148
#5  0x00000000092ebfce in primitiveAttributeChanged () at ../../third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceFilterPrimitive.h:53
#6  primitiveAttributeChanged () at ../../third_party/WebKit/Source/core/svg/SVGFilterPrimitiveStandardAttributes.cpp:143
#7  0x000000000f562692 in svgAttributeChanged () at ../../third_party/WebKit/Source/core/svg/SVGFEColorMatrixElement.cpp:97
#8  0x00000000092c6449 in attributeChanged () at ../../third_party/WebKit/Source/core/svg/SVGElement.cpp:859
#9  0x0000000005ed47ca in didModifyAttribute () at ../../third_party/WebKit/Source/core/dom/Element.cpp:2950
#10 setAttributeInternal () at ../../third_party/WebKit/Source/core/dom/Element.cpp:1059
#11 setAttribute () at ../../third_party/WebKit/Source/core/dom/Element.cpp:1012
#12 0x00000000098edd36 in setAttributeMethod () at gen/blink/bindings/core/v8/V8Element.cpp:1391
#13 setAttributeMethodCallback () at gen/blink/bindings/core/v8/V8Element.cpp:1401
#14 0x0000000004294ade in Call () at ../../v8/src/arguments.cc:33
#15 0x00000000031f5a83 in HandleApiCallHelper<false> () at ../../v8/src/builtins.cc:1077
#16 0x0000000003206e25 in Builtin_implHandleApiCall () at ../../v8/src/builtins.cc:1100
#17 Builtin_HandleApiCall () at ../../v8/src/builtins.cc:1096

senorblanco, do you want to own it and fix it? I overlooked it due to misconfigured email filters, but can fix it if you like.
Labels: M-42
Project Member

Comment 9 by ClusterFuzz, Apr 3 2015

Labels: -Security_Impact-Beta Security_Impact-Stable
Project Member

Comment 10 by ClusterFuzz, Apr 4 2015

ClusterFuzz has detected this issue as fixed in range 323876:323879.

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

Fuzzer: Miaubiz_svg_fuzzer
Job Type: Linux_asan_chrome_mp

Crash Type: Container-overflow READ 4
Crash Address: 0x602000065750
Crash State:
  blink::FEColorMatrix::createImageFilter
  blink::FilterEffect::createImageFilterWithoutValidation
  blink::SkiaImageFilterBuilder::build
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=316793:316810
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=323876:323879

Minimized Testcase (5.82 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97pnz2ZhT8jbhH--6BisaAdiCglwJovyAYeifbmnKHi3mhI2BwRp1FssTPUBHEJh51UvtFGIL8nYoOgdJlY6GXIN1Zw81CdoKC_eEnegMabkGGSGcA-ArNA4Upme8O5uxDRAAOlfkYb6OeVjmi5EfYvYLTEog

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

Project Member

Comment 11 by ClusterFuzz, Apr 5 2015

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

Fuzzer: Inferno_layout_test_unmodified
Job Type: Linux_asan_chrome_mp

Crash Type: Container-overflow READ 4
Crash Address: 0x602000064c90
Crash State:
  blink::FEColorMatrix::createImageFilter
  blink::FilterEffect::createImageFilterWithoutValidation
  blink::SkiaImageFilterBuilder::build
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=317506:317512

Minimized Testcase (4.49 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96Mw4W9TOpnQ2Vm7PggpCoak2JfP3eabrPGR89CVHvt9GbMI2uWw22-YZP2kRRQgX1lASxapQVy4hICStDclwStuC_ZM6_783FaQuTDzgBEWf_ocJDQNjt0XP_RSO5gcEgqDU6fKyklwShSnIs_nWSjx1Az2w

Filer: inferno
Cc: attek...@gmail.com
Status: Assigned
 Issue 475469  has been merged into this issue.
Project Member

Comment 14 by ClusterFuzz, Apr 10 2015

Labels: Nag
schenney@: 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
I can't reproduce this on ToT and we do get a console error for invalid values attribute. I agree that we shouldn't be checking but I'll make sure the setAttribute caller is always passing the correct size.
Are you using an ASAN build with these env options set. You need the detect_container_overflow=1

ASAN_OPTIONS = alloc_dealloc_mismatch=0:strict_memcmp=0:redzone=128:malloc_context_size=128:detect_stack_use_after_return=1:max_uar_stack_size_log=16:handle_segv=1:symbolize=false:check_malloc_usable_size=0:fast_unwind_on_fatal=1:allocator_may_return_null=1:detect_odr_violation=0:detect_container_overflow=1
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 10 2015

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

------------------------------------------------------------------
r193571 | schenney@chromium.org | 2015-04-10T22:35:26.259585Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/svg/SVGFEColorMatrixElement.cpp?r1=193571&r2=193570&pathrev=193571
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/graphics/filters/FEColorMatrix.cpp?r1=193571&r2=193570&pathrev=193571

Explicitly enforce values size in feColorMatrix.

R=senorblanco@chromium.org
BUG= 468519 

Review URL: https://codereview.chromium.org/1075413002
-----------------------------------------------------------------
Project Member

Comment 18 by ClusterFuzz, Apr 13 2015

ClusterFuzz has detected this issue as fixed in range 324711:324828.

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

Fuzzer: Inferno_layout_test_unmodified
Job Type: Linux_asan_chrome_mp

Crash Type: Container-overflow READ 4
Crash Address: 0x602000064c90
Crash State:
  blink::FEColorMatrix::createImageFilter
  blink::FilterEffect::createImageFilterWithoutValidation
  blink::SkiaImageFilterBuilder::build
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=317506:317512
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=324711:324828

Minimized Testcase (4.49 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96Mw4W9TOpnQ2Vm7PggpCoak2JfP3eabrPGR89CVHvt9GbMI2uWw22-YZP2kRRQgX1lASxapQVy4hICStDclwStuC_ZM6_783FaQuTDzgBEWf_ocJDQNjt0XP_RSO5gcEgqDU6fKyklwShSnIs_nWSjx1Az2w

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

Status: Fixed
Status: Started
I have to reopen. The fix violates the web spec so we'll need another round.
Cc: f...@opera.com pdr@chromium.org e...@opera.com
Project Member

Comment 22 by bugdroid1@chromium.org, Apr 16 2015

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

------------------------------------------------------------------
r193911 | schenney@chromium.org | 2015-04-16T23:40:41.049067Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/graphics/filters/FEColorMatrix.cpp?r1=193911&r2=193910&pathrev=193911
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash1-expected.txt?r1=193911&r2=193910&pathrev=193911
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash2-expected.txt?r1=193911&r2=193910&pathrev=193911
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/svg/SVGFEColorMatrixElement.cpp?r1=193911&r2=193910&pathrev=193911
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/graphics/filters/FEColorMatrix.h?r1=193911&r2=193910&pathrev=193911
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash1.html?r1=193911&r2=193910&pathrev=193911
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash2.html?r1=193911&r2=193910&pathrev=193911

Rework the checks for too-few values in feColorMatrix filter.

Revert the prevous fix because it was flat-out wrong for some
filters. Here we add checks at all points the values are used,
because it is impossible to enforce an always-valid m_values vector
inside the FEColorMatrix object.

For example, we need to support any ordering of setAttribute('type', ...)
and setAttribute('values', ...), but the valid number of values depends
on the type. We couldn't set the type from "hueRotate" to "matrix", for
example, without first adding more values than are necessary for the
"hueRotate". That's a bad user experience.

R=fs@opera.com,ed@opera.com,pdr@chromium.org
BUG= 468519 

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

Comment 24 by ClusterFuzz, Apr 21 2015

Labels: -Restrict-View-SecurityTeam M-43 Restrict-View-SecurityNotify Merge-Triage
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
Cc: timwillis@chromium.org
Labels: -M-42 -Merge-Triage Merge-Requested
Merge requested for M43 (branch 2357)
Labels: -Merge-Requested Merge-Review-43 Hotlist-Merge-Review
[Automated comment] Reverts referenced in bugdroid comments, needs manual review.

Comment 27 by laforge@google.com, May 11 2015

Labels: -Merge-Review-43 Merge-Approved
Project Member

Comment 28 by bugdroid1@chromium.org, May 13 2015

Labels: -Merge-Approved merge-merged-2357
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=195283

------------------------------------------------------------------
r195283 | schenney@chromium.org | 2015-05-13T01:13:11.384514Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/branches/chromium/2357/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash1.html?r1=195283&r2=195282&pathrev=195283
   A http://src.chromium.org/viewvc/blink/branches/chromium/2357/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash2.html?r1=195283&r2=195282&pathrev=195283
   M http://src.chromium.org/viewvc/blink/branches/chromium/2357/Source/platform/graphics/filters/FEColorMatrix.cpp?r1=195283&r2=195282&pathrev=195283
   A http://src.chromium.org/viewvc/blink/branches/chromium/2357/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash1-expected.txt?r1=195283&r2=195282&pathrev=195283
   A http://src.chromium.org/viewvc/blink/branches/chromium/2357/LayoutTests/svg/filters/feColorMatrix-setAttribute-crash2-expected.txt?r1=195283&r2=195282&pathrev=195283
   M http://src.chromium.org/viewvc/blink/branches/chromium/2357/Source/platform/graphics/filters/FEColorMatrix.h?r1=195283&r2=195282&pathrev=195283

Rework the checks for too-few values in feColorMatrix filter.

Revert the prevous fix because it was flat-out wrong for some
filters. Here we add checks at all points the values are used,
because it is impossible to enforce an always-valid m_values vector
inside the FEColorMatrix object.

For example, we need to support any ordering of setAttribute('type', ...)
and setAttribute('values', ...), but the valid number of values depends
on the type. We couldn't set the type from "hueRotate" to "matrix", for
example, without first adding more values than are necessary for the
"hueRotate". That's a bad user experience.

TBR=laforge@chromium.org
BUG= 468519 

Review URL: https://codereview.chromium.org/1087283002

(cherry picked from commit 9ae9e4492c79ea25f96297f198c4ad57636ab049)

Review URL: https://codereview.chromium.org/1135023003
-----------------------------------------------------------------
Labels: -Nag Release-0-M43
Labels: -reward-topanel reward-unpaid CVE-2015-1257 reward-1500
$1000 for the bug, $500 Fuzzer bonus for a total of $1500. I'll start payment shortly. 
Labels: -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Processing via our e-payment system can take up to two weeks, but the reward should be on its way to you. Thanks again for your help!

(Note: sorry for the delay here - it turns out in the new payment system, these payments were waiting for a second approval from me).
Project Member

Comment 33 by ClusterFuzz, Jul 28 2015

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member

Comment 34 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 35 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
Labels: CVE_description-submitted

Sign in to add a comment