src/core/SkPicturePlayback.cpp - Issue 2399043002: Harden SkPicturePlayback::handleOp() skips

Unified Diff: src/core/SkPicturePlayback.cpp

Issue 2399043002: Harden SkPicturePlayback::handleOp() skips (Closed)

Patch Set: BREAK_ON_READ_ERROR Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: src/core/SkPicturePlayback.cpp

diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp

index a246a306e6b1f1ded8714a90b0d0be142c92ce9e..bede60111ed07da9da2ba51f97c816de2c19c8e8 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

}

« no previous file with comments | « no previous file | no next file » | no next file with comments »