From 9dec1e5d10487bba20ea4ec2386ebbd65ea9352a Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 5 Sep 2018 12:12:37 +0200 Subject: [Backport] CVE-2018-16070 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Commit-Queue: Greg Daniel Reviewed-by: Michael Brüning --- .../third_party/skia/src/gpu/GrTessellator.cpp | 20 +++++++------- .../skia/src/gpu/ops/GrAAConvexPathRenderer.cpp | 22 +++++++++------ .../gpu/ops/GrAALinearizingConvexPathRenderer.cpp | 31 +++++++++++++++------- .../skia/src/gpu/ops/GrSmallPathRenderer.cpp | 6 +++++ 4 files changed, 52 insertions(+), 27 deletions(-) diff --git a/chromium/third_party/skia/src/gpu/GrTessellator.cpp b/chromium/third_party/skia/src/gpu/GrTessellator.cpp index 2974cf02db6..f26c3b925a2 100644 --- a/chromium/third_party/skia/src/gpu/GrTessellator.cpp +++ b/chromium/third_party/skia/src/gpu/GrTessellator.cpp @@ -2274,8 +2274,8 @@ int get_contour_count(const SkPath& path, SkScalar tolerance) { return contourCnt; } -int count_points(Poly* polys, SkPath::FillType fillType) { - int count = 0; +int64_t count_points(Poly* polys, SkPath::FillType fillType) { + int64_t count = 0; for (Poly* poly = polys; poly; poly = poly->fNext) { if (apply_fill_type(fillType, poly) && poly->fCount >= 3) { count += (poly->fCount - 2) * (TESSELLATOR_WIREFRAME ? 6 : 3); @@ -2284,8 +2284,8 @@ int count_points(Poly* polys, SkPath::FillType fillType) { return count; } -int count_outer_mesh_points(const VertexList& outerMesh) { - int count = 0; +int64_t count_outer_mesh_points(const VertexList& outerMesh) { + int64_t count = 0; for (Vertex* v = outerMesh.fHead; v; v = v->fNext) { for (Edge* e = v->fFirstEdgeBelow; e; e = e->fNextEdgeBelow) { count += TESSELLATOR_WIREFRAME ? 12 : 6; @@ -2327,13 +2327,14 @@ int PathToTriangles(const SkPath& path, SkScalar tolerance, const SkRect& clipBo Poly* polys = path_to_polys(path, tolerance, clipBounds, contourCnt, alloc, antialias, isLinear, &outerMesh); SkPath::FillType fillType = antialias ? SkPath::kWinding_FillType : path.getFillType(); - int count = count_points(polys, fillType); + int64_t count64 = count_points(polys, fillType); if (antialias) { - count += count_outer_mesh_points(outerMesh); + count64 += count_outer_mesh_points(outerMesh); } - if (0 == count) { + if (0 == count64 || count64 > SK_MaxS32) { return 0; } + int count = count64; void* verts = vertexAllocator->lock(count); if (!verts) { @@ -2366,11 +2367,12 @@ int PathToVertices(const SkPath& path, SkScalar tolerance, const SkRect& clipBou Poly* polys = path_to_polys(path, tolerance, clipBounds, contourCnt, alloc, false, &isLinear, nullptr); SkPath::FillType fillType = path.getFillType(); - int count = count_points(polys, fillType); - if (0 == count) { + int64_t count64 = count_points(polys, fillType); + if (0 == count64 || count64 > SK_MaxS32) { *verts = nullptr; return 0; } + int count = count64; *verts = new GrTessellator::WindingVertex[count]; GrTessellator::WindingVertex* vertsEnd = *verts; diff --git a/chromium/third_party/skia/src/gpu/ops/GrAAConvexPathRenderer.cpp b/chromium/third_party/skia/src/gpu/ops/GrAAConvexPathRenderer.cpp index badaadd329a..6ce8607d947 100644 --- a/chromium/third_party/skia/src/gpu/ops/GrAAConvexPathRenderer.cpp +++ b/chromium/third_party/skia/src/gpu/ops/GrAAConvexPathRenderer.cpp @@ -22,6 +22,7 @@ #include "SkPointPriv.h" #include "SkString.h" #include "SkTraceEvent.h" +#include "SkTypes.h" #include "glsl/GrGLSLFragmentShaderBuilder.h" #include "glsl/GrGLSLGeometryProcessor.h" #include "glsl/GrGLSLProgramDataManager.h" @@ -130,8 +131,8 @@ static void compute_vectors(SegmentArray* segments, normSide = SkPointPriv::kLeft_Side; } - *vCount = 0; - *iCount = 0; + int64_t vCount64 = 0; + int64_t iCount64 = 0; // compute normals at all points for (int a = 0; a < count; ++a) { Segment& sega = (*segments)[a]; @@ -147,11 +148,11 @@ static void compute_vectors(SegmentArray* segments, prevPt = &segb.fPts[p]; } if (Segment::kLine == segb.fType) { - *vCount += 5; - *iCount += 9; + vCount64 += 5; + iCount64 += 9; } else { - *vCount += 6; - *iCount += 12; + vCount64 += 6; + iCount64 += 12; } } @@ -164,9 +165,14 @@ static void compute_vectors(SegmentArray* segments, segb.fMid = segb.fNorms[0] + sega.endNorm(); segb.fMid.normalize(); // corner wedges - *vCount += 4; - *iCount += 6; + vCount64 += 4; + iCount64 += 6; } + if (vCount64 > SK_MaxS32 || iCount64 > SK_MaxS32) { + return; + } + *vCount = vCount64; + *iCount = iCount64; } struct DegenerateTestData { diff --git a/chromium/third_party/skia/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp b/chromium/third_party/skia/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp index cd87e286b5f..2c1ea50fc3f 100644 --- a/chromium/third_party/skia/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp +++ b/chromium/third_party/skia/src/gpu/ops/GrAALinearizingConvexPathRenderer.cpp @@ -255,10 +255,10 @@ private: int instanceCount = fPaths.count(); - int vertexCount = 0; - int indexCount = 0; - int maxVertices = DEFAULT_BUFFER_SIZE; - int maxIndices = DEFAULT_BUFFER_SIZE; + int64_t vertexCount = 0; + int64_t indexCount = 0; + int64_t maxVertices = DEFAULT_BUFFER_SIZE; + int64_t maxIndices = DEFAULT_BUFFER_SIZE; uint8_t* vertices = (uint8_t*) sk_malloc_throw(maxVertices * vertexStride); uint16_t* indices = (uint16_t*) sk_malloc_throw(maxIndices * sizeof(uint16_t)); for (int i = 0; i < instanceCount; i++) { @@ -270,9 +270,8 @@ private: continue; } - int currentIndices = tess.numIndices(); - SkASSERT(currentIndices <= UINT16_MAX); - if (indexCount + currentIndices > UINT16_MAX) { + int currentVertices = tess.numPts(); + if (vertexCount + currentVertices > static_cast(UINT16_MAX)) { // if we added the current instance, we would overflow the indices we can store in a // uint16_t. Draw what we've got so far and reset. this->draw(target, gp.get(), pipeline, vertexCount, vertexStride, vertices, @@ -280,13 +279,23 @@ private: vertexCount = 0; indexCount = 0; } - int currentVertices = tess.numPts(); if (vertexCount + currentVertices > maxVertices) { maxVertices = SkTMax(vertexCount + currentVertices, maxVertices * 2); + if (maxVertices * vertexStride > SK_MaxS32) { + sk_free(vertices); + sk_free(indices); + return; + } vertices = (uint8_t*) sk_realloc_throw(vertices, maxVertices * vertexStride); } + int currentIndices = tess.numIndices(); if (indexCount + currentIndices > maxIndices) { maxIndices = SkTMax(indexCount + currentIndices, maxIndices * 2); + if (maxIndices * sizeof(uint16_t) > SK_MaxS32) { + sk_free(vertices); + sk_free(indices); + return; + } indices = (uint16_t*) sk_realloc_throw(indices, maxIndices * sizeof(uint16_t)); } @@ -296,8 +305,10 @@ private: vertexCount += currentVertices; indexCount += currentIndices; } - this->draw(target, gp.get(), pipeline, vertexCount, vertexStride, vertices, indexCount, - indices); + if (vertexCount <= SK_MaxS32 && indexCount <= SK_MaxS32) { + this->draw(target, gp.get(), pipeline, vertexCount, vertexStride, vertices, indexCount, + indices); + } sk_free(vertices); sk_free(indices); } diff --git a/chromium/third_party/skia/src/gpu/ops/GrSmallPathRenderer.cpp b/chromium/third_party/skia/src/gpu/ops/GrSmallPathRenderer.cpp index 873ad35defb..10ae8c38b5d 100644 --- a/chromium/third_party/skia/src/gpu/ops/GrSmallPathRenderer.cpp +++ b/chromium/third_party/skia/src/gpu/ops/GrSmallPathRenderer.cpp @@ -269,6 +269,12 @@ private: SkASSERT(vertexStride == sizeof(SkPoint) + sizeof(GrColor) + 2*sizeof(uint16_t)); const GrBuffer* vertexBuffer; + + // We need to make sure we don't overflow a 32 bit int when we request space in the + // makeVertexSpace call below. + if (instanceCount > SK_MaxS32 / kVerticesPerQuad) { + return; + } void* vertices = target->makeVertexSpace(vertexStride, kVerticesPerQuad * instanceCount, &vertexBuffer, -- cgit v1.2.3