Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(138)

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
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 »

Powered by Google App Engine
This is Rietveld 408576698