From f93ed13d77fb28b1ab5c058c10bda3049ccbc5f5 Mon Sep 17 00:00:00 2001
From: Thomas Smith <thomsmit@google.com>
Date: Thu, 28 May 2026 16:48:15 -0400
Subject: [PATCH] Reconstruct subRun bounds from glyphs

* Reconstruct the bounds of a subRun after deserialization instead of packaging onto the VertexFiller.

* An attacker could create a VertexFiller with creation bounds that did not contain its glyphs but were entirely contained within the current clip, enabling to the glyphs to ignore the creation bounds clip and sample from stale scratch textures

Bug: b/513948227
Change-Id: Ib4902657e6a50dd5675db4d73a1576b77c4ce88e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1239916
Commit-Queue: Thomas Smith <thomsmit@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
---

diff --git a/src/text/gpu/GlyphVector.h b/src/text/gpu/GlyphVector.h
index 3543212..327fc22 100644
--- a/src/text/gpu/GlyphVector.h
+++ b/src/text/gpu/GlyphVector.h
@@ -147,6 +147,18 @@
     // the sub runs.
     int unflattenSize() const { return Size(fGlyphs.size()); }
 
+    SkPackedGlyphID getPackedGlyphID(int index) const {
+        SkASSERT(index >= 0 && index < this->glyphCount());
+        if (this->hasBackendData()) {
+            return fGetGlyphID(fGlyphs[index].data());
+        } else {
+            return *reinterpret_cast<const SkPackedGlyphID*>(fGlyphs[index].data());
+        }
+    }
+
+    SkStrikePromise& strikePromise() { return fStrikePromise; }
+    const SkStrikePromise& strikePromise() const { return fStrikePromise; }
+
     bool hasBackendData() const {
         SkASSERT(SkToBool(fBackendDataReleaser) == SkToBool(fGetGlyphID));
         return SkToBool(fBackendDataReleaser);
diff --git a/src/text/gpu/SubRunContainer.cpp b/src/text/gpu/SubRunContainer.cpp
index c59ff2f..bd5f827 100644
--- a/src/text/gpu/SubRunContainer.cpp
+++ b/src/text/gpu/SubRunContainer.cpp
@@ -101,11 +101,89 @@
     return accepted.get<0>();
 }
 
-template <typename U>
-SkSpan<const SkPoint> get_positions(SkZip<U, const SkPoint> accepted) {
+template <typename U> SkSpan<const SkPoint> get_positions(SkZip<U, const SkPoint> accepted) {
     return accepted.template get<1>();
 }
 
+SkGlyphRect glyph_bounds(SkGlyphDigest digest, int inset, SkPoint origin) {
+    return digest.bounds().inset(inset, inset).offset(origin);
+}
+
+SkGlyphRect glyph_bounds_from_left_top(SkGlyphDigest digest, int inset, SkPoint leftTop) {
+    SkGlyphRect bounds = digest.bounds();
+    SkPoint offset = leftTop - bounds.leftTop() - SkPoint{SkIntToScalar(inset),
+                                                          SkIntToScalar(inset)};
+    return glyph_bounds(digest, inset, offset);
+}
+
+// Returns the empty span if there is a problem reading the positions.
+SkSpan<SkPoint> make_points_from_buffer(SkReadBuffer& buffer, SubRunAllocator* alloc) {
+    uint32_t glyphCount = buffer.getArrayCount();
+
+    // Zero indicates a problem with serialization.
+    if (!buffer.validate(glyphCount != 0)) { return {}; }
+
+    // Check that the count will not overflow the arena.
+    if (!buffer.validate(glyphCount <= INT_MAX &&
+                         BagOfBytes::WillCountFit<SkPoint>(glyphCount))) { return {}; }
+
+    SkPoint* positionsData = alloc->makePODArray<SkPoint>(glyphCount);
+    if (!buffer.readPointArray({positionsData, glyphCount})) { return {}; }
+    return {positionsData, glyphCount};
+}
+
+SkRect reconstruct_safe_bounds(SkSpan<const SkPoint> leftTop,
+                               GlyphVector* glyphVector,
+                               skglyph::ActionType actionType,
+                               int inset) {
+    int count = leftTop.size();
+
+    StrikeForGPU* strike = glyphVector->strikePromise().strike();
+    StrikeMutationMonitor m{strike};
+
+    SkGlyphRect boundingRect = skglyph::empty_rect();
+    for (int i = 0; i < count; ++i) {
+        SkPackedGlyphID packedID = glyphVector->getPackedGlyphID(i);
+        SkGlyphDigest digest = strike->digestFor(actionType, packedID);
+        boundingRect = skglyph::rect_union(
+                boundingRect, glyph_bounds_from_left_top(digest, inset, leftTop[i]));
+    }
+    return boundingRect.empty() ? SkRect::MakeEmpty() : boundingRect.rect();
+}
+
+void flatten_vertex_filler(SkWriteBuffer &buffer, const VertexFiller& vf) {
+    buffer.writeInt(static_cast<int>(vf.maskFormat()));
+    buffer.writeBool(vf.canDrawDirect());
+    buffer.writeMatrix(vf.creationMatrix());
+    buffer.writePointArray(vf.topLefts());
+}
+
+std::optional<VertexFiller> make_vertex_filler_from_buffer(SkReadBuffer& buffer,
+                                                           SubRunAllocator* alloc,
+                                                           GlyphVector* glyphVector,
+                                                           skglyph::ActionType actionType,
+                                                           int inset) {
+    MaskFormat maskFormat = buffer.read32LE(MaskFormat::kLast);
+
+    const bool canDrawDirect = buffer.readBool();
+
+    SkMatrix creationMatrix;
+    buffer.readMatrix(&creationMatrix);
+
+    SkSpan<SkPoint> leftTop = make_points_from_buffer(buffer, alloc);
+    if (leftTop.empty()) {
+        return std::nullopt;
+    }
+
+    if (!buffer.validate(glyphVector->glyphCount() == (int)leftTop.size())) {
+        return std::nullopt;
+    }
+
+    SkRect safeBounds = reconstruct_safe_bounds(leftTop, glyphVector, actionType, inset);
+
+    return VertexFiller{maskFormat, creationMatrix, safeBounds, leftTop, canDrawDirect};
+}
+
 // -- PathOpSubmitter ------------------------------------------------------------------------------
 // PathOpSubmitter holds glyph ids until ready to draw. During drawing, the glyph ids are
 // converted to SkPaths. PathOpSubmitter can only be serialized when it is holding glyph ids;
@@ -196,7 +274,7 @@
     SkScalar strikeToSourceScale = buffer.readScalar();
     if (!buffer.validate(0 < strikeToSourceScale)) { return std::nullopt; }
 
-    SkSpan<SkPoint> positions = MakePointsFromBuffer(buffer, alloc);
+    SkSpan<SkPoint> positions = make_points_from_buffer(buffer, alloc);
     if (positions.empty()) { return std::nullopt; }
     const int glyphCount = SkCount(positions);
 
@@ -453,7 +531,7 @@
     SkScalar strikeToSourceScale = buffer.readScalar();
     if (!buffer.validate(0 < strikeToSourceScale)) { return std::nullopt; }
 
-    SkSpan<SkPoint> positions = MakePointsFromBuffer(buffer, alloc);
+    SkSpan<SkPoint> positions = make_points_from_buffer(buffer, alloc);
     if (positions.empty()) { return std::nullopt; }
     const int glyphCount = SkCount(positions);
 
@@ -594,6 +672,8 @@
 // -- DirectMaskSubRun -----------------------------------------------------------------------------
 class DirectMaskSubRun final : public AtlasSubRun {
 public:
+    static constexpr int kGlyphInsetting = 0;
+
     DirectMaskSubRun(VertexFiller&& vertexFiller, GlyphVector&& glyphVector)
             : AtlasSubRun{std::move(vertexFiller), std::move(glyphVector)} {}
 
@@ -619,14 +699,12 @@
     static SubRunOwner MakeFromBuffer(SkReadBuffer& buffer,
                                       SubRunAllocator* alloc,
                                       const SkStrikeClient* client) {
-        auto vertexFiller = VertexFiller::MakeFromBuffer(buffer, alloc);
-        if (!buffer.validate(vertexFiller.has_value())) { return nullptr; }
-
         auto glyphVector = GlyphVector::MakeFromBuffer(buffer, client, alloc);
         if (!buffer.validate(glyphVector.has_value())) { return nullptr; }
-        if (!buffer.validate(glyphVector->glyphCount() == vertexFiller->count())) {
-            return nullptr;
-        }
+
+        auto vertexFiller = make_vertex_filler_from_buffer(buffer, alloc, &glyphVector.value(),
+                                                           skglyph::kDirectMask, kGlyphInsetting);
+        if (!buffer.validate(vertexFiller.has_value())) { return nullptr; }
 
         SkASSERT(buffer.isValid());
         return alloc->makeUnique<DirectMaskSubRun>(std::move(*vertexFiller),
@@ -651,7 +729,7 @@
              + fVertexFiller.unflattenSize();
     }
 
-    int glyphSrcPadding() const override { return 0; }
+    int glyphSrcPadding() const override { return kGlyphInsetting; }
 
     std::tuple<bool, SkRect> deviceRectAndNeedsTransform(
             const SkMatrix &positionMatrix) const override {
@@ -680,14 +758,16 @@
     }
 
     void doFlatten(SkWriteBuffer& buffer) const override {
-        fVertexFiller.flatten(buffer);
         fGlyphVector.flatten(buffer);
+        flatten_vertex_filler(buffer, fVertexFiller);
     }
 };