[skif] Check for resolve failure/int overflows
Bug: oss-fuzz:70128
Bug: oss-fuzz:69848
Bug: oss-fuzz:70134
Change-Id: Iccf4fb389f16cccff3668d21d02ba439f54d80be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/900497
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/core/SkImageFilterTypes.cpp b/src/core/SkImageFilterTypes.cpp
index 0d8bd81..612337f 100644
--- a/src/core/SkImageFilterTypes.cpp
+++ b/src/core/SkImageFilterTypes.cpp
@@ -129,12 +129,12 @@
double cropHeight = crop.bottom() - cropT;
// Calculate normalized periodic coordinates of 'output' relative to the 'crop' being tiled.
- int periodL = sk_double_floor2int((output.left() - cropL) / cropWidth);
- int periodT = sk_double_floor2int((output.top() - cropT) / cropHeight);
- int periodR = sk_double_ceil2int((output.right() - cropL) / cropWidth);
- int periodB = sk_double_ceil2int((output.bottom() - cropT) / cropHeight);
+ double periodL = std::floor((output.left() - cropL) / cropWidth);
+ double periodT = std::floor((output.top() - cropT) / cropHeight);
+ double periodR = std::ceil((output.right() - cropL) / cropWidth);
+ double periodB = std::ceil((output.bottom() - cropT) / cropHeight);
- if (periodR - periodL <= 1 && periodB - periodT <= 1) {
+ if (periodR - periodL <= 1.0 && periodB - periodT <= 1.0) {
// The tiling pattern won't be visible, so we can draw the image without tiling and an
// adjusted transform. We calculate the final translation in double to be exact and then
// verify that it can round-trip as a float.
@@ -144,12 +144,13 @@
double ty = -cropT;
if (tileMode == SkTileMode::kMirror) {
- // Flip image when in odd periods on each axis.
- if (periodL % 2 != 0) {
+ // Flip image when in odd periods on each axis. The periods are stored as doubles but
+ // hold integer values since they came from floor or ceil.
+ if (std::fmod(periodL, 2.f) > SK_ScalarNearlyZero) {
sx = -1.f;
tx = cropWidth - tx;
}
- if (periodT % 2 != 0) {
+ if (std::fmod(periodT, 2.f) > SK_ScalarNearlyZero) {
sy = -1.f;
ty = cropHeight - ty;
}
@@ -536,7 +537,7 @@
public:
AutoSurface(const Context& ctx,
const LayerSpace<SkIRect>& dstBounds,
- [[maybe_unused]] PixelBoundary boundary,
+ PixelBoundary boundary,
bool renderInParameterSpace,
const SkSurfaceProps* props = nullptr)
: fDstBounds(dstBounds)
@@ -547,7 +548,21 @@
// to align with the actual desired output via FilterResult metadata).
sk_sp<SkDevice> device = nullptr;
if (!dstBounds.isEmpty()) {
- fDstBounds.outset(LayerSpace<SkISize>({this->padding(), this->padding()}));
+ int padding = this->padding();
+ if (padding) {
+ fDstBounds.outset(LayerSpace<SkISize>({padding, padding}));
+ // If we are dealing with pathological inputs, the bounds may be near the maximum
+ // represented by an int, in which case the outset gets saturated and we don't end
+ // up with the expected padding pixels. We could downgrade the boundary value in
+ // this case, but given that these values are going to be causing problems for any
+ // of the floating point math during rendering we just fail.
+ if (fDstBounds.left() >= dstBounds.left() ||
+ fDstBounds.right() <= dstBounds.right() ||
+ fDstBounds.top() >= dstBounds.top() ||
+ fDstBounds.bottom() <= dstBounds.bottom()) {
+ return;
+ }
+ }
device = ctx.backend()->makeDevice(SkISize(fDstBounds.size()),
ctx.refColorSpace(),
props);
@@ -1719,6 +1734,10 @@
// the rescaling steps because, for better or worse, the deferred transform does not
// otherwise participate in progressive scaling so we should be consistent.
image = image.resolve(ctx, srcRect);
+ if (!image) {
+ // Early out if the resolve failed
+ return {};
+ }
if (!cfBorder) {
// This sets the resolved image to match either kDecal or the deferred tile mode.
image.fTileMode = tileMode;
diff --git a/tests/FilterResultTest.cpp b/tests/FilterResultTest.cpp
index e58dbc7..3692e35 100644
--- a/tests/FilterResultTest.cpp
+++ b/tests/FilterResultTest.cpp
@@ -400,8 +400,6 @@
sk_sp<SkSpecialImage> source,
LayerSpace<SkIPoint> origin,
const LayerSpace<SkIRect>& desiredOutput) const {
- SkASSERT(source);
-
Expect effectiveExpectation = fExpectation;
SkISize size(desiredOutput.size());
if (desiredOutput.isEmpty()) {
@@ -410,17 +408,21 @@
}
auto device = ctx.backend()->makeDevice(size, ctx.refColorSpace());
+ if (!device) {
+ return nullptr;
+ }
SkCanvas canvas{device};
canvas.clear(SK_ColorTRANSPARENT);
canvas.translate(-desiredOutput.left(), -desiredOutput.top());
- LayerSpace<SkIRect> sourceBounds{
- SkIRect::MakeXYWH(origin.x(), origin.y(), source->width(), source->height())};
- LayerSpace<SkIRect> expectedBounds = this->expectedBounds(sourceBounds);
-
- canvas.clipIRect(SkIRect(expectedBounds), SkClipOp::kIntersect);
-
if (effectiveExpectation != Expect::kEmptyImage) {
+ SkASSERT(source);
+ LayerSpace<SkIRect> sourceBounds{
+ SkIRect::MakeXYWH(origin.x(), origin.y(), source->width(), source->height())};
+ LayerSpace<SkIRect> expectedBounds = this->expectedBounds(sourceBounds);
+
+ canvas.clipIRect(SkIRect(expectedBounds), SkClipOp::kIntersect);
+
SkPaint paint;
paint.setAntiAlias(true);
paint.setBlendMode(SkBlendMode::kSrc);
@@ -648,7 +650,11 @@
const FilterResult& actual,
float allowedPercentImageDiff,
int transparentCheckBorderTolerance) {
- SkASSERT(expectedImage);
+ if (!expectedImage) {
+ // For pathological desired outputs, we can't actually produce an expected image so
+ // just carry on w/o validating.
+ return true;
+ }
SkBitmap expectedBM = this->readPixels(expectedImage);
@@ -1202,7 +1208,12 @@
// resolved image, which can make its layer bounds larger than the desired output.
if (correctedExpectation == Expect::kDeferredImage ||
!FilterResultTestAccess::IsIntegerTransform(output)) {
- REPORTER_ASSERT(fRunner, actualBounds.intersect(desiredOutputs[i]));
+ // Skip the check if the desiredOutputs's SkIRect reports empty.
+ // LayerSpace<SkIRect> won't be empty but since the W/H don't fit into 32-bit
+ // SkIRect::intersect() will report false.
+ REPORTER_ASSERT(fRunner, SkIRect(desiredOutputs[i]).isEmpty() ||
+ actualBounds.intersect(desiredOutputs[i]));
+
}
REPORTER_ASSERT(fRunner, !expectedBounds.isEmpty());
REPORTER_ASSERT(fRunner, SkIRect(actualBounds) == SkIRect(expectedBounds));
@@ -1612,11 +1623,18 @@
.applyCrop({-5, -5, 15, 15}, tm, Expect::kNewImage)
.run(/*requestedOutput=*/{10, 10, 20, 20});
- TestCase(r, "Periodic applyCropp() with visible edge and complex transform creates image")
+ TestCase(r, "Periodic applyCrop() with visible edge and complex transform creates image")
.source({0, 0, 20, 20})
.applyTransform(SkMatrix::RotateDeg(15.f, {10.f, 10.f}), Expect::kDeferredImage)
.applyCrop({-5, -5, 25, 25}, tm, Expect::kNewImage)
.run(/*requestedOutput=*/{20, 20, 50, 50});
+
+ // oss-fuzz:70128 ensure period calculations don't overflow (but will fail image creation)
+ TestCase(r, "Pathologically large crop rect")
+ .source({0, 0, 10, 10})
+ .applyCrop({0, 0, 1, 2}, tm, Expect::kDeferredImage)
+ .applyTransform(SkMatrix::RotateDeg(1.f, {5.f, 5.f}), Expect::kEmptyImage)
+ .run(/*requestedOutput=*/{-726713344, 7, 1464866662, 15});
}
}
@@ -1950,6 +1968,14 @@
.applyColorFilter(alpha_modulate(0.5f), Expect::kDeferredImage)
.applyColorFilter(affect_transparent(SkColors::kBlue), Expect::kDeferredImage)
.run(/*requestedOutput=*/{-8, -8, 32, 32});
+
+ // oss-fuzz:70134 Requesting the output of an infinite image (e.g. transparency affecting
+ // color filter) at the limit of 32-bit ints doesn't leave room for border padding pixels.
+ // This should cleanly fail to an empty image.
+ TestCase(r, "Pathologic output bounds with transparency-affecting color filter is empty")
+ .source({0, 0, 16, 16})
+ .applyColorFilter(affect_transparent(SkColors::kRed), Expect::kEmptyImage)
+ .run(/*requestedOutput=*/{-INT32_MAX, 0, -INT32_MAX + 10, 16});
}
DEF_TEST_SUITE(TransformedColorFilter, r, CtsEnforcement::kApiLevel_T,
