Chrome · Skia › Graphite
CVE-2026-14410
Logic Error in Skia (Graphite)
Overview
Low
Severity
4.3
CVSS
No
Exploited ITW
Fixed
Fix Status
Background
- `drawEdgeAAImageSet`
- the Graphite
Deviceroutine that renders a batch of images (anImageSetEntryarray) as edge-antialiased quads in one call, iterating over eachset[i]entry. - `dstClips` / `dstClipIndex`
dstClipsis a flat, caller-supplied array ofSkPoints holding 4 clip corners per clipped entry, anddstClipIndexis the running cursor that tells the loop which 4 points belong to the current entry.- `fHasClip`
- the per-entry boolean flag indicating whether
set[i]carries a 4-point clip quad indstClips, so thatdstClipIndexmust advance by 4 for that entry. - `dstToDraw`
- the clipped destination rectangle computed for the current entry, which can become empty (
isEmpty()) when the source rectangle falls outside the image bounds so nothing is visible.
Root Cause Analysis
The loop in drawEdgeAAImageSet advanced the shared clip cursor with dstClipIndex += 4 * set[i].fHasClip; only at the very end of each iteration. When an entry produced an empty dstToDraw, the original code executed continue, which skipped the rest of the iteration — including that dstClipIndex advance — even though a skipped-but-clipped entry still owns 4 points in the dstClips array. This desynchronized the cursor: every later clipped entry then read its clip corners from the wrong (earlier) offset in dstClips, so images were rendered against another entry’s clip quad.
The fix removes the early continue and instead wraps the draw work in if (!dstToDraw.isEmpty()) { ... }, so control always falls through to dstClipIndex += 4 * set[i].fHasClip; and the cursor stays aligned with fHasClip regardless of whether the entry actually drew.
Key insight
The single mistake was tying a mandatory, cursor-advancing side effect (
dstClipIndex += 4 * set[i].fHasClip) to a continue that could skip it for empty entries; the fix converts the early-out continue into a positive if (!dstToDraw.isEmpty()) guard so the clip cursor is advanced unconditionally.Attack Path
- Craft a multi-entry image set
A page drives a canvas so the compositor issues
experimental_DrawEdgeAAImageSetwith severalImageSetEntryitems, each carryingfHasClip = trueand its own 4-point quad indstClips. - Force an empty entry first
Give an early entry a
fSrcRectthat lies outside the image (e.g.SkRect::MakeLTRB(-100, -100, -50, -50)) so itsdstToDrawbecomes empty and the buggycontinuefires. - Skip the cursor advance
Because
continuebypassesdstClipIndex += 4 * set[i].fHasClip, the cursor is left 4 points short even though the empty entry owned a clip quad. - Misclip subsequent entries
Later clipped entries read
dstClips + dstClipIndexfrom the wrong offset, so their image content is drawn against a different entry’s clip region than the client specified, producing incorrect rendered output (the test observes the right-hand pixel rendered wrong). - Escalation
Any read/write primitive, sandbox escape, or code execution beyond this clip-region desynchronization is not demonstrated by the diff
(inferred).
Impact Assessment
An attacker who controls canvas draw commands gains the ability to make Skia’s Graphite backend render image content clipped to the wrong region, an inappropriate-implementation / logic error in the GPU process rendering path; the precondition is a page (or compositor client) able to submit an edge-AA image set where an earlier clipped entry is culled to an empty
dstToDraw. The observable effect proven by the patch is incorrect clipping/rendering rather than memory corruption, so the array under-advances and stays in-bounds. The advisory states the bug class as LogicError with impact “Inappropriate implementation in Skia” and does not report exploitation in the wild; any escalation to a memory-safety primitive is (inferred) and not established by the diff.Changed Functions
| Function | Change | Notes |
|---|---|---|
DEF_GRAPHITE_TEST_FOR_ALL_CONTEXTStests/graphite/crbug_513836996.cpp |
modified |
Files Changed
gn/tests.gnisrc/gpu/graphite/Device.cpptests/graphite/crbug_513836996.cpp
Audit Directions
- Cursor-advance under `continue`Audit loops where an index or offset into a caller-supplied parallel array is advanced at the tail of an iteration, and check that no early
continue/break/returncan skip that advance for an entry that still consumes elements. - Empty/degenerate geometry short-circuitsReview paths that early-out on empty rects, zero sizes, or fully-clipped geometry, ensuring per-entry bookkeeping (clip indices, matrix indices, alpha cursors) is still updated when the visible work is skipped.
- Batched draw APIs with side arraysInspect other batch entrypoints such as edge-AA quad and image-set drawing that stride through flat arrays (
dstClips,preViewMatrices) using per-entry flags likefHasCliporfMatrixIndex, verifying the stride stays synchronized across all control-flow branches.
Patch
From 17b4bab488449ed4aaca0a406bc950dcc88fa357 Mon Sep 17 00:00:00 2001
From: Thomas Smith <thomsmit@google.com>
Date: Tue, 19 May 2026 15:33:06 -0400
Subject: [PATCH] [graphite] correctly advance index in drawEdgeAAImageSet
Bug: b/513836996
Change-Id: Iff633e0d99443cfc431352cea05051ff567b8f64
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1238179
Reviewed-by: Kaylee Lubick <kjlubick@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
---
diff --git a/gn/tests.gni b/gn/tests.gni
index f4a4605..3430e2d 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -336,6 +336,7 @@
"$_tests/graphite/UpdateBackendTextureTest.cpp",
"$_tests/graphite/UploadBufferManagerTest.cpp",
"$_tests/graphite/VerticesPaddingTest.cpp",
+ "$_tests/graphite/crbug_513836996.cpp",
]
precompile_tests_sources = [
diff --git a/src/gpu/graphite/Device.cpp b/src/gpu/graphite/Device.cpp
index 218c763..c75b096 100644
--- a/src/gpu/graphite/Device.cpp
+++ b/src/gpu/graphite/Device.cpp
@@ -1309,32 +1309,31 @@
dstToDraw.setEmpty();
}
}
- if (dstToDraw.isEmpty()) {
- continue; // Nothing to draw for this set entry
+ if (!dstToDraw.isEmpty()) {
+ PaintParams::SimpleImage imageShader{
+ set[i].fImage.get(),
+ &localMatrix,
+ constraint == SkCanvas::kStrict_SrcRectConstraint ? subset : imageBounds,
+ sampling};
+
+ // NOTE: See drawEdgeAAQuad for details, we do not snap non-AA quads.
+ SkEnumBitMask<EdgeAAQuad::Flags> flags =
+ static_cast<EdgeAAQuad::Flags>(set[i].fAAFlags);
+ EdgeAAQuad quad = set[i].fHasClip ? EdgeAAQuad(dstClips + dstClipIndex, flags)
+ : EdgeAAQuad(dstToDraw, flags);
+
+ // TODO: Calling drawGeometry() for each entry re-evaluates the clip stack every time,
+ // which is consistent with Ganesh's behavior. It also matches the behavior if edge-AA
+ // images were submitted one at a time by SkiaRenderer (a nice client simplification).
+ // However, we should explore the performance trade off with doing one bulk evaluation
+ // for the whole set
+ const SkMatrix* xtraXform =
+ set[i].fMatrixIndex < 0 ? nullptr : &preViewMatrices[set[i].fMatrixIndex];
+ this->drawGeometry(xtraXform ? localToDevice.concat(SkM44(*xtraXform)) : localToDevice,
+ Geometry(quad),
+ PaintParams(paint, imageShader, set[i].fAlpha),
+ DefaultFillStyle());
}
-
- PaintParams::SimpleImage imageShader{set[i].fImage.get(),
- &localMatrix,
- constraint == SkCanvas::kStrict_SrcRectConstraint ?
- subset : imageBounds,
- sampling};
-
- // NOTE: See drawEdgeAAQuad for details, we do not snap non-AA quads.
- SkEnumBitMask<EdgeAAQuad::Flags> flags = static_cast<EdgeAAQuad::Flags>(set[i].fAAFlags);
- EdgeAAQuad quad = set[i].fHasClip ? EdgeAAQuad(dstClips + dstClipIndex, flags)
- : EdgeAAQuad(dstToDraw, flags);
-
- // TODO: Calling drawGeometry() for each entry re-evaluates the clip stack every time, which
- // is consistent with Ganesh's behavior. It also matches the behavior if edge-AA images were
- // submitted one at a time by SkiaRenderer (a nice client simplification). However, we
- // should explore the performance trade off with doing one bulk evaluation for the whole set
- const SkMatrix* xtraXform = set[i].fMatrixIndex < 0 ? nullptr
- : &preViewMatrices[set[i].fMatrixIndex];
- this->drawGeometry(xtraXform ? localToDevice.concat(SkM44(*xtraXform)) : localToDevice,
- Geometry(quad),
- PaintParams(paint, imageShader, set[i].fAlpha),
- DefaultFillStyle());
-
dstClipIndex += 4 * set[i].fHasClip;
}
}
diff --git a/tests/graphite/crbug_513836996.cpp b/tests/graphite/crbug_513836996.cpp
new file mode 100644
index 0000000..436468f
--- /dev/null
+++ b/tests/graphite/crbug_513836996.cpp
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2026 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/core/SkBitmap.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkColor.h"
+#include "include/core/SkImageInfo.h"
+#include "include/core/SkPoint.h"
+#include "include/core/SkRect.h"
+#include "include/core/SkRefCnt.h"
+#include "include/core/SkSamplingOptions.h"
+#include "include/core/SkSurface.h"
+#include "include/gpu/graphite/Context.h"
+#include "include/gpu/graphite/Image.h"
+#include "include/gpu/graphite/Recorder.h"
+#include "include/gpu/graphite/Recording.h"
+#include "include/gpu/graphite/Surface.h"
+#include "tests/Test.h"
+
+using namespace skgpu::graphite;
+
+DEF_GRAPHITE_TEST_FOR_ALL_CONTEXTS(crbug_513836996, reporter, context, CtsEnforcement::kNever) {
+ constexpr int kW = 200;
+ constexpr int kH = 100;
+ const SkImageInfo ii = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+
+ std::unique_ptr<Recorder> recorder = context->makeRecorder();
+ sk_sp<SkSurface> surface = SkSurfaces::RenderTarget(recorder.get(), ii);
+ SkCanvas* canvas = surface->getCanvas();
+ canvas->clear(SK_ColorBLACK);
+
+ auto src_ii = SkImageInfo::Make(32, 32, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+ SkBitmap src_bm;
+ src_bm.allocPixels(src_ii);
+ src_bm.eraseColor(SK_ColorGREEN);
+
+ sk_sp<SkImage> rasterImage = SkImages::RasterFromBitmap(src_bm);
+ sk_sp<SkImage> green = SkImages::TextureFromImage(recorder.get(), rasterImage);
+
+ const SkPoint dstClips[8] = {
+ {10, 10}, {60, 10}, {60, 60}, {10, 60}, // LEFT
+ {110, 10}, {190, 10}, {190, 90}, {110, 90}, // RIGHT
+ };
+
+ SkCanvas::ImageSetEntry entries[2];
+ entries[0].fImage = green;
+ entries[0].fSrcRect = SkRect::MakeLTRB(-100, -100, -50, -50);
+ entries[0].fDstRect = SkRect::MakeWH(kW, kH);
+ entries[0].fMatrixIndex = -1;
+ entries[0].fAlpha = 1.f;
+ entries[0].fAAFlags = SkCanvas::kNone_QuadAAFlags;
+ entries[0].fHasClip = true;
+
+ entries[1].fImage = green;
+ entries[1].fSrcRect = SkRect::MakeWH(32, 32);
+ entries[1].fDstRect = SkRect::MakeWH(kW, kH);
+ entries[1].fMatrixIndex = -1;
+ entries[1].fAlpha = 1.f;
+ entries[1].fAAFlags = SkCanvas::kNone_QuadAAFlags;
+ entries[1].fHasClip = true;
+
+ canvas->experimental_DrawEdgeAAImageSet(
+ entries, 2, dstClips, nullptr, SkSamplingOptions(),
+ nullptr, SkCanvas::kFast_SrcRectConstraint);
+
+ std::unique_ptr<Recording> recording = recorder->snap();
+ context->insertRecording({recording.get()});
+
+ SkBitmap bm;
+ bm.allocPixels(ii);
+ REPORTER_ASSERT(reporter, surface->readPixels(bm, 0, 0), "Failed to read pixels");
+
+ SkColor left_px = bm.getColor(35, 35);
+ SkColor right_px = bm.getColor(150, 50);
+
+ REPORTER_ASSERT(reporter, left_px == SK_ColorBLACK,
+ "Left pixel should be BLACK, got 0x%08x", left_px);
+ REPORTER_ASSERT(reporter, right_px == SK_ColorGREEN,
+ "Right pixel should be GREEN, got 0x%08x", right_px);
+}
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page