commit 88c0dee2fcd6095be1383e0b42fd5e5eb56c89b5
author Kevin Lubick <kjlubick@google.com> Thu Sep 29 09:05:31 2022 -0400
committer Kevin Lubick <kjlubick@google.com> Thu Sep 29 16:59:11 2022 +0000
tree 7f0f8c0c4c6a6248c8afca729c91bfcf996758fc
parent ad2aed5c2f56f555108c35beb0fb55f13ca90803

Add more guards to deserializing SkCustomTypeface

A well-defined font shouldn't have glyphs with infinite bounds nor
try to allocate more memory for path data than exists in the stream.

Change-Id: I3fc8ae83958acbb2d1ad28a5fcf63162253c14fd
Bug: oss-fuzz:49770
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/585956
Owners-Override: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>

include/core/SkStream.h

diff --git a/include/core/SkStream.h b/include/core/SkStream.h
index 32dfff2..768de9b 100644
--- a/include/core/SkStream.h
+++ b/include/core/SkStream.h
@@ -122,7 +122,7 @@
     }
 
 //SkStreamSeekable
-    /** Returns true if this stream can report it's current position. */
+    /** Returns true if this stream can report its current position. */
     virtual bool hasPosition() const { return false; }
     /** Returns the current position in the stream. If this cannot be done, returns 0. */
     virtual size_t getPosition() const { return 0; }
@@ -140,7 +140,7 @@
     virtual bool move(long /*offset*/) { return false; }
 
 //SkStreamAsset
-    /** Returns true if this stream can report it's total length. */
+    /** Returns true if this stream can report its total length. */
     virtual bool hasLength() const { return false; }
     /** Returns the total length of the stream. If this cannot be done, returns 0. */
     virtual size_t getLength() const { return 0; }

src/utils/SkCustomTypeface.cpp

diff --git a/src/utils/SkCustomTypeface.cpp b/src/utils/SkCustomTypeface.cpp
index 456d645..79470f0 100644
--- a/src/utils/SkCustomTypeface.cpp
+++ b/src/utils/SkCustomTypeface.cpp
@@ -458,7 +458,8 @@
 
     for (int i = 0; i < glyphCount; ++i) {
         uint32_t gtype;
-        if (!stream->readU32(&gtype)) {
+        if (!stream->readU32(&gtype) ||
+            (gtype != GlyphType::kDrawable && gtype != GlyphType::kPath)) {
             return nullptr;
         }
 
@@ -468,7 +469,7 @@
         }
 
         SkRect bounds;
-        if (stream->read(&bounds, sizeof(bounds)) != sizeof(bounds)) {
+        if (stream->read(&bounds, sizeof(bounds)) != sizeof(bounds) || !bounds.isFinite()) {
             return nullptr;
         }
 
@@ -477,6 +478,17 @@
         if (stream->read(&sz, sizeof(sz)) != sizeof(sz)) {
             return nullptr;
         }
+
+        // The amount of bytes in the stream must be at least as big as sz, otherwise
+        // sz is invalid.
+        if (stream->hasLength() && stream->hasPosition()) {
+            SkASSERT(stream->getLength() >= stream->getPosition());
+            size_t remainingBytes = stream->getLength() - stream->getPosition();
+            if (sz > remainingBytes) {
+                return nullptr;
+            }
+        }
+
         auto data = SkData::MakeUninitialized(sz);
         if (stream->read(data->writable_data(), sz) != sz) {
             return nullptr;