validate text during deserialization

Bug: 796473
Change-Id: I7b6a6c698a5b53c915ef6564852fa51ce7410a3e
Reviewed-on: https://skia-review.googlesource.com/88520
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>

9 files changed, 109 insertions, 55 deletions

diff --git a/src/core/SkFont.cpp b/src/core/SkFont.cpp
index aa7fe366c8..ca6cdb13b0 100644
--- a/src/core/SkFont.cpp
+++ b/src/core/SkFont.cpp
@@ -69,7 +69,7 @@ int SkFont::textToGlyphs(const void* text, size_t byteLength, SkTextEncoding enc
             count = SkUTF8_CountUnichars((const char*)text, byteLength);
             break;
         case kUTF16_SkTextEncoding:
-            count = SkUTF16_CountUnichars((const uint16_t*)text, SkToInt(byteLength >> 1));
+            count = SkUTF16_CountUnichars((const uint16_t*)text, byteLength);
             break;
         case kUTF32_SkTextEncoding:
             count = SkToInt(byteLength >> 2);
diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp
index 751bcedcf6..f8e5c11efd 100644
--- a/src/core/SkPaint.cpp
+++ b/src/core/SkPaint.cpp
@@ -420,9 +420,9 @@ int SkPaint::textToGlyphs(const void* textData, size_t byteLength, uint16_t glyp
     if (nullptr == glyphs) {
         switch (this->getTextEncoding()) {
         case kUTF8_TextEncoding:
-            return SkUTF8_CountUnichars((const char*)textData, byteLength);
+            return SkUTF8_CountUnichars(textData, byteLength);
         case kUTF16_TextEncoding:
-            return SkUTF16_CountUnichars((const uint16_t*)textData, SkToInt(byteLength >> 1));
+            return SkUTF16_CountUnichars(textData, byteLength);
         case kUTF32_TextEncoding:
             return SkToInt(byteLength >> 2);
         case kGlyphID_TextEncoding:
diff --git a/src/core/SkPaintPriv.cpp b/src/core/SkPaintPriv.cpp
index 7d771f88a6..ad361a23c1 100644
--- a/src/core/SkPaintPriv.cpp
+++ b/src/core/SkPaintPriv.cpp
@@ -11,6 +11,7 @@
 #include "SkImage.h"
 #include "SkPaint.h"
 #include "SkShaderBase.h"
+#include "SkUtils.h"
 #include "SkXfermodePriv.h"
 
 static bool changes_alpha(const SkPaint& paint) {
@@ -84,3 +85,21 @@ bool SkPaintPriv::ShouldDither(const SkPaint& p, SkColorType dstCT) {
     return p.getImageFilter() || p.getMaskFilter()
         || !p.getShader() || !as_SB(p.getShader())->isConstant();
 }
+
+int SkPaintPriv::ValidCountText(const void* text, size_t length, SkPaint::TextEncoding encoding) {
+    if (length == 0) {
+        return 0;
+    }
+    switch (encoding) {
+        case SkPaint::kUTF8_TextEncoding: return SkUTF8_CountUnichars(text, length);
+        case SkPaint::kUTF16_TextEncoding: return SkUTF16_CountUnichars(text, length);
+        case SkPaint::kUTF32_TextEncoding: return SkUTF32_CountUnichars(text, length);
+        case SkPaint::kGlyphID_TextEncoding:
+            if (SkIsAlign2(intptr_t(text)) && SkIsAlign2(length)) {
+                return length >> 1;
+            }
+            break;
+    }
+    return 0;
+}
+
diff --git a/src/core/SkPaintPriv.h b/src/core/SkPaintPriv.h
index 72c9274859..08f80bf93a 100644
--- a/src/core/SkPaintPriv.h
+++ b/src/core/SkPaintPriv.h
@@ -64,6 +64,9 @@ public:
     }
 
     static bool ShouldDither(const SkPaint&, SkColorType);
+
+    // returns 0 if buffer is invalid for specified encoding
+    static int ValidCountText(const void* text, size_t length, SkPaint::TextEncoding);
 };
 
 #endif
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 9a9aed3192..a2260952d7 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -7,6 +7,7 @@
 
 #include "SkCanvas.h"
 #include "SkDrawShadowInfo.h"
+#include "SkPaintPriv.h"
 #include "SkPatchUtils.h"
 #include "SkPictureData.h"
 #include "SkPicturePlayback.h"
@@ -70,20 +71,28 @@ static const SkRect* get_rect_ptr(SkReadBuffer* reader, SkRect* storage) {
 
 class TextContainer {
 public:
-    size_t length() { return fByteLength; }
-    const void* text() { return (const void*)fText; }
-    size_t fByteLength;
-    const char* fText;
-
-    bool validate(SkReadBuffer* reader, int count, const SkPaint& paint) const {
-        return reader->validate(paint.countText(fText, fByteLength) == count);
+    TextContainer(SkReadBuffer* reader, const SkPaint* paint) {
+        if (reader->validate(paint != nullptr)) {
+            fByteLength = reader->readInt();
+            fText = (const char*)reader->skip(fByteLength);
+            if (reader->isValid()) {
+                fCount = SkPaintPriv::ValidCountText(fText, fByteLength, paint->getTextEncoding());
+                reader->validate(fCount >= 0);
+            }
+        }
     }
-};
 
-void get_text(SkReadBuffer* reader, TextContainer* text) {
-    size_t length = text->fByteLength = reader->readInt();
-    text->fText = (const char*)reader->skip(length);
-}
+    operator bool() const { return fCount >= 0; }
+
+    size_t length() const { return fByteLength; }
+    const void* text() const { return (const void*)fText; }
+    unsigned count() const { return fCount; }
+
+private:
+    size_t fByteLength = 0;
+    const char* fText = nullptr;
+    int fCount = -1;
+};
 
 void SkPicturePlayback::draw(SkCanvas* canvas,
                              SkPicture::AbortCallback* callback,
@@ -480,10 +489,9 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_POS_TEXT: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             size_t points = reader->readInt();
-            text.validate(reader, points, *paint);
+            reader->validate(points == text.count());
             const SkPoint* pos = (const SkPoint*)reader->skip(points * sizeof(SkPoint));
             BREAK_ON_READ_ERROR(reader);
 
@@ -493,10 +501,9 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_POS_TEXT_TOP_BOTTOM: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             size_t points = reader->readInt();
-            text.validate(reader, points, *paint);
+            reader->validate(points == text.count());
             const SkPoint* pos = (const SkPoint*)reader->skip(points * sizeof(SkPoint));
             const SkScalar top = reader->readScalar();
             const SkScalar bottom = reader->readScalar();
@@ -510,10 +517,9 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_POS_TEXT_H: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             size_t xCount = reader->readInt();
-            text.validate(reader, xCount, *paint);
+            reader->validate(xCount == text.count());
             const SkScalar constY = reader->readScalar();
             const SkScalar* xpos = (const SkScalar*)reader->skip(xCount * sizeof(SkScalar));
             BREAK_ON_READ_ERROR(reader);
@@ -524,10 +530,9 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_POS_TEXT_H_TOP_BOTTOM: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             size_t xCount = reader->readInt();
-            text.validate(reader, xCount, *paint);
+            reader->validate(xCount == text.count());
             const SkScalar* xpos = (const SkScalar*)reader->skip((3 + xCount) * sizeof(SkScalar));
             BREAK_ON_READ_ERROR(reader);
 
@@ -592,8 +597,7 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_TEXT: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             SkScalar x = reader->readScalar();
             SkScalar y = reader->readScalar();
             BREAK_ON_READ_ERROR(reader);
@@ -615,8 +619,7 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_TEXT_TOP_BOTTOM: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             const SkScalar* ptr = (const SkScalar*)reader->skip(4 * sizeof(SkScalar));
             BREAK_ON_READ_ERROR(reader);
 
@@ -633,8 +636,7 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         } break;
         case DRAW_TEXT_ON_PATH: {
             const SkPaint* paint = fPictureData->getPaint(reader);
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             const SkPath& path = fPictureData->getPath(reader);
             SkMatrix matrix;
             reader->readMatrix(&matrix);
@@ -648,8 +650,7 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkPaint* paint = fPictureData->getPaint(reader);
             int count = reader->readInt();
             uint32_t flags = reader->read32();
-            TextContainer text;
-            get_text(reader, &text);
+            TextContainer text(reader, paint);
             const SkRSXform* xform = (const SkRSXform*)reader->skip(count * sizeof(SkRSXform));
             const SkRect* cull = nullptr;
             if (flags & DRAW_TEXT_RSXFORM_HAS_CULL) {
diff --git a/src/core/SkUtils.cpp b/src/core/SkUtils.cpp
index 1eb2a7df43..6eceef52c1 100644
--- a/src/core/SkUtils.cpp
+++ b/src/core/SkUtils.cpp
@@ -82,8 +82,12 @@ int SkUTF8_CountUnichars(const char utf8[]) {
 }
 
 // SAFE: returns -1 if invalid UTF-8
-int SkUTF8_CountUnicharsWithError(const char utf8[], size_t byteLength) {
-    SkASSERT(utf8 || 0 == byteLength);
+int SkUTF8_CountUnichars(const void* text, size_t byteLength) {
+    SkASSERT(text);
+    const char* utf8 = static_cast<const char*>(text);
+    if (byteLength == 0) {
+        return 0;
+    }
 
     int         count = 0;
     const char* stop = utf8 + byteLength;
@@ -91,8 +95,8 @@ int SkUTF8_CountUnicharsWithError(const char utf8[], size_t byteLength) {
     while (utf8 < stop) {
         int type = utf8_byte_type(*(const uint8_t*)utf8);
         SkASSERT(type >= -1 && type <= 4);
-        if (!utf8_type_is_valid_leading_byte(type) ||
-            utf8 + type > stop) {  // Sequence extends beyond end.
+        if (!utf8_type_is_valid_leading_byte(type) || utf8 + type > stop) {
+            // Sequence extends beyond end.
             return -1;
         }
         while(type-- > 1) {
@@ -254,16 +258,26 @@ int SkUTF16_CountUnichars(const uint16_t src[]) {
     return count;
 }
 
-int SkUTF16_CountUnichars(const uint16_t src[], int numberOf16BitValues) {
-    SkASSERT(src);
+// returns -1 on error
+int SkUTF16_CountUnichars(const void* text, size_t byteLength) {
+    SkASSERT(text);
+    if (byteLength == 0) {
+        return 0;
+    }
+    if (!SkIsAlign2(intptr_t(text)) || !SkIsAlign2(byteLength)) {
+        return -1;
+    }
 
-    const uint16_t* stop = src + numberOf16BitValues;
+    const uint16_t* src = static_cast<const uint16_t*>(text);
+    const uint16_t* stop = src + (byteLength >> 1);
     int count = 0;
     while (src < stop) {
         unsigned c = *src++;
         SkASSERT(!SkUTF16_IsLowSurrogate(c));
         if (SkUTF16_IsHighSurrogate(c)) {
-            SkASSERT(src < stop);
+            if (src >= stop) {
+                return -1;
+            }
             c = *src++;
             SkASSERT(SkUTF16_IsLowSurrogate(c));
         }
@@ -361,3 +375,24 @@ const char SkHexadecimalDigits::gUpper[16] =
 const char SkHexadecimalDigits::gLower[16] =
            { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
 
+
+// returns -1 on error
+int SkUTF32_CountUnichars(const void* text, size_t byteLength) {
+    if (byteLength == 0) {
+        return 0;
+    }
+    if (!SkIsAlign4(intptr_t(text)) || !SkIsAlign4(byteLength)) {
+        return -1;
+    }
+    const uint32_t kInvalidUnicharMask = 0xFF000000;    // unichar fits in 24 bits
+    const uint32_t* ptr = static_cast<const uint32_t*>(text);
+    const uint32_t* stop = ptr + (byteLength >> 2);
+    while (ptr < stop) {
+        if (*ptr & kInvalidUnicharMask) {
+            return -1;
+        }
+        ptr += 1;
+    }
+    return SkToInt(byteLength >> 2);
+}
+
diff --git a/src/core/SkUtils.h b/src/core/SkUtils.h
index d894467ce6..9d7c64524f 100644
--- a/src/core/SkUtils.h
+++ b/src/core/SkUtils.h
@@ -43,13 +43,10 @@ inline int SkUTF8_CountUTF8Bytes(const char utf8[]) {
 
 int         SkUTF8_CountUnichars(const char utf8[]);
 
-/** This function is safe: invalid UTF8 sequences will return -1; */
-int         SkUTF8_CountUnicharsWithError(const char utf8[], size_t byteLength);
-
-/** This function is safe: invalid UTF8 sequences will return 0; */
-inline int  SkUTF8_CountUnichars(const char utf8[], size_t byteLength) {
-    return SkClampPos(SkUTF8_CountUnicharsWithError(utf8, byteLength));
-}
+/** These functions are safe: invalid sequences will return -1; */
+int SkUTF8_CountUnichars(const void* utf8, size_t byteLength);
+int SkUTF16_CountUnichars(const void* utf16, size_t byteLength);
+int SkUTF32_CountUnichars(const void* utf32, size_t byteLength);
 
 /** This function is safe: invalid UTF8 sequences will return -1
  *  When -1 is returned, ptr is unchanged.
@@ -83,7 +80,6 @@ size_t      SkUTF8_FromUnichar(SkUnichar uni, char utf8[] = nullptr);
 #define SkUTF16_IsLowSurrogate(c)   (((c) & 0xFC00) == 0xDC00)
 
 int SkUTF16_CountUnichars(const uint16_t utf16[]);
-int SkUTF16_CountUnichars(const uint16_t utf16[], int numberOf16BitValues);
 // returns the current unichar and then moves past it (*p++)
 SkUnichar SkUTF16_NextUnichar(const uint16_t**);
 // this guy backs up to the previus unichar value, and returns it (*--p)
diff --git a/src/xps/SkXPSDevice.cpp b/src/xps/SkXPSDevice.cpp
index 801b80c0b0..354070ce66 100644
--- a/src/xps/SkXPSDevice.cpp
+++ b/src/xps/SkXPSDevice.cpp
@@ -2022,9 +2022,9 @@ HRESULT SkXPSDevice::AddGlyphs(IXpsOMObjectFactory* xpsFactory,
 static int num_glyph_guess(SkPaint::TextEncoding encoding, const void* text, size_t byteLength) {
     switch (encoding) {
     case SkPaint::kUTF8_TextEncoding:
-        return SkUTF8_CountUnichars(static_cast<const char *>(text), byteLength);
+        return SkUTF8_CountUnichars(text, byteLength);
     case SkPaint::kUTF16_TextEncoding:
-        return SkUTF16_CountUnichars(static_cast<const uint16_t *>(text), SkToInt(byteLength));
+        return SkUTF16_CountUnichars(text, byteLength);
     case SkPaint::kGlyphID_TextEncoding:
         return SkToInt(byteLength / 2);
     default:
diff --git a/tests/UtilsTest.cpp b/tests/UtilsTest.cpp
index ed861c69a0..7e11f8478a 100644
--- a/tests/UtilsTest.cpp
+++ b/tests/UtilsTest.cpp
@@ -174,7 +174,7 @@ static void test_utf16(skiatest::Reporter* reporter) {
     for (size_t i = 0; i < SK_ARRAY_COUNT(gUni); i++) {
         size_t count = SkUTF16_FromUnichar(gUni[i], buf);
         REPORTER_ASSERT(reporter, count == 2);
-        size_t count2 = SkUTF16_CountUnichars(buf, 2);
+        size_t count2 = SkUTF16_CountUnichars(buf, 2 * sizeof(uint16_t));
         REPORTER_ASSERT(reporter, count2 == 1);
         const uint16_t* ptr = buf;
         SkUnichar c = SkUTF16_NextUnichar(&ptr);
@@ -227,7 +227,7 @@ DEF_TEST(Utils, reporter) {
 #define LEADING_FOUR_BYTE  "\xF0"
 #define INVALID_BYTE       "\xFC"
 static bool valid_utf8(const char* p, size_t l) {
-    return SkUTF8_CountUnicharsWithError(p, l) >= 0;
+    return SkUTF8_CountUnichars(p, l) >= 0;
 }
 DEF_TEST(Utils_UTF8_ValidLength, r) {
     const char* goodTestcases[] = {