"Fix" some ImageFilter fuzzer issues
SkClipOp.h & SkPictureFlat.h
Invalid SkClipOps were getting through - the question here is where (for a class enum) is a good place to put the k*Mask definition.
SkPath1DPathEffect
NaNs were getting past.
SkBlurMaskFilter
Assert wasn't necessary since we whacked the flag on the next line.
Change-Id: I87f95ad39f4760284f881d7c4500eb82fcdba282
Reviewed-on: https://skia-review.googlesource.com/6194
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Herb Derby <herb@google.com>
diff --git a/gm/drawfilter.cpp b/gm/drawfilter.cpp
index d989c4c..b283c13 100644
--- a/gm/drawfilter.cpp
+++ b/gm/drawfilter.cpp
@@ -46,8 +46,8 @@
void onOnceBeforeDraw() override {
fBlur = SkBlurMaskFilter::Make(kNormal_SkBlurStyle,
- SkBlurMask::ConvertRadiusToSigma(10.0f),
- kLow_SkBlurQuality);
+ SkBlurMask::ConvertRadiusToSigma(10.0f),
+ kLow_SkBlurQuality);
}
void onDraw(SkCanvas* canvas) override {
diff --git a/include/core/SkClipOp.h b/include/core/SkClipOp.h
index 1c4b3b7..3035565 100644
--- a/include/core/SkClipOp.h
+++ b/include/core/SkClipOp.h
@@ -61,6 +61,8 @@
kXOR_private_internal_do_not_use = 3,
kReverseDifference_private_internal_do_not_use = 4,
kReplace_private_internal_do_not_use = 5,
+
+ kMax_EnumValue = kReplace_private_internal_do_not_use
};
#endif
diff --git a/include/effects/SkBlurMaskFilter.h b/include/effects/SkBlurMaskFilter.h
index 4106b01..8b032e5 100644
--- a/include/effects/SkBlurMaskFilter.h
+++ b/include/effects/SkBlurMaskFilter.h
@@ -22,13 +22,13 @@
static SkScalar ConvertRadiusToSigma(SkScalar radius);
enum BlurFlags {
- kNone_BlurFlag = 0x00,
+ kNone_BlurFlag = 0x00,
/** The blur layer's radius is not affected by transforms */
kIgnoreTransform_BlurFlag = 0x01,
/** Use a smother, higher qulity blur algorithm */
kHighQuality_BlurFlag = 0x02,
/** mask for all blur flags */
- kAll_BlurFlag = 0x03
+ kAll_BlurFlag = 0x03
};
/** Create a blur maskfilter.
diff --git a/src/core/SkPictureFlat.h b/src/core/SkPictureFlat.h
index 1ac91b7..3489043 100644
--- a/src/core/SkPictureFlat.h
+++ b/src/core/SkPictureFlat.h
@@ -133,8 +133,17 @@
return (doAABit << 4) | static_cast<int>(op);
}
-static inline SkClipOp ClipParams_unpackRegionOp(uint32_t packed) {
- return (SkClipOp)(packed & 0xF);
+template <typename T> T asValidEnum(SkReadBuffer* buffer, uint32_t candidate) {
+
+ if (buffer->validate(candidate <= static_cast<uint32_t>(T::kMax_EnumValue))) {
+ return static_cast<T>(candidate);
+ }
+
+ return T::kMax_EnumValue;
+}
+
+static inline SkClipOp ClipParams_unpackRegionOp(SkReadBuffer* buffer, uint32_t packed) {
+ return asValidEnum<SkClipOp>(buffer, packed & 0xF);
}
static inline bool ClipParams_unpackDoAA(uint32_t packed) {
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 85e5c03..c52233a 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -136,7 +136,7 @@
case CLIP_PATH: {
const SkPath& path = fPictureData->getPath(reader);
uint32_t packed = reader->readInt();
- SkClipOp clipOp = ClipParams_unpackRegionOp(packed);
+ SkClipOp clipOp = ClipParams_unpackRegionOp(reader, packed);
bool doAA = ClipParams_unpackDoAA(packed);
size_t offsetToRestore = reader->readInt();
BREAK_ON_READ_ERROR(reader);
@@ -151,7 +151,7 @@
SkRegion region;
reader->readRegion(®ion);
uint32_t packed = reader->readInt();
- SkClipOp clipOp = ClipParams_unpackRegionOp(packed);
+ SkClipOp clipOp = ClipParams_unpackRegionOp(reader, packed);
size_t offsetToRestore = reader->readInt();
BREAK_ON_READ_ERROR(reader);
@@ -165,7 +165,7 @@
SkRect rect;
reader->readRect(&rect);
uint32_t packed = reader->readInt();
- SkClipOp clipOp = ClipParams_unpackRegionOp(packed);
+ SkClipOp clipOp = ClipParams_unpackRegionOp(reader, packed);
bool doAA = ClipParams_unpackDoAA(packed);
size_t offsetToRestore = reader->readInt();
BREAK_ON_READ_ERROR(reader);
@@ -180,7 +180,7 @@
SkRRect rrect;
reader->readRRect(&rrect);
uint32_t packed = reader->readInt();
- SkClipOp clipOp = ClipParams_unpackRegionOp(packed);
+ SkClipOp clipOp = ClipParams_unpackRegionOp(reader, packed);
bool doAA = ClipParams_unpackDoAA(packed);
size_t offsetToRestore = reader->readInt();
BREAK_ON_READ_ERROR(reader);
diff --git a/src/effects/Sk1DPathEffect.cpp b/src/effects/Sk1DPathEffect.cpp
index 26cd046..2247a79 100644
--- a/src/effects/Sk1DPathEffect.cpp
+++ b/src/effects/Sk1DPathEffect.cpp
@@ -205,7 +205,7 @@
sk_sp<SkPathEffect> SkPath1DPathEffect::Make(const SkPath& path, SkScalar advance, SkScalar phase,
Style style) {
- if (advance <= 0 || path.isEmpty()) {
+ if (advance <= 0 || !SkScalarIsFinite(advance) || path.isEmpty()) {
return nullptr;
}
return sk_sp<SkPathEffect>(new SkPath1DPathEffect(path, advance, phase, style));
diff --git a/src/effects/SkBlurMaskFilter.cpp b/src/effects/SkBlurMaskFilter.cpp
index 84d9e18..f56c273 100644
--- a/src/effects/SkBlurMaskFilter.cpp
+++ b/src/effects/SkBlurMaskFilter.cpp
@@ -128,14 +128,12 @@
sk_sp<SkMaskFilter> SkBlurMaskFilter::Make(SkBlurStyle style, SkScalar sigma,
const SkRect& occluder, uint32_t flags) {
+ SkASSERT(!(flags & ~SkBlurMaskFilter::kAll_BlurFlag));
+ SkASSERT(style <= kLastEnum_SkBlurStyle);
+
if (!SkScalarIsFinite(sigma) || sigma <= 0) {
return nullptr;
}
- if ((unsigned)style > (unsigned)kLastEnum_SkBlurStyle) {
- return nullptr;
- }
- SkASSERT(flags <= SkBlurMaskFilter::kAll_BlurFlag);
- flags &= SkBlurMaskFilter::kAll_BlurFlag;
return sk_sp<SkMaskFilter>(new SkBlurMaskFilterImpl(sigma, style, occluder, flags));
}
@@ -735,7 +733,12 @@
sk_sp<SkFlattenable> SkBlurMaskFilterImpl::CreateProc(SkReadBuffer& buffer) {
const SkScalar sigma = buffer.readScalar();
const unsigned style = buffer.readUInt();
- const unsigned flags = buffer.readUInt();
+ unsigned flags = buffer.readUInt();
+
+ buffer.validate(style <= kLastEnum_SkBlurStyle);
+ buffer.validate(!(flags & ~SkBlurMaskFilter::kAll_BlurFlag));
+
+ flags &= SkBlurMaskFilter::kAll_BlurFlag;
SkRect occluder;
if (buffer.isVersionLT(SkReadBuffer::kBlurMaskFilterWritesOccluder)) {
diff --git a/tests/BlurTest.cpp b/tests/BlurTest.cpp
index 2893a0c..f2c35d7 100644
--- a/tests/BlurTest.cpp
+++ b/tests/BlurTest.cpp
@@ -507,7 +507,7 @@
// Test asABlur for SkBlurMaskFilter
//
for (size_t i = 0; i < SK_ARRAY_COUNT(styles); ++i) {
- const SkBlurStyle style = (SkBlurStyle)styles[i];
+ const SkBlurStyle style = styles[i];
for (size_t j = 0; j < SK_ARRAY_COUNT(sigmas); ++j) {
const SkScalar sigma = sigmas[j];
for (int flags = 0; flags <= SkBlurMaskFilter::kAll_BlurFlag; ++flags) {