Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactOut of bounds read in Skia
DescriptionOut of bounds read in Skia
ComponentSkia
Bug ClassOOB
Tracker500172224
Fix commitb6c7a5bc832b (skia) +205/-24
CISA KEVNot listed
CreditedGoogle
Disclosed2026-07-29

Changed Functions

FunctionChangeNotes
if
src/gpu/ganesh/Device_drawTexture.cpp
modified
for
src/gpu/ganesh/Device_drawTexture.cpp
modified
for
src/gpu/ganesh/ops/TextureOp.cpp
modified
if
src/gpu/ganesh/ops/TextureOp.cpp
modified

Files Changed

  • gn/tests.gni
  • src/gpu/ganesh/Device_drawTexture.cpp
  • src/gpu/ganesh/SurfaceDrawContext.cpp
  • src/gpu/ganesh/SurfaceDrawContext.h
  • src/gpu/ganesh/ops/TextureOp.cpp
From b6c7a5bc832b785a445ab89b3365ed80ab2e9d47 Mon Sep 17 00:00:00 2001
From: Robert Phillips <robertphillips@google.com>
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 <robertphillips@google.com>
> > Reviewed-by: Michael Ludwig <michaelludwig@google.com>
>
> 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: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: Robert Phillips <robertphillips@google.com>
> Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>

Bug: b/500172224
Change-Id: If745b11a8e4b0dda535f114018009085ee2c0ce8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1269156
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
---

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();
+            }
+
Loading diff…

Original Bug Report

reported by vm...@google.com

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.cpp
  • third_party/skia/src/gpu/ganesh/ops/QuadPerEdgeAA.cpp
  • third_party/skia/src/gpu/ganesh/GrOpsRenderPass.cpp
  • components/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:

  1. Bypass High-Level Filters: High-level Canvas2D APIs strip perspective from matrices. To bypass this, the attacker manually constructs a cc::PaintOpBuffer.
  2. Inject Perspective Matrix: The attacker pushes a cc::SetMatrixOp configured with a 3D perspective SkM44 matrix designed to place exactly one vertex of upcoming quads behind the W=0 plane.
  3. Trigger Tiling: The attacker pushes a cc::DrawImageRectOp referencing an image larger than the GPU’s max_texture_size (e.g., 16384x16384).
  4. Dispatch via RasterInterface: The renderer sends these ops via RasterInterface::RasterCHROMIUM.
  5. GPU Execution: In the GPU process, the RasterDecoder plays back the ops into a SharedImage. The large image triggers SkTiledImageUtils::DrawImageRect, which tiles the image and routes into the vulnerable TextureOp::AddTextureSetOps batching logic.
  6. 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 using RasterInterface::ReadPixels.

Suggested Fix

Modify TextureOp::AddTextureSetOps to account for the possibility of perspective quad inflation. Specifically:

  1. Limit the input batch size for perspective quads to MaxNumAAQuads() / 2 (256) since each quad can at most be split into two.
  2. Alternatively, dynamically track the post-clipping quad count as quads are appended, and flush/calve off a new TextureOpImpl if appending a split quad would cause fMetadata.fTotalQuadCount to exceed MaxNumAAQuads().

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.

View on issue tracker