Security: Heap overflow write in SkEdgeBuilder::buildPoly |
|||||||||||||||||||
Issue descriptionThis was originally reported to Mozilla by abbaolk@gmail.com as https://bugzilla.mozilla.org/show_bug.cgi?id=1465686. Mozilla fixed this downstream but wanted to give us a heads up. ---- "User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20180517113820 Steps to reproduce: This is a variant of vulnerability hrer:https://bugzilla.mozilla.org/show_bug.cgi?id=1418447 System: Ubuntu 16.04.4 LTS (64bit) Firefox version: 60.0.1 (64-bit) RAM size:16G SWAP size:5.9G Root cause: https://searchfox.org/mozilla-central/source/gfx/skia/skia/src/core/SkEdgeBuilder.cpp#255 SkPath::Iter iter(path, true); SkPoint pts[4]; SkPath::Verb verb; size_t maxEdgeCount = path.countPoints(); if (iclip) { // clipping can turn 1 line into (up to) kMaxClippedLineSegments, since // we turn portions that are clipped out on the left/right into vertical // segments. maxEdgeCount *= SkLineClipper::kMaxClippedLineSegments; } size_t edgeSize; char* edge; switch (fEdgeType) { case kEdge: edgeSize = sizeof(SkEdge); edge = (char*)fAlloc.makeArrayDefault<SkEdge>(maxEdgeCount); break; case kAnalyticEdge: edgeSize = sizeof(SkAnalyticEdge); edge = (char*)fAlloc.makeArrayDefault<SkAnalyticEdge>(maxEdgeCount); break; case kBezier: edgeSize = sizeof(SkLine); edge = (char*)fAlloc.makeArrayDefault<SkLine>(maxEdgeCount); break; } .................................... T* makeArrayDefault(size_t count) { uint32_t safeCount = SkTo<uint32_t>(count); "SkTo<uint32_t>(count)" just convert "maxEdgeCount" to a uint32_t type variable. So if "maxEdgeCount" is larger than 0x100000000(4G),....... Steps to reproduce: To trigger the vulnerability, we need about 16G memory. Open attached html file(with attached gdb) and clicker "trigger", ff will crash in less than 10 minutes.(My computer is pretty old, with better CPU and larger memory, ff should crash more quickly) (gdb) sharedlibrary libxul Reading symbols from /usr/lib/firefox/libxul.so...Reading symbols from /usr/lib/debug/.build-id/8d/03b3c494a48356d5df2ea89f4fcbfa565904c4.debug...done. done. (gdb) bt 8 #0 SkEdgeBuilder::CombineVertical (this=0x7ffce76b9750, edge=0x7f08dae5f018, last=0x1680000) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:22 #1 0x00007f08e774ffe8 in SkEdgeBuilder::addPolyLine (shiftUp=23592960, edgePtr=@0x7ffce76b91c8: 0x7f08dae5f030, edgeSize=40, edge=@0x7ffce76b91c0: 0x7f08dae5f018 "8\\\334\332\b\177", pts=0x7ffce76b9240, this=0x7ffce76b9750) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.h:110 #2 SkEdgeBuilder::buildPoly (this=this@entry=0x7ffce76b9750, path=..., iclip=iclip@entry=0x7ffce76b96b0, shiftUp=shiftUp@entry=2, canCullToTheRight=canCullToTheRight@entry=true) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:306 #3 0x00007f08e7755b57 in SkEdgeBuilder::build (this=this@entry=0x7ffce76b9750, path=..., iclip=0x7ffce76b96b0, shiftUp=2, canCullToTheRight=<optimized out>, edgeType=SkEdgeBuilder::kEdge) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:354 #4 0x00007f08e7755d7b in SkEdgeBuilder::build_edges (this=this@entry=0x7ffce76b9750, path=..., shiftedClip=shiftedClip@entry=0x7ffce76b96b0, shiftEdgesUp=shiftEdgesUp@entry=2, pathContainedInClip=pathContainedInClip@entry=false, edgeType=edgeType@entry=SkEdgeBuilder::kEdge) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkEdgeBuilder.cpp:457 #5 0x00007f08e77ab814 in sk_fill_path (path=..., clipRect=..., blitter=blitter@entry=0x7ffce76b9a10, start_y=-150, stop_y=150, shiftEdgesUp=shiftEdgesUp@entry=2, pathContainedInClip=false) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkScan_Path.cpp:406 #6 0x00007f08e74f46d6 in SkScan::SAAFillPath (path=..., blitter=blitter@entry=0x7ffce76ba618, ir=..., clipBounds=..., forceRLE=forceRLE@entry=false) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkScan_AntiPath.cpp:629 #7 0x00007f08e74f4d12 in SkScan::AntiFillPath (path=..., origClip=..., blitter=<optimized out>, blitter@entry=0x7ffce76ba618, forceRLE=forceRLE@entry=false, daaRecord=daaRecord@entry=0x0) at /build/firefox-AIw0T6/firefox-60.0.1+build2/gfx/skia/skia/src/core/SkScan_AntiPath.cpp:747 (More stack frames follow...)
,
Jun 5 2018
Assigning the labels since this went untriaged for a few days. I presume this impacts stable channel.
,
Jun 5 2018
Again, base/numerics from Chromium is a pure header library with no further dependencies. It's the best option for dealing with integer overflow in a consistent way. Please consider using it. Thanks!
,
Jun 7 2018
I have proposed base/numerics to the eng leads again, waiting for feedback/action. We merged the patch from Mozilla into 68, so we do have the asserts in place now.
,
Jun 15 2018
Hi! Any update from eng leads?
,
Jun 20 2018
reed: Uh oh! This issue still open and hasn't been updated in the last 19 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. 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
,
Jun 20 2018
I still cannot repro this using poc16G.html so not sure if there is still an issue.
,
Jun 20 2018
Over on mozilla bug they have this fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/9019db1eaddb
,
Jun 20 2018
... ok, just repro'd it on my mac.
,
Jun 21 2018
,
Jun 23 2018
mozilla have assigned CVE-2018-12371 to this issue
,
Jul 6
reed: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. 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
,
Jul 11
Is this fixed per #4 ("We merged the patch from Mozilla into 68, so we do have the asserts in place now.")? Can someone from the Skia team please provide an update? Thanks!
,
Jul 13
The following revision refers to this bug: https://skia.googlesource.com/skia/+/1128ebcb6dde49b639b5ba9917c14bb8b7a9eaae commit 1128ebcb6dde49b639b5ba9917c14bb8b7a9eaae Author: Mike Reed <reed@google.com> Date: Fri Jul 13 21:00:14 2018 make u32 check runtime Bug: 848521 Change-Id: Id3deb30447cc5caa27203b46b6b257e76cd88679 Reviewed-on: https://skia-review.googlesource.com/140986 Reviewed-by: Herb Derby <herb@google.com> Reviewed-by: Mike Klein <mtklein@google.com> Commit-Queue: Mike Reed <reed@google.com> [modify] https://crrev.com/1128ebcb6dde49b639b5ba9917c14bb8b7a9eaae/include/private/SkArenaAlloc.h
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e5807fa8f7c4c8a54eb79c4baa2ed821db110bb commit 2e5807fa8f7c4c8a54eb79c4baa2ed821db110bb Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Fri Jul 13 22:49:28 2018 Roll src/third_party/skia a80a012456a9..1128ebcb6dde (1 commits) https://skia.googlesource.com/skia.git/+log/a80a012456a9..1128ebcb6dde git log a80a012456a9..1128ebcb6dde --date=short --no-merges --format='%ad %ae %s' 2018-07-13 reed@google.com make u32 check runtime Created with: gclient setdep -r src/third_party/skia@1128ebcb6dde The AutoRoll server is located here: https://autoroll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:848521 TBR=robertphillips@chromium.org Change-Id: I7a78fd591eae7f4a7916fcce8e977fa1d8a8ed80 Reviewed-on: https://chromium-review.googlesource.com/1136772 Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#575102} [modify] https://crrev.com/2e5807fa8f7c4c8a54eb79c4baa2ed821db110bb/DEPS
,
Jul 20
,
Jul 27
Can we mark this as fixed?
,
Jul 31
We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue? For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 2
,
Aug 10
Hi reed@, is this already fixed?
,
Aug 11
The following revision refers to this bug: https://skia.googlesource.com/skia/+/26f9b10a8add3abe552cb86abe2c5ff7f9c0b980 commit 26f9b10a8add3abe552cb86abe2c5ff7f9c0b980 Author: Mike Reed <reed@google.com> Date: Sat Aug 11 00:10:04 2018 check for overflow in maxedgecount Bug: 848521 Change-Id: I5d5f28bc2ceef6e7a90b87f5e8c064473c6f67a3 Reviewed-on: https://skia-review.googlesource.com/146880 Auto-Submit: Mike Reed <reed@google.com> Commit-Queue: Herb Derby <herb@google.com> Reviewed-by: Herb Derby <herb@google.com> [modify] https://crrev.com/26f9b10a8add3abe552cb86abe2c5ff7f9c0b980/src/core/SkEdgeBuilder.cpp
,
Aug 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90a8fb500096caf920d8999a1d314fdda4701dc0 commit 90a8fb500096caf920d8999a1d314fdda4701dc0 Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Sat Aug 11 08:33:04 2018 Roll src/third_party/skia f3ca1c6abb07..5fb1b7e0ffe1 (2 commits) https://skia.googlesource.com/skia.git/+log/f3ca1c6abb07..5fb1b7e0ffe1 git log f3ca1c6abb07..5fb1b7e0ffe1 --date=short --no-merges --format='%ad %ae %s' 2018-08-11 angle-skia-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com Roll third_party/externals/angle2 04c084dd3cf4..ea926a362b77 (4 commits) 2018-08-11 reed@google.com check for overflow in maxedgecount Created with: gclient setdep -r src/third_party/skia@5fb1b7e0ffe1 The AutoRoll server is located here: https://autoroll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:848521 TBR=djsollen@chromium.org Change-Id: I010a73604f808dbb485a82845cf42e72a0e464be Reviewed-on: https://chromium-review.googlesource.com/1171406 Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#582442} [modify] https://crrev.com/90a8fb500096caf920d8999a1d314fdda4701dc0/DEPS
,
Aug 16
A couple CLs seem to have landed related to this, has the fix been completed? If so, could you close the bug?
,
Aug 24
,
Sep 5
,
Sep 11
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5912356973182976.
,
Sep 11
Clusterfuzz wasn't able to reproduce the test case in the original report. reed: Is this bug fixed now?
,
Sep 11
Testcase 5912356973182976 failed to reproduce the crash. Please inspect the program output at https://clusterfuzz.com/testcase?key=5912356973182976.
,
Oct 2
reed: Ping, can we close this as fixed? Thanks.
,
Oct 2
,
Oct 3
,
Oct 15
,
Oct 26
,
Oct 26
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 26
,
Jan 9
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 |
|||||||||||||||||||
Comment 1 by awhalley@chromium.org
, Jun 1 20182.3 KB
2.3 KB Download