1372421 - skia: heap-buffer-overflow in SkARGB32_Shader_Blitter::blitRect

Status: RESOLVED INVALID

(Core :: Graphics, defect)

Description

[Tyson Smith [:tsmith]]

I logged this upstream as https://bugs.chromium.org/p/skia/issues/detail?id=6754 ping me if you need access.

Not sure if this affect Firefox.

==27013==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60c000000540 at pc 0x000000c66c0b bp 0x7ffda4f90950 sp 0x7ffda4f90948
WRITE of size 4 at 0x60c000000540 thread T0
    #0 0xc66c0a in sse2::memset32(unsigned int*, unsigned int, int) src/opts/SkUtils_opts.h:20:23
    #1 0x1c15ce9 in SkARGB32_Shader_Blitter::blitRect(int, int, int, int) src/core/SkBlitter_ARGB32.cpp:405:28
    #2 0xde8afb in walk_convex_edges(SkEdge*, SkPath::FillType, SkBlitter*, int, int, void (*)(SkBlitter*, int, bool)) src/core/SkScan_Path.cpp:251:26
    #3 0xde6ae9 in sk_fill_path(SkPath const&, SkIRect const&, SkBlitter*, int, int, int, bool) src/core/SkScan_Path.cpp:465:9
    #4 0xdeb2fe in SkScan::FillPath(SkPath const&, SkRegion const&, SkBlitter*) src/core/SkScan_Path.cpp:666:9
    #5 0xdcc511 in SkScan::FillPath(SkPath const&, SkRasterClip const&, SkBlitter*) src/core/SkScan_AntiPath.cpp:745:9
    #6 0xbaa641 in SkDraw::drawDevPath(SkPath const&, SkPaint const&, bool, SkBlitter*, bool) const src/core/SkDraw.cpp:1032:5
    #7 0xbab750 in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*) const src/core/SkDraw.cpp:1125:11
    #8 0x1bee76c in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) const src/core/SkDraw.h:56:15
    #9 0x1bee76c in SkBitmapDevice::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) src/core/SkBitmapDevice.cpp:233
    #10 0xa3b6de in SkCanvas::onDrawPath(SkPath const&, SkPaint const&) src/core/SkCanvas.cpp:2152:23
    #11 0x7651af in fuzz_drawPath(Fuzz*) fuzz/FuzzDrawFunctions.cpp:249:10
    #12 0x7651af in fuzz_DrawFunctions(Fuzz*) fuzz/FuzzDrawFunctions.cpp:347
    #13 0x7899b1 in fuzz_api(sk_sp<SkData>) fuzz/fuzz.cpp:170:13
    #14 0x7899b1 in fuzz_file(char const*) fuzz/fuzz.cpp:103
    #15 0x787d4d in main fuzz/fuzz.cpp:75:19
    #16 0x7fe02d35e82f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291
    #17 0x5d4e28 in _start (fuzz+0x5d4e28)

0x60c000000540 is located 0 bytes to the right of 128-byte region [0x60c0000004c0,0x60c000000540)
allocated by thread T0 here:
    #0 0x68d328 in __interceptor_malloc (fuzz+0x68d328)
    #1 0xf7906c in sk_malloc_flags(unsigned long, unsigned int) src/ports/SkMemory_malloc.cpp:73:15
    #2 0xf7906c in sk_malloc_throw(unsigned long) src/ports/SkMemory_malloc.cpp:59
    #3 0x1c0833f in SkARGB32_Shader_Blitter* SkArenaAlloc::make<SkARGB32_Shader_Blitter, SkPixmap const&, SkPaint const&, SkShaderBase::Context*&>(SkPixmap const&, SkPaint const&, SkShaderBase::Context*&) src/core/SkArenaAlloc.h:103:30
    #4 0x1c0833f in SkBlitter::Choose(SkPixmap const&, SkMatrix const&, SkPaint const&, SkArenaAlloc*, bool) src/core/SkBlitter.cpp:925
    #5 0xbaa39a in SkAutoBlitterChoose::choose(SkPixmap const&, SkMatrix const&, SkPaint const&, bool) src/core/SkAutoBlitterChoose.h:34:20
    #6 0xbaa39a in SkDraw::drawDevPath(SkPath const&, SkPaint const&, bool, SkBlitter*, bool) const src/core/SkDraw.cpp:978
    #7 0xbab750 in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool, bool, SkBlitter*) const src/core/SkDraw.cpp:1125:11
    #8 0x1bee76c in SkDraw::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) const src/core/SkDraw.h:56:15
    #9 0x1bee76c in SkBitmapDevice::drawPath(SkPath const&, SkPaint const&, SkMatrix const*, bool) src/core/SkBitmapDevice.cpp:233
    #10 0xa3b6de in SkCanvas::onDrawPath(SkPath const&, SkPaint const&) src/core/SkCanvas.cpp:2152:23
    #11 0x7651af in fuzz_drawPath(Fuzz*) fuzz/FuzzDrawFunctions.cpp:249:10
    #12 0x7651af in fuzz_DrawFunctions(Fuzz*) fuzz/FuzzDrawFunctions.cpp:347
    #13 0x7899b1 in fuzz_api(sk_sp<SkData>) fuzz/fuzz.cpp:170:13
    #14 0x7899b1 in fuzz_file(char const*) fuzz/fuzz.cpp:103
    #15 0x787d4d in main fuzz/fuzz.cpp:75:19
    #16 0x7fe02d35e82f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-buffer-overflow src/opts/SkUtils_opts.h:20:23 in sse2::memset32(unsigned int*, unsigned int, int)
Shadow bytes around the buggy address:
  0x0c187fff8050: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c187fff8060: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c187fff8070: 00 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa
  0x0c187fff8080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
  0x0c187fff8090: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c187fff80a0: 00 00 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa
  0x0c187fff80b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff80c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff80d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff80e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fff80f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==27013==ABORTING

Comment 1

[Lee Salzman [:lsalzman]]

I would appreciate a testcase to help make sense of this. The stack trace and asan output here is not enough to draw any firm conclusions of what is going on.

Comment 2

[Tyson Smith [:tsmith]]

(In reply to Lee Salzman [:lsalzman] from comment #1)
> I would appreciate a testcase to help make sense of this. The stack trace
> and asan output here is not enough to draw any firm conclusions of what is
> going on.

The test case is attached to the upstream bug and requires the upstream fuzzing harness. https://skia.googlesource.com/skia.git/+/master/fuzz/

Comment 3

[Lee Salzman [:lsalzman]]

This is a bug in Skia, but it thankfully does not affect us. We use the SK_RASTERIZE_EVEN_ROUNDING option, which modifies round_asymmetric_to_int()s behavior such that clipping gets properly handled for this test case. This was already fixed for our usage of SK_RASTERIZE_EVEN_ROUNDING by bug 1347262.

Comment 4

[Lee Salzman [:lsalzman]]

Actually, the fix in bug 1347262 hides this bug from affecting us, though the source of it might be a bit different. Still investigating...

Comment 5

[Lee Salzman [:lsalzman]]

So as far as I can tell by inspecting the running test case in the debugger, the rounding error coming from the SkConic is causing this, but otherwise it would just overlap with the fudge factor we're adding on to deal with SK_RASTERIZE_EVEN_ROUNDING already.

Now here is the really good news. Regardless, this rounding error is coming from SkConic, which means the path using conic segments. But we don't actually use conic segments in paths in Skia (i.e. SkPath::conicTo), so we're never going to trigger this code in Firefox in the first place. We're doubly safe on this one. Huzzah!

Comment 6

[Tyson Smith [:tsmith]]

(In reply to Lee Salzman [:lsalzman] from comment #5)
> Now here is the really good news. Regardless, this rounding error is coming
> from SkConic, which means the path using conic segments. But we don't
> actually use conic segments in paths in Skia (i.e. SkPath::conicTo), so
> we're never going to trigger this code in Firefox in the first place. We're
> doubly safe on this one. Huzzah!

Thanks Lee! I am happy to close this bug since we are unaffected and the issue has been reported upstream. Is that OK with you?

Comment 7

[Tyson Smith [:tsmith]]

Oops I mean to wait :P

Comment 8

[Lee Salzman [:lsalzman]]

I would also be happy to just close this one. :)

Comment 9

[Tyson Smith [:tsmith]]

Done. Thanks again.