New issue
Advanced search Search tips

Issue 883178 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Sep 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Skia heap use-after-free SkBitmap::setPixelRef not set ref count to pixmap

Reported by hungtt28...@gmail.com, Sep 12

Issue description



VULNERABILITY DETAILS

There is a heap use-after-free in Skia at SkBitmap::setPixels(), but the root cause may be in 
SkBitmap::setPixelRef(). This function do not increase fRefCnt when set pixels address 
into SkPixmap, that lead to incorrect reference count and could be used to trigger use-after-free.

https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkBitmap.cpp?type=cs&q=setPixelRef&g=0&l=192




REPRODUCTION CASE

Build Skia program with ASAN
============================

#include "SkBitmap.h"
#include "SkPixmap.h"

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


SkBitmap bitmap;
SkPixmap pixmap;
uint8_t buffer[256];

// alloc 
bitmap.allocN32Pixels(16, 16, true);

// get pixmap info
bitmap.peekPixels(&(pixmap));

// replace pixels by buffer, free the pixels
bitmap.setPixels(buffer);

// heap use-after-free, pixmap still ref to the old pixels
pixmap.getColor(0, 0);


SkDebugf("width: %d height: %d\n", pixmap.width(), pixmap.height());
SkDebugf("0x%08x\n", pixmap.getColor(0, 0));


return 0; 
}

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION

=================================================================
==661==ERROR: AddressSanitizer: heap-use-after-free on address 0x619000000a80 at pc 0x00000069d9bb bp 0x7ffeee6844c0 sp 0x7ffeee6844b8
READ of size 4 at 0x619000000a80 thread T0
    #0 0x69d9ba in SkPixmap::getColor(int, int) const /home/monkie/skia-latest/out/asan/../../src/core/SkPixmap.cpp:380:30
    #1 0x63fe86 in main /home/monkie/skia-latest/out/asan/../../tools/test.cpp:22:8
    #2 0x7f76fba6382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #3 0x54bd78 in _start (/home/monkie/skia-latest/out/asan/test+0x54bd78)

0x619000000a80 is located 0 bytes inside of 1024-byte region [0x619000000a80,0x619000000e80)
freed by thread T0 here:
    #0 0x60e938 in __interceptor_cfree.localalias.0 /tmp/tmpFq_AZD/llvm/out/../projects/compiler-rt/lib/asan/asan_malloc_linux.cc:76
    #1 0x64ec06 in SkMallocPixelRef::~SkMallocPixelRef() /home/monkie/skia-latest/out/asan/../../src/core/SkMallocPixelRef.cpp:147:9
    #2 0x64ec06 in SkMallocPixelRef::~SkMallocPixelRef() /home/monkie/skia-latest/out/asan/../../src/core/SkMallocPixelRef.cpp:145
    #3 0x642b26 in SkRefCntBase::unref() const /home/monkie/skia-latest/out/asan/../../include/core/SkRefCnt.h:89:19
    #4 0x642b26 in void SkSafeUnref<SkPixelRef>(SkPixelRef*) /home/monkie/skia-latest/out/asan/../../include/core/SkRefCnt.h:162
    #5 0x642b26 in sk_sp<SkPixelRef>::reset(SkPixelRef*) /home/monkie/skia-latest/out/asan/../../include/core/SkRefCnt.h:309
    #6 0x642b26 in sk_sp<SkPixelRef>::operator=(sk_sp<SkPixelRef>&&) /home/monkie/skia-latest/out/asan/../../include/core/SkRefCnt.h:279
    #7 0x642b26 in SkBitmap::setPixelRef(sk_sp<SkPixelRef>, int, int) /home/monkie/skia-latest/out/asan/../../src/core/SkBitmap.cpp:180
    #8 0x642b26 in SkBitmap::setPixels(void*) /home/monkie/skia-latest/out/asan/../../src/core/SkBitmap.cpp:207
    #9 0x63fe7a in main /home/monkie/skia-latest/out/asan/../../tools/test.cpp:19:8
    #10 0x7f76fba6382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

previously allocated by thread T0 here:
    #0 0x60eaf8 in __interceptor_malloc /tmp/tmpFq_AZD/llvm/out/../projects/compiler-rt/lib/asan/asan_malloc_linux.cc:88
    #1 0x6d4038 in sk_malloc_flags(unsigned long, unsigned int) /home/monkie/skia-latest/out/asan/../../src/ports/SkMemory_malloc.cpp:69:13
    #2 0x64de6a in sk_malloc_canfail(unsigned long) /home/monkie/skia-latest/out/asan/../../include/private/SkMalloc.h:93:12
    #3 0x64de6a in SkMallocPixelRef::MakeUsing(void* (*)(unsigned long), SkImageInfo const&, unsigned long) /home/monkie/skia-latest/out/asan/../../src/core/SkMallocPixelRef.cpp:76
    #4 0x64de6a in SkMallocPixelRef::MakeAllocate(SkImageInfo const&, unsigned long) /home/monkie/skia-latest/out/asan/../../src/core/SkMallocPixelRef.cpp:86
    #5 0x643468 in SkBitmap::tryAllocPixels(SkImageInfo const&, unsigned long) /home/monkie/skia-latest/out/asan/../../src/core/SkBitmap.cpp:238:28
    #6 0x63fde2 in SkBitmap::allocPixels(SkImageInfo const&, unsigned long) /home/monkie/skia-latest/out/asan/../../include/core/SkBitmap.h:515:9
    #7 0x63fde2 in SkBitmap::allocPixels(SkImageInfo const&) /home/monkie/skia-latest/out/asan/../../include/core/SkBitmap.h:551
    #8 0x63fde2 in SkBitmap::allocN32Pixels(int, int, bool) /home/monkie/skia-latest/out/asan/../../include/core/SkBitmap.h:595
    #9 0x63fde2 in main /home/monkie/skia-latest/out/asan/../../tools/test.cpp:13
    #10 0x7f76fba6382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: heap-use-after-free /home/monkie/skia-latest/out/asan/../../src/core/SkPixmap.cpp:380:30 in SkPixmap::getColor(int, int) const
Shadow bytes around the buggy address:
  0x0c327fff8100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8110: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8120: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fff8140: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c327fff8150:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8160: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8170: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff8190: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c327fff81a0: fd fd fd fd fd fd fd fd 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
==661==ABORTING



 
Cc: bunge...@chromium.org
Components: Internals>Skia
Labels: Security_Severity-Medium Security_Impact-Stable M-70 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Owner: reed@chromium.org
Status: Assigned (was: Unconfirmed)
reed: Can you take a look or help route?
Owner: reed@google.com
Status: WontFix (was: Assigned)
This is incorrect/invalid usage of the Skia api.

    /** Replaces SkPixelRef with pixels, preserving SkImageInfo and rowBytes().
        Sets SkPixelRef origin to (0, 0).

        If pixels is nullptr, or if info().colorType equals kUnknown_SkColorType;
        release reference to SkPixelRef, and set SkPixelRef to nullptr.

        Caller is responsible for handling ownership pixel memory for the lifetime
        of SkBitmap and SkPixelRef.

        @param pixels  address of pixel storage, managed by caller
    */
    void setPixels(void* pixels);

The caller in this test has called setPixels(buffer). This "replaced" the previous pixelref (deleting it in this case). SkPixmap is document to not manage any of its memory (it is up to the caller).
Dear reed,

Actually, SkPixmap object in the poc not a new object. It's pointer of the fPixmap.
And I think SkBitmap internal object still get wrong state, fPixmap should be fixed.
Other areas where use fPixmap may crash.


bool SkBitmap::peekPixels(SkPixmap* pmap) const {
    if (this->getPixels()) {
        if (pmap) {
            *pmap = fPixmap;
        }
        return true;
    }
    return false;
}

This pixmap does not change, but it was never an "owner" of the memory it receives in peekPixels(). The sole owner is still just the SkBitmap, and when it releases the memory (because setPixels() was called, there was no other owner, so the memory was freed. At that point, the pixmap's pointer to pixels is invalid, and should not be used.
That's right. Thank you for the review.
Project Member

Comment 6 by sheriffbot@chromium.org, Dec 20

Labels: -Restrict-View-SecurityTeam 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