From 00bea4ad310c4ec4dd95809b47ce3fbfa8fd0e1e Mon Sep 17 00:00:00 2001 From: reed Date: Sat, 20 Feb 2016 14:18:27 -0800 Subject: [PATCH] fix misc asserts and checks found by fuzzer BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1713383002 TBR=robertphilips Review URL: https://codereview.chromium.org/1713383002 --- include/core/SkPathEffect.h | 16 +++++++-- include/effects/Sk1DPathEffect.h | 6 +--- include/effects/SkDashPathEffect.h | 7 ++-- samplecode/SampleFilterFuzz.cpp | 4 ++- src/core/SkCanvas.cpp | 6 +++- src/effects/Sk1DPathEffect.cpp | 60 ++++++++++++++++++---------------- src/effects/SkAlphaThresholdFilter.cpp | 9 ++++- src/effects/SkDashPathEffect.cpp | 14 ++++++++ 8 files changed, 80 insertions(+), 42 deletions(-) diff --git a/include/core/SkPathEffect.h b/include/core/SkPathEffect.h index bd68c8f..a77b946 100644 --- a/include/core/SkPathEffect.h +++ b/include/core/SkPathEffect.h @@ -183,7 +183,13 @@ public: The reference counts for outer and inner are both incremented in the constructor, and decremented in the destructor. */ - static SkComposePathEffect* Create(SkPathEffect* outer, SkPathEffect* inner) { + static SkPathEffect* Create(SkPathEffect* outer, SkPathEffect* inner) { + if (!outer) { + return SkSafeRef(inner); + } + if (!inner) { + return SkSafeRef(outer); + } return new SkComposePathEffect(outer, inner); } @@ -220,7 +226,13 @@ public: The reference counts for first and second are both incremented in the constructor, and decremented in the destructor. */ - static SkSumPathEffect* Create(SkPathEffect* first, SkPathEffect* second) { + static SkPathEffect* Create(SkPathEffect* first, SkPathEffect* second) { + if (!first) { + return SkSafeRef(second); + } + if (!second) { + return SkSafeRef(first); + } return new SkSumPathEffect(first, second); } diff --git a/include/effects/Sk1DPathEffect.h b/include/effects/Sk1DPathEffect.h index 3419dc2..c832888 100644 --- a/include/effects/Sk1DPathEffect.h +++ b/include/effects/Sk1DPathEffect.h @@ -45,8 +45,6 @@ public: kTranslate_Style, // translate the shape to each position kRotate_Style, // rotate the shape about its center kMorph_Style, // transform each point, and turn lines into curves - - kStyleCount }; /** Dash by replicating the specified path. @@ -56,9 +54,7 @@ public: @param style how to transform path at each point (based on the current position and tangent) */ - static SkPathEffect* Create(const SkPath& path, SkScalar advance, SkScalar phase, Style style) { - return new SkPath1DPathEffect(path, advance, phase, style); - } + static SkPathEffect* Create(const SkPath& path, SkScalar advance, SkScalar phase, Style); virtual bool filterPath(SkPath*, const SkPath&, SkStrokeRec*, const SkRect*) const override; diff --git a/include/effects/SkDashPathEffect.h b/include/effects/SkDashPathEffect.h index 3c1407b..08b0a46 100644 --- a/include/effects/SkDashPathEffect.h +++ b/include/effects/SkDashPathEffect.h @@ -36,10 +36,7 @@ public: Note: only affects stroked paths. */ - static SkPathEffect* Create(const SkScalar intervals[], int count, SkScalar phase) { - return new SkDashPathEffect(intervals, count, phase); - } - virtual ~SkDashPathEffect(); + static SkPathEffect* Create(const SkScalar intervals[], int count, SkScalar phase); virtual bool filterPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect*) const override; @@ -58,6 +55,7 @@ public: #endif protected: + virtual ~SkDashPathEffect(); SkDashPathEffect(const SkScalar intervals[], int count, SkScalar phase); void flatten(SkWriteBuffer&) const override; @@ -66,6 +64,7 @@ private: int32_t fCount; SkScalar fPhase; // computed from phase + SkScalar fInitialDashLength; int32_t fInitialDashIndex; SkScalar fIntervalLength; diff --git a/samplecode/SampleFilterFuzz.cpp b/samplecode/SampleFilterFuzz.cpp index f8ceed4..45b43df 100644 --- a/samplecode/SampleFilterFuzz.cpp +++ b/samplecode/SampleFilterFuzz.cpp @@ -204,7 +204,7 @@ static SkTypeface::Style make_typeface_style() { } static SkPath1DPathEffect::Style make_path_1d_path_effect_style() { - return static_cast(R(SkPath1DPathEffect::kStyleCount)); + return static_cast(R((int)SkPath1DPathEffect::kMorph_Style + 1)); } static SkColor make_color() { @@ -529,7 +529,9 @@ static SkPaint make_paint() { paint.setMaskFilter(make_mask_filter()); SkAutoTUnref typeface( SkTypeface::CreateFromName(make_font_name().c_str(), make_typeface_style())); +#if 0 paint.setTypeface(typeface); +#endif SkLayerRasterizer::Builder rasterizerBuilder; SkPaint paintForRasterizer; if (R(2) == 1) { diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 653f4b1..aa3261d 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -1072,11 +1072,14 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveLayerFlags saveLayerFlag if (!this->getClipDeviceBounds(&clipBounds)) { return false; } + SkASSERT(!clipBounds.isEmpty()); const SkMatrix& ctm = fMCRec->fMatrix; // this->getTotalMatrix() if (imageFilter) { - imageFilter->filterBounds(clipBounds, ctm, &clipBounds); + if (!imageFilter->filterBounds(clipBounds, ctm, &clipBounds) || clipBounds.isEmpty()) { + return false; + } if (bounds && !imageFilter->canComputeFastBounds()) { bounds = nullptr; } @@ -1778,6 +1781,7 @@ bool SkCanvas::getClipDeviceBounds(SkIRect* bounds) const { return false; } + SkASSERT(!clip.getBounds().isEmpty()); if (bounds) { *bounds = clip.getBounds(); } diff --git a/src/effects/Sk1DPathEffect.cpp b/src/effects/Sk1DPathEffect.cpp index 041886e..4be6f97 100644 --- a/src/effects/Sk1DPathEffect.cpp +++ b/src/effects/Sk1DPathEffect.cpp @@ -35,39 +35,33 @@ bool Sk1DPathEffect::filterPath(SkPath* dst, const SkPath& src, SkPath1DPathEffect::SkPath1DPathEffect(const SkPath& path, SkScalar advance, SkScalar phase, Style style) : fPath(path) { - if (advance <= 0 || path.isEmpty()) { - SkDEBUGF(("SkPath1DPathEffect can't use advance <= 0\n")); - fAdvance = 0; // signals we can't draw anything - fInitialOffset = 0; - fStyle = kStyleCount; - } else { - // cleanup their phase parameter, inverting it so that it becomes an - // offset along the path (to match the interpretation in PostScript) - if (phase < 0) { - phase = -phase; - if (phase > advance) { - phase = SkScalarMod(phase, advance); - } - } else { - if (phase > advance) { - phase = SkScalarMod(phase, advance); - } - phase = advance - phase; + SkASSERT(advance > 0 && !path.isEmpty()); + // cleanup their phase parameter, inverting it so that it becomes an + // offset along the path (to match the interpretation in PostScript) + if (phase < 0) { + phase = -phase; + if (phase > advance) { + phase = SkScalarMod(phase, advance); } - // now catch the edge case where phase == advance (within epsilon) - if (phase >= advance) { - phase = 0; + } else { + if (phase > advance) { + phase = SkScalarMod(phase, advance); } - SkASSERT(phase >= 0); + phase = advance - phase; + } + // now catch the edge case where phase == advance (within epsilon) + if (phase >= advance) { + phase = 0; + } + SkASSERT(phase >= 0); - fAdvance = advance; - fInitialOffset = phase; + fAdvance = advance; + fInitialOffset = phase; - if ((unsigned)style >= kStyleCount) { - SkDEBUGF(("SkPath1DPathEffect style enum out of range %d\n", style)); - } - fStyle = style; + if ((unsigned)style > kMorph_Style) { + SkDEBUGF(("SkPath1DPathEffect style enum out of range %d\n", style)); } + fStyle = style; } bool SkPath1DPathEffect::filterPath(SkPath* dst, const SkPath& src, @@ -207,3 +201,13 @@ void SkPath1DPathEffect::toString(SkString* str) const { str->appendf(")"); } #endif + +/////////////////////////////////////////////////////////////////////////////////////////////////// + +SkPathEffect* SkPath1DPathEffect::Create(const SkPath& path, SkScalar advance, SkScalar phase, + Style style) { + if (advance <= 0 || path.isEmpty()) { + return nullptr; + } + return new SkPath1DPathEffect(path, advance, phase, style); +} diff --git a/src/effects/SkAlphaThresholdFilter.cpp b/src/effects/SkAlphaThresholdFilter.cpp index 7952060..4067970 100644 --- a/src/effects/SkAlphaThresholdFilter.cpp +++ b/src/effects/SkAlphaThresholdFilter.cpp @@ -45,11 +45,19 @@ SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkAlphaThresholdFilter) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkAlphaThresholdFilterImpl) SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END +static bool outside_unit(SkScalar x) { + return x < 0 || x > 1; +} SkImageFilter* SkAlphaThresholdFilter::Create(const SkRegion& region, SkScalar innerThreshold, SkScalar outerThreshold, SkImageFilter* input) { + if (outside_unit(innerThreshold) || outside_unit(outerThreshold) || + innerThreshold > outerThreshold) + { + return nullptr; + } return new SkAlphaThresholdFilterImpl(region, innerThreshold, outerThreshold, input); } @@ -334,7 +342,6 @@ void SkAlphaThresholdFilterImpl::flatten(SkWriteBuffer& buffer) const { bool SkAlphaThresholdFilterImpl::onFilterImageDeprecated(Proxy* proxy, const SkBitmap& src, const Context& ctx, SkBitmap* dst, SkIPoint* offset) const { - SkASSERT(src.colorType() == kN32_SkColorType); if (src.colorType() != kN32_SkColorType) { return false; diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp index 6e10e54..ced0aab 100644 --- a/src/effects/SkDashPathEffect.cpp +++ b/src/effects/SkDashPathEffect.cpp @@ -384,3 +384,17 @@ void SkDashPathEffect::toString(SkString* str) const { str->appendf("))"); } #endif + +////////////////////////////////////////////////////////////////////////////////////////////////// + +SkPathEffect* SkDashPathEffect::Create(const SkScalar intervals[], int count, SkScalar phase) { + if ((count < 2) || !SkIsAlign2(count)) { + return nullptr; + } + for (int i = 0; i < count; i++) { + if (intervals[i] < 0) { + return nullptr; + } + } + return new SkDashPathEffect(intervals, count, phase); +} -- 2.7.4