CVE-2026-17745
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifsrc/gpu/ganesh/Device_drawTexture.cpp |
modified | |
forsrc/gpu/ganesh/Device_drawTexture.cpp |
modified | |
forsrc/gpu/ganesh/ops/TextureOp.cpp |
modified | |
ifsrc/gpu/ganesh/ops/TextureOp.cpp |
modified |
Files Changed
gn/tests.gnisrc/gpu/ganesh/Device_drawTexture.cppsrc/gpu/ganesh/SurfaceDrawContext.cppsrc/gpu/ganesh/SurfaceDrawContext.hsrc/gpu/ganesh/ops/TextureOp.cpp
Patch
From b6c7a5bc832b785a445ab89b3365ed80ab2e9d47 Mon Sep 17 00:00:00 2001 From: Robert Phillips <[email protected]> Date: Wed, 17 Jun 2026 15:01:07 -0400 Subject: [PATCH] Reland "[Ganesh] TextureOp quad illegal memory access" This reverts commit 5e976cb2f034067e006ec8db2889791f0d4eb443. Reason for revert: The unit test was exceeding maxTextureSize on some devices Original change's description: > Revert "[Ganesh] TextureOp quad illegal memory access" > > This reverts commit 148b2b1948019f8f89435b4df7d598224a2ab0e1. > > Reason for revert: Crashing on some Android devices > > Failure Link: <LINK TO FAILURE> > > Original change's description: > > [Ganesh] TextureOp quad illegal memory access > > > > This CL fixes an overflow in the number of allowed quads in a TextureOp. It works on two fronts: > > It conservatively tracks the number of quads (incl. perspective) > > It prevents a fast path when there possibly might be an overflow. > > > > Bug: b/500172224 > > Change-Id: Icd92ed5c80d81cdbfea8d2463407cc48a0a32843 > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1262376 > > Commit-Queue: Robert Phillips <[email protected]> > > Reviewed-by: Michael Ludwig <[email protected]> > > Bug: b/500172224 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Change-Id: Ic4ea8537eb642131fbbcb14a485b11ce3ed4a636 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1268256 > Bot-Commit: [email protected] <[email protected]> > Auto-Submit: Robert Phillips <[email protected]> > Commit-Queue: [email protected] <[email protected]> Bug: b/500172224 Change-Id: If745b11a8e4b0dda535f114018009085ee2c0ce8 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1269156 Commit-Queue: Robert Phillips <[email protected]> Reviewed-by: Michael Ludwig <[email protected]> --- diff --git a/gn/tests.gni b/gn/tests.gni index da779fd..4307055 100644 --- a/gn/tests.gni +++ b/gn/tests.gni @@ -523,6 +523,7 @@ "$_tests/VkBackendSurfaceTest.cpp", "$_tests/VkWrapTests.cpp", "$_tests/WrappedSurfaceCopyOnWriteTest.cpp", + "$_tests/crbug_500172224.cpp", ] ganesh_gl_tests_sources = [ diff --git a/src/gpu/ganesh/Device_drawTexture.cpp b/src/gpu/ganesh/Device_drawTexture.cpp index 3864216..103e684 100644 --- a/src/gpu/ganesh/Device_drawTexture.cpp +++ b/src/gpu/ganesh/Device_drawTexture.cpp @@ -608,11 +608,11 @@ SkBlendMode mode = paint.getBlendMode_or(SkBlendMode::kSrcOver); AutoTArray<GrTextureSetEntry> textures(count); - // We accumulate compatible proxies until we find an an incompatible one or reach the end and + // We accumulate compatible proxies until we find an incompatible one or reach the end and // issue the accumulated 'n' draws starting at 'base'. 'p' represents the number of proxy // switches that occur within the 'n' entries. int base = 0, n = 0, p = 0; - auto draw = [&](int nextBase) { + auto draw = [&](int nextBase, bool setMayHavePersp) { if (n > 0) { auto textureXform = GrColorSpaceXform::Make(set[base].fImage->imageInfo().colorInfo(), fSurfaceDrawContext->colorInfo()); @@ -625,12 +625,16 @@ mode, constraint, this->localToDevice(), - std::move(textureXform)); + std::move(textureXform), + setMayHavePersp); } base = nextBase; n = 0; p = 0; }; + // This is a conservatively computed property of the image set that disables a fast path + // in TextureOp::AddTextureSetOps. + bool setMayHavePersp = this->localToDevice().hasPerspective(); int dstClipIndex = 0; for (int i = 0; i < count; ++i) { SkASSERT(!set[i].fHasClip || dstClips); @@ -644,7 +648,7 @@ // The default SkDevice implementation is based on drawImageRect which does not allow // non-sorted src rects. TODO: Decide this is OK or make sure we handle it. if (!set[i].fSrcRect.isSorted()) { - draw(i + 1); + draw(i + 1, setMayHavePersp); continue; } @@ -668,7 +672,7 @@ if (!view) { // This image can't go through the texture op, send through general image pipeline // after flushing current batch. - draw(i + 1); + draw(i + 1, setMayHavePersp); SkTCopyOnFirstWrite<SkPaint> entryPaint(paint); if (set[i].fAlpha != 1.f) { auto paintAlpha = paint.getAlphaf(); @@ -689,6 +693,10 @@ textures[i].fDstClipQuad = clip; textures[i].fPreViewMatrix = set[i].fMatrixIndex < 0 ? nullptr : preViewMatrices + set[i].fMatrixIndex; + if (textures[i].fPreViewMatrix && textures[i].fPreViewMatrix->hasPerspective()) { + // Once set, this flag stays on for the rest of the image set + setMayHavePersp = true; + } textures[i].fColor = texture_color(paint.getColor4f(), set[i].fAlpha, SkColorTypeToGrColorType(image->colorType()), fSurfaceDrawContext->colorInfo()); @@ -701,7 +709,7 @@ textures[i].fProxyView.swizzle() != textures[base].fProxyView.swizzle() || set[i].fImage->alphaType() != set[base].fImage->alphaType() || !SkColorSpace::Equals(set[i].fImage->colorSpace(), set[base].fImage->colorSpace()))) { - draw(i); + draw(i, setMayHavePersp); } // Whether or not we submitted a draw in the above if(), this ith entry is in the current // set being accumulated so increment n, and increment p if proxies are different. @@ -712,7 +720,7 @@ ++p; } } - draw(count); + draw(count, setMayHavePersp); } bool Device::drawBlurredRRect(const SkRRect& rrect, const SkPaint& paint, float deviceSigma) { diff --git a/src/gpu/ganesh/SurfaceDrawContext.cpp b/src/gpu/ganesh/SurfaceDrawContext.cpp index 2ec011d..a8339d2 100644 --- a/src/gpu/ganesh/SurfaceDrawContext.cpp +++ b/src/gpu/ganesh/SurfaceDrawContext.cpp @@ -910,7 +910,8 @@ SkBlendMode mode, SkCanvas::SrcRectConstraint constraint, const SkMatrix& viewMatrix, - sk_sp<GrColorSpaceXform> texXform) { + sk_sp<GrColorSpaceXform> texXform, + bool setMayHavePersp) { ASSERT_SINGLE_OWNER RETURN_IF_ABANDONED SkDEBUGCODE(this->validate();) @@ -924,7 +925,7 @@ : ganesh::TextureOp::Saturate::kNo; ganesh::TextureOp::AddTextureSetOps(this, clip, fContext, set, cnt, proxyRunCnt, filter, mm, saturate, mode, aaType, constraint, viewMatrix, - std::move(texXform)); + std::move(texXform), setMayHavePersp); } void SurfaceDrawContext::drawVertices(const GrClip* clip, diff --git a/src/gpu/ganesh/SurfaceDrawContext.h b/src/gpu/ganesh/SurfaceDrawContext.h index 1684687..24e3838 100644 --- a/src/gpu/ganesh/SurfaceDrawContext.h +++ b/src/gpu/ganesh/SurfaceDrawContext.h @@ -323,7 +323,8 @@ SkBlendMode mode, SkCanvas::SrcRectConstraint, const SkMatrix& viewMatrix, - sk_sp<GrColorSpaceXform> texXform); + sk_sp<GrColorSpaceXform> texXform, + bool setMayHavePersp); /** * Draw a roundrect using a paint. diff --git a/src/gpu/ganesh/ops/TextureOp.cpp b/src/gpu/ganesh/ops/TextureOp.cpp index 0c04a67..cf2287e 100644 --- a/src/gpu/ganesh/ops/TextureOp.cpp +++ b/src/gpu/ganesh/ops/TextureOp.cpp @@ -1262,6 +1262,28 @@ , fTextureColorSpaceXform(textureColorSpaceXform) , fNumLeft(numEntries) {} + int determineClumpSize(const GrTextureSetEntry set[], int quadLimit) const { + bool hasPersp = fViewMatrix.hasPerspective(); + + int conservativeNumQuads = 0; + for (int i = 0; i < fNumLeft; ++i) { + int absIndex = this->baseIndex() + i; + + bool hasPrePersp = false; + if (set[absIndex].fPreViewMatrix) { + hasPrePersp = set[absIndex].fPreViewMatrix->hasPerspective(); + } +
Original Bug Report
OOB GPU Memory Read in Skia Ganesh via Perspective Clipping Quad Inflation
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the security team.
Overview: Skia Ganesh’s TextureOp batches up to 512 input quads, but perspective clipping can split these quads, inflating the total count to 1024. Because the operation uses a shared, fixed-size index buffer provisioned for only 512 quads, the GPU is instructed to read indices out-of-bounds. A compromised renderer can potentially exploit this via the RasterInterface to rasterize and exfiltrate out-of-bounds GPU memory into a readable SharedImage.
Affected files:
third_party/skia/src/gpu/ganesh/ops/TextureOp.cppthird_party/skia/src/gpu/ganesh/ops/QuadPerEdgeAA.cppthird_party/skia/src/gpu/ganesh/GrOpsRenderPass.cppcomponents/viz/service/display/skia_renderer.cc
Estimated timestamp from git blame: 2021-06-02
Summary
There is a potential Out-of-Bounds (OOB) memory read vulnerability in the GPU process caused by Skia Ganesh’s handling of perspective-clipped quads. When rendering large tiled images, TextureOp limits the number of input quads to match the capacity of a shared index buffer. However, it fails to account for quad count inflation caused by perspective clipping, leading to a draw call that reads past the end of the index buffer.
Root Cause Analysis
In third_party/skia/src/gpu/ganesh/ops/TextureOp.cpp, the AddTextureSetOps function groups input quads into a TextureOpImpl. For Anti-Aliased (AA) quads, it restricts the maximum number of input quads per batch to GrResourceProvider::MaxNumAAQuads() (512).
During TextureOpImpl construction, appendQuad is called for each quad. This function invokes GrQuadUtils::ClipToW0 to perform perspective clipping. If a quad has exactly one vertex behind the W=0 camera plane, ClipToW0 splits the resulting pentagon into two distinct quads. This inflates the total quad count for the batch from 512 up to 1024.
The op later binds a shared index buffer retrieved via QuadPerEdgeAA::GetIndexBuffer(). This buffer (refAAQuadIndexBuffer()) is statically sized to hold exactly 15,360 indices (512 quads * 30 indices/quad).
Finally, QuadPerEdgeAA::IssueDraw dispatches the draw command using the inflated quad count (1024), calculating the required indices as quadCnt * 30 (30,720 indices). All safety SkASSERT checks are disabled in Release builds. The native graphics API is thus instructed to read 30,720 indices from a 15,360-index buffer, resulting in an out-of-bounds read on the GPU.
Suggested Exploit Path
Note: These are potential steps deduced through static analysis; our tooling agent does not currently have the capability to run code or provide a working Proof of Concept.
A compromised renderer process could theoretically trigger and exploit this issue to exfiltrate cross-origin GPU memory:
- Bypass High-Level Filters: High-level Canvas2D APIs strip perspective from matrices. To bypass this, the attacker manually constructs a
cc::PaintOpBuffer. - Inject Perspective Matrix: The attacker pushes a
cc::SetMatrixOpconfigured with a 3D perspectiveSkM44matrix designed to place exactly one vertex of upcoming quads behind theW=0plane. - Trigger Tiling: The attacker pushes a
cc::DrawImageRectOpreferencing an image larger than the GPU’smax_texture_size(e.g., 16384x16384). - Dispatch via RasterInterface: The renderer sends these ops via
RasterInterface::RasterCHROMIUM. - GPU Execution: In the GPU process, the
RasterDecoderplays back the ops into aSharedImage. The large image triggersSkTiledImageUtils::DrawImageRect, which tiles the image and routes into the vulnerableTextureOp::AddTextureSetOpsbatching logic. - Exfiltration: The GPU hardware reads out-of-bounds indices, causing it to fetch out-of-bounds vertices and rasterize garbage/leaked memory (which may include other origins’ textures or internal handles) into the
SharedImage. The attacker then retrieves the leaked pixels usingRasterInterface::ReadPixels.
Suggested Fix
Modify TextureOp::AddTextureSetOps to account for the possibility of perspective quad inflation. Specifically:
- Limit the input batch size for perspective quads to
MaxNumAAQuads() / 2(256) since each quad can at most be split into two. - Alternatively, dynamically track the post-clipping quad count as quads are appended, and flush/calve off a new
TextureOpImplif appending a split quad would causefMetadata.fTotalQuadCountto exceedMaxNumAAQuads().
Evaluated with Chrome root at commit: f200f57a19490707ff8bc7aa5de3cbc443a3afad
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.