From f1b8ba877c07cccc080451fc4f6b17c27fa76581 Mon Sep 17 00:00:00 2001
From: Kaylee Lubick <kjlubick@google.com>
Date: Wed, 27 May 2026 11:55:53 -0400
Subject: [PATCH] Restrict deserial types further in SkGlyph and
 SkCustomTypeface

Importantly, we don't want typefaces or textblobs in
SkPictureBackedGlyphDrawable. In fact, we can look at
the code used to make the drawables by our various backends
[1][2][3][4] and see we only draw paths and use paints. Thus, we can
restrict deserializing these to just those (and the helper tags).

The SVG fonts could be a bit more complex [5] but chromium doesn't
use those (SkGraphics::SetOpenTypeSVGDecoderFactory is never called)

See also http://graphviz/#57cc93d0c3c73546055bec64fa8e27cb

[1] https://skia.googlesource.com/skia/+/9da67e212e59fbe4f144a92315ca6f8b876c9c01/src/ports/SkFontHost_FreeType_common.cpp#1567
[2] https://skia.googlesource.com/skia/+/9da67e212e59fbe4f144a92315ca6f8b876c9c01/src/ports/SkFontHost_FreeType_common.cpp#1078
[3] https://skia.googlesource.com/skia/+/9da67e212e59fbe4f144a92315ca6f8b876c9c01/src/ports/SkScalerContext_win_dw.cpp#677
[4] https://skia.googlesource.com/skia/+/9da67e212e59fbe4f144a92315ca6f8b876c9c01/src/ports/SkTypeface_fontations.cpp#1336

[4] https://skia.googlesource.com/skia/+/9da67e212e59fbe4f144a92315ca6f8b876c9c01/modules/svg/src/SkSVGOpenTypeSVGDecoder.cpp#160

Bug: https://issues.chromium.org/issues/516457532
Fixed: 516457532
Change-Id: If852aac417e04f5574dd073138abbe74f792734d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1245617
Reviewed-by: Florin Malita <fmalita@google.com>
---
 include/core/SkSerialProcs.h |  7 +++++++
 src/core/SkDebugUtils.h      |  5 +++++
 src/core/SkGlyph.cpp         |  8 ++++++++
 src/core/SkPictureData.cpp   | 15 +++++++++++++++
 src/core/SkPictureData.h     |  2 +-
 src/core/SkReadBuffer.h      |  8 ++++++++
 6 files changed, 44 insertions(+), 1 deletion(-)

--- a/include/core/SkSerialProcs.h
+++ b/include/core/SkSerialProcs.h
@@ -8,6 +8,7 @@
 #ifndef SkSerialProcs_DEFINED
 #define SkSerialProcs_DEFINED
 
+#include "include/core/SkFourByteTag.h"
 #include "include/core/SkRefCnt.h"
 #include "include/private/base/SkAPI.h"
 
@@ -86,6 +87,8 @@
 using SkDeserialTypefaceStreamProc = sk_sp<SkTypeface> (*)(SkStream&, void* ctx);
 using SkDeserialTypefaceProc = sk_sp<SkTypeface> (*)(const void* data, size_t length, void* ctx);
 
+using SkDeserialAllowTagsProc = bool(*)(SkFourByteTag, void* ctx);
+
 struct SK_API SkSerialProcs {
     SkSerialPictureProc fPictureProc = nullptr;
     void*               fPictureCtx = nullptr;
@@ -115,6 +118,10 @@
     // parameters and returns a bool). Given that there are only two valid implementations of that
     // proc, we just insert the bool directly.
     bool                         fAllowSkSL = true;
+
+    // Can be used to restrict the tags that will be deserialized from SkPictures.
+    SkDeserialAllowTagsProc      fAllowTagsProc = nullptr;
+    void*                        fAllowTagsCtx = nullptr;
 };
 
 #endif
--- a/src/core/SkDebugUtils.h
+++ b/src/core/SkDebugUtils.h
@@ -8,6 +8,7 @@
 #ifndef SkDebugUtils_DEFINED
 #define SkDebugUtils_DEFINED
 
+#include "include/core/SkFourByteTag.h"
 #include "include/core/SkTileMode.h"
 #include "include/private/base/SkAssert.h"
 #include "include/private/base/SkDebug.h"
@@ -51,5 +52,9 @@
 inline void SkDumpBuffer(uint8_t*, int, int, int) {}
 #endif
 
+static inline void SkDumpTag(SkFourByteTag tag) {
+    SkDebugf("%c%c%c%c", (char)(tag >> 24), (char)(tag >> 16) & 0xFF, (char)(tag >> 8) & 0xFF, (char)tag & 0xFF);
+}
+
 
 #endif // SkDebugUtils_DEFINED
--- a/src/core/SkGlyph.cpp
+++ b/src/core/SkGlyph.cpp
@@ -19,6 +19,7 @@
 #include "include/private/base/SkTo.h"
 #include "src/base/SkArenaAlloc.h"
 #include "src/base/SkBezierCurves.h"
+#include "src/core/SkPictureData.h"
 #include "src/core/SkReadBuffer.h"
 #include "src/core/SkScalerContext.h"
 #include "src/core/SkWriteBuffer.h"
@@ -48,6 +49,13 @@
     // the deserial procs.
     SkDeserialProcs procs;
     procs.fAllowSkSL = buffer.allowSkSL();
+    procs.fAllowTagsProc = [](SkFourByteTag tag, void*) -> bool {
+        return tag == SK_PICT_BUFFER_SIZE_TAG ||
+               tag == SK_PICT_FACTORY_TAG ||
+               tag == SK_PICT_PAINT_BUFFER_TAG ||
+               tag == SK_PICT_PATH_BUFFER_TAG ||
+               tag == SK_PICT_READER_TAG;
+    };
     sk_sp<SkPicture> picture = SkPicture::MakeFromData(pictureData.get(), &procs);
     if (!buffer.validate(picture != nullptr)) {
         return nullptr;
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -18,6 +18,7 @@
 #include "include/private/base/SkTemplates.h"
 #include "include/private/base/SkTo.h"
 #include "src/base/SkAutoMalloc.h"
+#include "src/core/SkDebugUtils.h"
 #include "src/core/SkPicturePriv.h"
 #include "src/core/SkPictureRecord.h"
 #include "src/core/SkPtrRecorder.h"
@@ -309,6 +310,11 @@
                                    const SkDeserialProcs& procs,
                                    SkTypefacePlayback* topLevelTFPlayback,
                                    int recursionLimit) {
+    if (procs.fAllowTagsProc && !procs.fAllowTagsProc(tag, procs.fAllowTagsCtx)) {
+        // Typefaces are always set but if there's 0 of them, we won't mark the stream as invalid
+        // if the typeface tag is not allowed.
+        return size == 0;
+    }
     switch (tag) {
         case SK_PICT_READER_TAG:
             SkASSERT(nullptr == fOpData);
@@ -449,6 +455,15 @@
 }
 
 void SkPictureData::parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t size) {
+#if defined(SK_DUMP_TAGS)
+    SkDebugf("parseBufferTag ");
+    SkDumpTag(tag);
+    SkDebugf(" size %u\n", size);
+#endif
+    if (!buffer.allowTags(tag)) {
+        buffer.validate(size == 0);
+        return;
+    }
     switch (tag) {
         case SK_PICT_PAINT_BUFFER_TAG: {
             if (!buffer.validate(SkTFitsIn<int>(size))) {
--- a/src/core/SkPictureData.h
+++ b/src/core/SkPictureData.h
@@ -71,7 +71,7 @@
 #define SK_PICT_PAINT_BUFFER_TAG    SkSetFourByteTag('p', 'n', 't', ' ')
 #define SK_PICT_PATH_BUFFER_TAG     SkSetFourByteTag('p', 't', 'h', ' ')
 #define SK_PICT_TEXTBLOB_BUFFER_TAG SkSetFourByteTag('b', 'l', 'o', 'b')
-#define SK_PICT_SLUG_BUFFER_TAG SkSetFourByteTag('s', 'l', 'u', 'g')
+#define SK_PICT_SLUG_BUFFER_TAG     SkSetFourByteTag('s', 'l', 'u', 'g')
 #define SK_PICT_VERTICES_BUFFER_TAG SkSetFourByteTag('v', 'e', 'r', 't')
 #define SK_PICT_IMAGE_BUFFER_TAG    SkSetFourByteTag('i', 'm', 'a', 'g')
 
--- a/src/core/SkReadBuffer.h
+++ b/src/core/SkReadBuffer.h
@@ -11,6 +11,7 @@
 #include "include/core/SkColor.h"
 #include "include/core/SkColorFilter.h"
 #include "include/core/SkFlattenable.h"
+#include "include/core/SkFourByteTag.h"
 #include "include/core/SkImageFilter.h"
 #include "include/core/SkPaint.h"
 #include "include/core/SkPathEffect.h"
@@ -186,6 +187,13 @@
     bool allowSkSL() const { return fAllowSkSL; }
     void setAllowSkSL(bool allow) { fAllowSkSL = allow; }
 
+    bool allowTags(SkFourByteTag tag) const {
+        if (!fProcs.fAllowTagsProc) {
+            return true;
+        }
+        return fProcs.fAllowTagsProc(tag, fProcs.fAllowTagsCtx);
+    }
+
     /**
      *  If isValid is false, sets the buffer to be "invalid". Returns true if the buffer
      *  is still valid.
