CVE-2026-14410
Overview
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.
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
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 <[email protected]> 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 <[email protected]> Reviewed-by: Michael Ludwig <[email protected]> --- 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); +}