New issue
Advanced search Search tips

Issue 845489 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Incomplete fix for crbug/844457 (Heap overflow in SkScan::FillPath due to precision error)

Project Member Reported by ifratric@google.com, May 22 2018

Issue description

Unfortunately, the fix for crbug/844457 is incomplete. While the fix does work on the Skia config Chrome uses, it will not work with SK_RASTERIZE_EVEN_ROUNDING enabled (e.g. example in Firefox). It is still possible to reproduce with a minor tweak to the original testcase that I included below.

Skia program to reproduce the issue:

Note: #define SK_RASTERIZE_EVEN_ROUNDING must be added to SkUserConfig.h

====================================

#include "SkCanvas.h"
#include "SkPath.h"
#include "SkBitmap.h"
#include "SkGradientShader.h"

int main (int argc, char * const argv[]) {

  int width = 100;
  int height = 100;

  SkBitmap bitmap;
  bitmap.allocN32Pixels(width, height);
  SkCanvas bitmapcanvas(bitmap);
  SkCanvas *canvas = &bitmapcanvas;

  SkPaint p;

  p.setAntiAlias(false);

  p.setStyle(SkPaint::kFill_Style);

  SkColor colors[2] = {SkColorSetARGB(10,0,0,0), SkColorSetARGB(10,255,255,255)};
  SkPoint points[2] = {
     SkPoint::Make(0.0f, 0.0f),
     SkPoint::Make(256.0f, 256.0f)
  };
  p.setShader(SkGradientShader::MakeLinear(
             points, colors, nullptr, 2,
             SkShader::kClamp_TileMode, 0, nullptr));

  SkPath path;
  path.moveTo(-30/64.0, -31/64.0 + 1/256.0);
  path.cubicTo(-31/64.0 + 1/256.0, -31/64.0 + 1/256.0,-31/64.0 + 1/256.0, -31/64.0 + 1/256.0,-31/64.0 + 1/256.0, 100);
  path.lineTo(100,100);
  path.lineTo(100,-31/64.0 + 1/256.0);

  canvas->drawPath(path, p);

  return 0; 
}

====================================


 
Cc: -reed@google.com
Components: Internals>Skia
Labels: Pri-1
Owner: reed@google.com
Status: Assigned (was: Unconfirmed)
Mike, can you take a look?
Project Member

Comment 2 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/3087c1f382f1cd547598dc75f47ccbc8fe1e6e0f

commit 3087c1f382f1cd547598dc75f47ccbc8fe1e6e0f
Author: Mike Reed <reed@google.com>
Date: Wed May 23 13:26:41 2018

tweak tolerance again for cubics

Bug:  845489 
Bug: skia:7995
Change-Id: I05554377bd5630b7134864b6db282358613f1030
Reviewed-on: https://skia-review.googlesource.com/129721
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Mike Reed <reed@google.com>

[modify] https://crrev.com/3087c1f382f1cd547598dc75f47ccbc8fe1e6e0f/src/core/SkScan_Path.cpp
[modify] https://crrev.com/3087c1f382f1cd547598dc75f47ccbc8fe1e6e0f/tests/ClipCubicTest.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fbe8fc147cffd192a15ba243545c44e232f0e92c

commit fbe8fc147cffd192a15ba243545c44e232f0e92c
Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 23 15:32:45 2018

Roll src/third_party/skia/ bb9cad053..f104fec6d (4 commits)

https://skia.googlesource.com/skia.git/+log/bb9cad053ba5..f104fec6d745

$ git log bb9cad053..f104fec6d --date=short --no-merges --format='%ad %ae %s'
2018-05-22 csmartdalton Delete GrDrawOp::wasRecorded
2018-05-23 reed tweak tolerance again for cubics
2018-05-23 robertphillips Add a Win10/QuadroP400 SKPBench bot
2018-05-22 mtklein Reland "start cleaning up non-skcms SkColorSpaceXforms"

Created with:
  roll-dep src/third_party/skia
BUG= chromium:845489 


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
TBR=allanmac@chromium.org

Change-Id: I9f7f64699351da82f908c75fd4c8604e70b69e86
Reviewed-on: https://chromium-review.googlesource.com/1070211
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@{#561097}
[modify] https://crrev.com/fbe8fc147cffd192a15ba243545c44e232f0e92c/DEPS

Comment 4 by mea...@chromium.org, May 30 2018

reed: Can we mark this as fixed? Thanks.

Comment 5 by reed@google.com, May 30 2018

Status: Fixed (was: Assigned)
Project Member

Comment 6 by sheriffbot@chromium.org, May 31 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Security_Severity-High Security_Impact-Stable OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Project Member

Comment 8 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