From 889d521d8715f4934accb630097bc09bf7ad1a32 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Thu, 21 Dec 2017 13:34:24 -0500 Subject: validate text during deserialization Bug: 796473 Change-Id: I7b6a6c698a5b53c915ef6564852fa51ce7410a3e Reviewed-on: https://skia-review.googlesource.com/88520 Commit-Queue: Mike Reed Reviewed-by: Hal Canary --- src/core/SkFont.cpp | 2 +- src/core/SkPaint.cpp | 4 +-- src/core/SkPaintPriv.cpp | 19 ++++++++++++ src/core/SkPaintPriv.h | 3 ++ src/core/SkPicturePlayback.cpp | 65 +++++++++++++++++++++--------------------- src/core/SkUtils.cpp | 51 +++++++++++++++++++++++++++------ src/core/SkUtils.h | 12 +++----- src/xps/SkXPSDevice.cpp | 4 +-- 8 files changed, 107 insertions(+), 53 deletions(-) (limited to 'src') 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(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(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(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(text), byteLength); + return SkUTF8_CountUnichars(text, byteLength); case SkPaint::kUTF16_TextEncoding: - return SkUTF16_CountUnichars(static_cast(text), SkToInt(byteLength)); + return SkUTF16_CountUnichars(text, byteLength); case SkPaint::kGlyphID_TextEncoding: return SkToInt(byteLength / 2); default: -- cgit v1.2.3