From f6a9a592efb842825da1ebacb92493858abfad02 Mon Sep 17 00:00:00 2001
From: Alexis Cruz-Ayala <[email protected]>
Date: Mon, 08 Jun 2026 15:50:12 -0400
Subject: [PATCH] Created an RAII Wrapper for SkCanvas->fScratchGlyphRunBuilder


There was an issue with fScratchGlyphRunBuilder that allowed a malicious drawTextBlob to use an
SkCustomTypeface with a reentrant drawable that contains a drawTextBlob, which caused a UAF. The
first drawTextBlob would start using fScratchGlyphRunBuilder, then the second drawTextBlob would
clear and use fScratchGlyphRunBuilder before the first drawTextBlob would finish.

To resolve this issue, an STArray of GlyphRunBuilders (fScratchGlyphRunBuilders) is used with 1
GlyphRunBuilder by default, with more allocated as needed. To verify the behavior is consistent and
make the maintenance easier for the user, an RAII pattern was used (AutoGlyphRunBuilder).

A benchmark was added to show the performance change on RSXform Glyph Runs within a text blob, and a
test was added to verify the UAF bug was fixed.

# Without Changes:
alexisdavidc@alexisdavidc2:~/skia/skia (testingbefore)$ ninja -C out/Release/ &&  out/Release/nanobench --match GlyphRunRSXform
ninja: Entering directory `out/Release/'
ninja: no work to do.
Timer overhead: 23.9ns
curr/maxrss     loops   min     median  mean    max     stddev  samples         config  bench
 123/122 MB     1       29.2µs  29.8µs  29.7µs  30.1µs  1%      █▅▁▃▆▇▄▇▄▅      8888    GlyphRunRSXform_cached
 147/140 MB     215     22.7µs  22.9µs  22.9µs  23.1µs  1%      █▄▁▁▄▂▃▆█▃      gl      GlyphRunRSXform_cached
alexisdavidc@alexisdavidc2:~/skia/skia (testingbefore)$ ninja -C out/Release/ &&  out/Release/nanobench --match GlyphRunRSXform
ninja: Entering directory `out/Release/'
ninja: no work to do.
Timer overhead: 23.8ns
curr/maxrss     loops   min     median  mean    max     stddev  samples         config  bench
 123/121 MB     1       29.7µs  30µs    29.9µs  30.2µs  0%      ▄█▃▃▁▅▆▅▄▅      8888    GlyphRunRSXform_cached
 147/139 MB     219     22.8µs  23.5µs  23.6µs  25.8µs  4%      ▄█▁▄▆▁▁▃▁▁      gl      GlyphRunRSXform_cached


# With Changes:
alexisdavidc@alexisdavidc2:~/skia/skia (security)$ ninja -C out/Release/ &&  out/Release/nanobench --match GlyphRunRSXform
ninja: Entering directory `out/Release/'
ninja: no work to do.
Timer overhead: 23.8ns
curr/maxrss     loops   min     median  mean    max     stddev  samples         config  bench
 123/119 MB     1       29.6µs  29.9µs  29.9µs  30.4µs  1%      ▇▄█▁▃▄▃▁▆▂      8888    GlyphRunRSXform_cached
 147/138 MB     204     22.7µs  22.9µs  23.4µs  24.9µs  4%      ▁█▂▁██▂▂▁▂      gl      GlyphRunRSXform_cached
alexisdavidc@alexisdavidc2:~/skia/skia (security)$ ninja -C out/Release/ &&  out/Release/nanobench --match GlyphRunRSXform
ninja: Entering directory `out/Release/'
ninja: no work to do.
Timer overhead: 23.8ns
curr/maxrss     loops   min     median  mean    max     stddev  samples         config  bench
 123/116 MB     1       29.3µs  29.7µs  29.7µs  30.3µs  1%      ▄█▄▂▃▂▃▁▃▄      8888    GlyphRunRSXform_cached
 147/134 MB     221     22.5µs  22.7µs  22.7µs  22.9µs  0%      ▃▂▄█▄▄▄▁▂▃      gl      GlyphRunRSXform_cached


Bug: b/513820666
Change-Id: If8e5e9a3dc0f656eb1fc6b03ae79fdf39e67328e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1248477
Commit-Queue: Alexis Cruz-Ayala <[email protected]>
Reviewed-by: Kaylee Lubick <[email protected]>
---

diff --git a/bench/GlyphRunRSXformBench.cpp b/bench/GlyphRunRSXformBench.cpp
new file mode 100644
index 0000000..4bf7bcc
--- /dev/null
+++ b/bench/GlyphRunRSXformBench.cpp
@@ -0,0 +1,71 @@
+/*
+ * 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 "bench/Benchmark.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkFont.h"
+#include "include/core/SkPaint.h"
+#include "include/core/SkRSXform.h"
+#include "include/core/SkString.h"
+#include "include/core/SkTextBlob.h"
+#include "include/private/base/SkTDArray.h"
+#include "src/base/SkRandom.h"
+#include "tools/fonts/FontToolUtils.h"
+
+// Creates an RSX form blob from the the randomized fXforms
+sk_sp<SkTextBlob> makeBlob() {
+    SkFont font;
+    SkTDArray<SkGlyphID> glyphs;
+    SkTDArray<SkRSXform> xForms;
+    font = ToolUtils::DefaultFont();
+    font.setSubpixel(true);
+    font.setSize(20);
+
+    const char* text = "Keep your sentences short, but not overly so.";
+    int glyphCount = font.countText(text, strlen(text), SkTextEncoding::kUTF8);
+    glyphs.append(glyphCount);
+    font.textToGlyphs(
+            text, strlen(text), SkTextEncoding::kUTF8, {glyphs.data(), (size_t)glyphCount});
+
+    xForms.append(glyphCount);
+    SkRandom rand;
+    SkScalar x = 0;
+    for (int i = 0; i < glyphCount; ++i) {
+        SkScalar s = rand.nextF() * 0.5f + 0.5f;
+        SkScalar a = rand.nextF() * SK_ScalarPI * 0.25f;
+        xForms[i] = SkRSXform::Make(s * SkScalarCos(a), s * SkScalarSin(a), x, rand.nextF() * 20);
+        x += 20;
+    }
+
+    return SkTextBlob::MakeFromRSXform(glyphs.data(),
+                                       glyphs.size() * sizeof(SkGlyphID),
+                                       {xForms.data(), (size_t)xForms.size()},
+                                       font,
+                                       SkTextEncoding::kGlyphID);
+}
+
+class GlyphRunRSXformCachedBench : public Benchmark {
+public:
+    GlyphRunRSXformCachedBench() {}
+
+protected:
+    const char* onGetName() override { return "GlyphRunRSXform_cached"; }
+
+    void onDelayedSetup() override { fBlob = makeBlob(); }
+
+    void onDraw(int loops, SkCanvas* canvas) override {
+        SkPaint paint;
+        for (int i = 0; i < loops; i++) {
+            canvas->drawTextBlob(fBlob, 0, 0, paint);
+        }
+    }
+
+private:
+    sk_sp<SkTextBlob> fBlob;
+};
+
+DEF_BENCH(return new GlyphRunRSXformCachedBench();)
diff --git a/gn/bench.gni b/gn/bench.gni
index 0f95208..0cb5cca 100644
--- a/gn/bench.gni
+++ b/gn/bench.gni
@@ -60,6 +60,7 @@
   "$_bench/GameBench.cpp",
   "$_bench/GeometryBench.cpp",
   "$_bench/GlyphQuadFillBench.cpp",
+  "$_bench/GlyphRunRSXformBench.cpp",
   "$_bench/GrMemoryPoolBench.cpp",
   "$_bench/GrMipmapBench.cpp",
   "$_bench/GrQuadBench.cpp",
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index cf31ffa..24702c8 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -45,11 +45,6 @@
 #define SK_SUPPORT_LEGACY_GETTOTALMATRIX
 #endif
 
-namespace sktext {
-class GlyphRunBuilder;
-class GlyphRunList;
-}
-
 class AutoLayerForImageFilter;
 class GrRecordingContext;
 
@@ -81,6 +76,12 @@
 template<typename E>
 class SkEnumBitMask;
 
+namespace sktext {
+class GlyphRunBuilder;
+class GlyphRun;
+class GlyphRunList;
+}  // namespace sktext
+
 namespace skgpu::graphite { class Recorder; }
 namespace sktext::gpu { class Slug; }
 namespace SkRecords { class Draw; }
@@ -2559,6 +2560,7 @@
     friend class SkRecords::Draw;
     template <typename Key>
     friend class skiatest::TestCanvas;
+    friend class AutoGlyphRunBuilder;
 
 protected:
     // For use by SkNoDrawCanvas (via SkCanvasVirtualEnforcer, which can't be a friend)
@@ -2704,7 +2706,15 @@
     class AutoUpdateQRBounds;
     void validateClip() const;
 
-    std::unique_ptr<sktext::GlyphRunBuilder> fScratchGlyphRunBuilder;
+    sktext::GlyphRunBuilder* obtainGlyphRunBuilder();
+
+    void releaseGlyphRunBuilder();
+
+    /**
+     * fRunBuilders will be reused across text drawing commands
+     */
+    skia_private::STArray<1, std::unique_ptr<sktext::GlyphRunBuilder>> fRunBuilders;
+    int fRunBuildersUsed = 0;
 };