Harden SkPicturePlayback::handleOp() skips

SkValidatingReadBuffer::skip() may return null - tread more carefully
around it.

BUG=skia:5828
R=reed@google.com,mtklein@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2399043002

Review-Url: https://codereview.chromium.org/2399043002

1 files changed, 126 insertions, 23 deletions

diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index a246a306e6..bede60111e 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -126,6 +126,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
                                  uint32_t size,
                                  SkCanvas* canvas,
                                  const SkMatrix& initialMatrix) {
+#define BREAK_ON_READ_ERROR(r) if (!r->isValid()) { break; }
+
     switch (op) {
         case NOOP: {
             SkASSERT(size >= 4);
@@ -137,6 +139,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkCanvas::ClipOp clipOp = ClipParams_unpackRegionOp(packed);
             bool doAA = ClipParams_unpackDoAA(packed);
             size_t offsetToRestore = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
             SkASSERT(!offsetToRestore || offsetToRestore >= reader->offset());
             canvas->clipPath(path, clipOp, doAA);
             if (canvas->isClipEmpty() && offsetToRestore) {
@@ -149,6 +153,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             uint32_t packed = reader->readInt();
             SkCanvas::ClipOp clipOp = ClipParams_unpackRegionOp(packed);
             size_t offsetToRestore = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
             SkASSERT(!offsetToRestore || offsetToRestore >= reader->offset());
             canvas->clipRegion(region, clipOp);
             if (canvas->isClipEmpty() && offsetToRestore) {
@@ -162,6 +168,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkCanvas::ClipOp clipOp = ClipParams_unpackRegionOp(packed);
             bool doAA = ClipParams_unpackDoAA(packed);
             size_t offsetToRestore = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
             SkASSERT(!offsetToRestore || offsetToRestore >= reader->offset());
             canvas->clipRect(rect, clipOp, doAA);
             if (canvas->isClipEmpty() && offsetToRestore) {
@@ -175,6 +183,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkCanvas::ClipOp clipOp = ClipParams_unpackRegionOp(packed);
             bool doAA = ClipParams_unpackDoAA(packed);
             size_t offsetToRestore = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
             SkASSERT(!offsetToRestore || offsetToRestore >= reader->offset());
             canvas->clipRRect(rrect, clipOp, doAA);
             if (canvas->isClipEmpty() && offsetToRestore) {
@@ -186,6 +196,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         case CONCAT: {
             SkMatrix matrix;
             reader->readMatrix(&matrix);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->concat(matrix);
             break;
         }
@@ -194,7 +206,10 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             reader->readRect(&rect);
             SkString key;
             reader->readString(&key);
-            canvas->drawAnnotation(rect, key.c_str(), reader->readByteArrayAsData().get());
+            sk_sp<SkData> data = reader->readByteArrayAsData();
+            BREAK_ON_READ_ERROR(reader);
+
+            canvas->drawAnnotation(rect, key.c_str(), data.get());
         } break;
         case DRAW_ARC: {
             const SkPaint* paint = fPictureData->getPaint(reader);
@@ -203,6 +218,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkScalar startAngle = reader->readScalar();
             SkScalar sweepAngle = reader->readScalar();
             int useCenter = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawArc(rect, startAngle, sweepAngle, SkToBool(useCenter), *paint);
             }
@@ -224,6 +241,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             if (flags & DRAW_ATLAS_HAS_CULL) {
                 cull = (const SkRect*)reader->skip(sizeof(SkRect));
             }
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawAtlas(atlas, xform, tex, colors, count, mode, cull, paint);
         } break;
         case DRAW_BITMAP: {
@@ -231,6 +250,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkImage* image = fPictureData->getBitmapAsImage(reader);
             SkPoint loc;
             reader->readPoint(&loc);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawImage(image, loc.fX, loc.fY, paint);
         } break;
         case DRAW_BITMAP_RECT: {
@@ -241,6 +262,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkRect dst;
             reader->readRect(&dst);     // required
             SkCanvas::SrcRectConstraint constraint = (SkCanvas::SrcRectConstraint)reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
             if (src) {
                 canvas->drawImageRect(image, *src, dst, paint, constraint);
             } else {
@@ -252,6 +275,7 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkImage* image = fPictureData->getBitmapAsImage(reader);
             SkMatrix matrix;
             reader->readMatrix(&matrix);
+            BREAK_ON_READ_ERROR(reader);
 
             SkAutoCanvasRestore acr(canvas, true);
             canvas->concat(matrix);
@@ -264,24 +288,34 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             reader->readIRect(&src);
             SkRect dst;
             reader->readRect(&dst);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawImageNine(image, src, dst, paint);
         } break;
-        case DRAW_CLEAR:
-            canvas->clear(reader->readInt());
-            break;
+        case DRAW_CLEAR: {
+            auto c = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
+            canvas->clear(c);
+        } break;
         case DRAW_DATA: {
             // This opcode is now dead, just need to skip it for backwards compatibility
             size_t length = reader->readInt();
             (void)reader->skip(length);
             // skip handles padding the read out to a multiple of 4
         } break;
-        case DRAW_DRAWABLE:
-            canvas->drawDrawable(fPictureData->getDrawable(reader));
-            break;
+        case DRAW_DRAWABLE: {
+            auto* d = fPictureData->getDrawable(reader);
+            BREAK_ON_READ_ERROR(reader);
+
+            canvas->drawDrawable(d);
+        } break;
         case DRAW_DRAWABLE_MATRIX: {
             SkMatrix matrix;
             reader->readMatrix(&matrix);
             SkDrawable* drawable = fPictureData->getDrawable(reader);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawDrawable(drawable, &matrix);
         } break;
         case DRAW_DRRECT: {
@@ -289,6 +323,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkRRect outer, inner;
             reader->readRRect(&outer);
             reader->readRRect(&inner);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawDRRect(outer, inner, *paint);
             }
@@ -314,6 +350,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkImage* image = fPictureData->getImage(reader);
             SkPoint loc;
             reader->readPoint(&loc);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawImage(image, loc.fX, loc.fY, paint);
         } break;
         case DRAW_IMAGE_LATTICE: {
@@ -332,6 +370,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             lattice.fBounds = &src;
             SkRect dst;
             reader->readRect(&dst);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawImageLattice(image, lattice, dst, paint);
         } break;
         case DRAW_IMAGE_NINE: {
@@ -341,6 +381,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             reader->readIRect(&center);
             SkRect dst;
             reader->readRect(&dst);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawImageNine(image, center, dst, paint);
         } break;
         case DRAW_IMAGE_RECT_STRICT:
@@ -357,18 +399,24 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
                 // newer op-code stores the constraint explicitly
                 constraint = (SkCanvas::SrcRectConstraint)reader->readInt();
             }
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->legacy_drawImageRect(image, src, dst, paint, constraint);
         } break;
         case DRAW_OVAL: {
             const SkPaint* paint = fPictureData->getPaint(reader);
             SkRect rect;
             reader->readRect(&rect);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawOval(rect, *paint);
             }
         } break;
         case DRAW_PAINT: {
             const SkPaint* paint = fPictureData->getPaint(reader);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawPaint(*paint);
             }
@@ -396,24 +444,34 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
                 }
                 xfer = SkXfermode::Make((SkXfermode::Mode)mode);
             }
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawPatch(cubics, colors, texCoords, std::move(xfer), *paint);
             }
         } break;
         case DRAW_PATH: {
             const SkPaint* paint = fPictureData->getPaint(reader);
+            const auto& path = fPictureData->getPath(reader);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
-                canvas->drawPath(fPictureData->getPath(reader), *paint);
+                canvas->drawPath(path, *paint);
             }
         } break;
-        case DRAW_PICTURE:
-            canvas->drawPicture(fPictureData->getPicture(reader));
-            break;
+        case DRAW_PICTURE: {
+            const auto* pic = fPictureData->getPicture(reader);
+            BREAK_ON_READ_ERROR(reader);
+
+            canvas->drawPicture(pic);
+        } break;
         case DRAW_PICTURE_MATRIX_PAINT: {
             const SkPaint* paint = fPictureData->getPaint(reader);
             SkMatrix matrix;
             reader->readMatrix(&matrix);
             const SkPicture* pic = fPictureData->getPicture(reader);
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->drawPicture(pic, &matrix, paint);
         } break;
         case DRAW_POINTS: {
@@ -421,6 +479,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             SkCanvas::PointMode mode = (SkCanvas::PointMode)reader->readInt();
             size_t count = reader->readInt();
             const SkPoint* pts = (const SkPoint*)reader->skip(sizeof(SkPoint)* count);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawPoints(mode, count, pts, *paint);
             }
@@ -431,6 +491,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             get_text(reader, &text);
             size_t points = reader->readInt();
             const SkPoint* pos = (const SkPoint*)reader->skip(points * sizeof(SkPoint));
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint && text.text()) {
                 canvas->drawPosText(text.text(), text.length(), pos, *paint);
             }
@@ -443,6 +505,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkPoint* pos = (const SkPoint*)reader->skip(points * sizeof(SkPoint));
             const SkScalar top = reader->readScalar();
             const SkScalar bottom = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             SkRect clip;
             canvas->getClipBounds(&clip);
             if (top < clip.fBottom && bottom > clip.fTop && paint && text.text()) {
@@ -456,6 +520,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             size_t xCount = reader->readInt();
             const SkScalar constY = reader->readScalar();
             const SkScalar* xpos = (const SkScalar*)reader->skip(xCount * sizeof(SkScalar));
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint && text.text()) {
                 canvas->drawPosTextH(text.text(), text.length(), xpos, constY, *paint);
             }
@@ -466,6 +532,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             get_text(reader, &text);
             size_t xCount = reader->readInt();
             const SkScalar* xpos = (const SkScalar*)reader->skip((3 + xCount) * sizeof(SkScalar));
+            BREAK_ON_READ_ERROR(reader);
+
             const SkScalar top = *xpos++;
             const SkScalar bottom = *xpos++;
             const SkScalar constY = *xpos++;
@@ -479,6 +547,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkPaint* paint = fPictureData->getPaint(reader);
             SkRect rect;
             reader->readRect(&rect);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawRect(rect, *paint);
             }
@@ -487,6 +557,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkPaint* paint = fPictureData->getPaint(reader);
             SkRegion region;
             reader->readRegion(&region);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawRegion(region, *paint);
             }
@@ -495,6 +567,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkPaint* paint = fPictureData->getPaint(reader);
             SkRRect rrect;
             reader->readRRect(&rrect);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawRRect(rrect, *paint);
             }
@@ -512,6 +586,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             get_text(reader, &text);
             SkScalar x = reader->readScalar();
             SkScalar y = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint && text.text()) {
                 canvas->drawText(text.text(), text.length(), x, y, *paint);
             }
@@ -521,6 +597,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkTextBlob* blob = fPictureData->getTextBlob(reader);
             SkScalar x = reader->readScalar();
             SkScalar y = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawTextBlob(blob, x, y, *paint);
             }
@@ -530,6 +608,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             TextContainer text;
             get_text(reader, &text);
             const SkScalar* ptr = (const SkScalar*)reader->skip(4 * sizeof(SkScalar));
+            BREAK_ON_READ_ERROR(reader);
+
             // ptr[0] == x
             // ptr[1] == y
             // ptr[2] == top
@@ -549,6 +629,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkPath& path = fPictureData->getPath(reader);
             SkMatrix matrix;
             reader->readMatrix(&matrix);
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint && text.text()) {
                 canvas->drawTextOnPath(text.text(), text.length(), path, &matrix, *paint);
             }
@@ -564,6 +646,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             if (flags & DRAW_TEXT_RSXFORM_HAS_CULL) {
                 cull = (const SkRect*)reader->skip(sizeof(SkRect));
             }
+            BREAK_ON_READ_ERROR(reader);
+
             if (text.text()) {
                 canvas->drawTextRSXform(text.text(), text.length(), xform, cull, *paint);
             }
@@ -596,6 +680,8 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
                 }
                 xfer = SkXfermode::Make((SkXfermode::Mode)mode);
             }
+            BREAK_ON_READ_ERROR(reader);
+
             if (paint) {
                 canvas->drawVertices(vmode, vCount, verts, texs, colors,
                                      xfer, indices, iCount, *paint);
@@ -604,16 +690,13 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
         case RESTORE:
             canvas->restore();
             break;
-        case ROTATE:
-            canvas->rotate(reader->readScalar());
-            break;
+        case ROTATE: {
+            auto deg = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
+            canvas->rotate(deg);
+        } break;
         case SAVE:
-            // SKPs with version < 29 also store a SaveFlags param.
-            if (size > 4) {
-                if (reader->validate(8 == size)) {
-                    reader->readInt();
-                }
-            }
             canvas->save();
             break;
         case SAVE_LAYER_SAVEFLAGS_DEPRECATED: {
@@ -621,13 +704,18 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             const SkRect* boundsPtr = get_rect_ptr(reader, &storage);
             const SkPaint* paint = fPictureData->getPaint(reader);
             auto flags = SkCanvas::LegacySaveFlagsToSaveLayerFlags(reader->readInt());
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->saveLayer(SkCanvas::SaveLayerRec(boundsPtr, paint, flags));
         } break;
         case SAVE_LAYER_SAVELAYERFLAGS_DEPRECATED_JAN_2016: {
             SkRect storage;
             const SkRect* boundsPtr = get_rect_ptr(reader, &storage);
             const SkPaint* paint = fPictureData->getPaint(reader);
-            canvas->saveLayer(SkCanvas::SaveLayerRec(boundsPtr, paint, reader->readInt()));
+            auto flags = reader->readInt();
+            BREAK_ON_READ_ERROR(reader);
+
+            canvas->saveLayer(SkCanvas::SaveLayerRec(boundsPtr, paint, flags));
         } break;
         case SAVE_LAYER_SAVELAYERREC: {
             SkCanvas::SaveLayerRec rec(nullptr, nullptr, nullptr, 0);
@@ -641,42 +729,57 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
                 rec.fPaint = fPictureData->getPaint(reader);
             }
             if (flatFlags & SAVELAYERREC_HAS_BACKDROP) {
-                const SkPaint* paint = fPictureData->getPaint(reader);
-                rec.fBackdrop = paint->getImageFilter();
+                if (const auto* paint = fPictureData->getPaint(reader)) {
+                    rec.fBackdrop = paint->getImageFilter();
+                }
             }
             if (flatFlags & SAVELAYERREC_HAS_FLAGS) {
                 rec.fSaveLayerFlags = reader->readInt();
             }
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->saveLayer(rec);
         } break;
         case SCALE: {
             SkScalar sx = reader->readScalar();
             SkScalar sy = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->scale(sx, sy);
         } break;
         case SET_MATRIX: {
             SkMatrix matrix;
             reader->readMatrix(&matrix);
+            BREAK_ON_READ_ERROR(reader);
+
             matrix.postConcat(initialMatrix);
             canvas->setMatrix(matrix);
         } break;
         case SKEW: {
             SkScalar sx = reader->readScalar();
             SkScalar sy = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->skew(sx, sy);
         } break;
         case TRANSLATE: {
             SkScalar dx = reader->readScalar();
             SkScalar dy = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->translate(dx, dy);
         } break;
         case TRANSLATE_Z: {
 #ifdef SK_EXPERIMENTAL_SHADOWING
             SkScalar dz = reader->readScalar();
+            BREAK_ON_READ_ERROR(reader);
+
             canvas->translateZ(dz);
 #endif
         } break;
         default:
             SkASSERTF(false, "Unknown draw type: %d", op);
     }
+
+#undef BREAK_ON_READ_ERROR
 }