From 1e18aa6d7df79ce36fa7f6b86dc24dc4ffe9a374 Mon Sep 17 00:00:00 2001
From: Florin Malita <fmalita@chromium.org>
Date: Sun, 19 Nov 2017 10:22:22 -0500
Subject: Harden SkTextBlob deserialization

1) validate allocInternal args - these can originate either from users
  or deserialization
2) skip invoking SkTypefaceResolverProc if we failed to read a valid id
  in SkTypefaceResolverReadBuffer::readTypeface
3) validate textSize and buffer sanity in MakeFromBuffer before
  attempting to allocate runs

BUG=chromium:786524

Change-Id: I6cf80dc60bc3ca6fcad7198d36dacf84d091b779
Reviewed-on: https://skia-review.googlesource.com/73521
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
---
 src/core/SkTextBlob.cpp | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

(limited to 'src/core/SkTextBlob.cpp')

diff --git a/src/core/SkTextBlob.cpp b/src/core/SkTextBlob.cpp
index c6325dfbc5..4a029287b0 100644
--- a/src/core/SkTextBlob.cpp
+++ b/src/core/SkTextBlob.cpp
@@ -596,10 +596,13 @@ bool SkTextBlobBuilder::mergeRun(const SkPaint &font, SkTextBlob::GlyphPositioni
 
 void SkTextBlobBuilder::allocInternal(const SkPaint &font,
                                       SkTextBlob::GlyphPositioning positioning,
-                                      int count, int textSize, SkPoint offset, const SkRect* bounds) {
-    SkASSERT(count > 0);
-    SkASSERT(textSize >= 0);
-    SkASSERT(SkPaint::kGlyphID_TextEncoding == font.getTextEncoding());
+                                      int count, int textSize, SkPoint offset,
+                                      const SkRect* bounds) {
+    if (count <= 0 || textSize < 0 || font.getTextEncoding() != SkPaint::kGlyphID_TextEncoding) {
+        fCurrentRunBuffer = { nullptr, nullptr, nullptr, nullptr };
+        return;
+    }
+
     if (textSize != 0 || !this->mergeRun(font, positioning, count, offset)) {
         this->updateDeferredBounds();
 
@@ -772,13 +775,20 @@ sk_sp<SkTextBlob> SkTextBlob::MakeFromBuffer(SkReadBuffer& reader) {
         if (glyphCount <= 0 || pos > kFull_Positioning) {
             return nullptr;
         }
-        uint32_t textSize = pe.extended ? (uint32_t)reader.read32() : 0;
+        int textSize = pe.extended ? reader.read32() : 0;
+        if (textSize < 0) {
+            return nullptr;
+        }
 
         SkPoint offset;
         reader.readPoint(&offset);
         SkPaint font;
         reader.readPaint(&font);
 
+        if (!reader.isValid()) {
+            return nullptr;
+        }
+
         const SkTextBlobBuilder::RunBuffer* buf = nullptr;
         switch (pos) {
             case kDefault_Positioning:
@@ -850,7 +860,8 @@ public:
     {}
 
     sk_sp<SkTypeface> readTypeface() override {
-        return fResolverProc(this->read32(), fResolverCtx);
+        auto id = this->readUInt();
+        return this->isValid() ? fResolverProc(id, fResolverCtx) : nullptr;
     }
 
     SkTypefaceResolverProc  fResolverProc;
-- 
cgit v1.2.3