From 9c5eadaba1024495f9f59d98fd9dab820ba5868d Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Mon, 6 Jun 2022 16:09:50 -0500 Subject: [MozSec] Bug 1767365 - Clip image data transfers. --- dom/canvas/CanvasRenderingContext2D.cpp | 31 ++++++++++++------ system/graphics/2d/BaseRect.h | 58 +++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp index 7d00b2cb2..8cb14a1f3 100644 --- a/dom/canvas/CanvasRenderingContext2D.cpp +++ b/dom/canvas/CanvasRenderingContext2D.cpp @@ -5745,6 +5745,18 @@ inline uint8_t PoisonValue(uint8_t v) return v + rand() %3 -1; } +static IntRect ClipImageDataTransfer(IntRect& aSrc, + const IntPoint& aDestOffset, + const IntSize& aDestBounds) +{ + IntRect dest = aSrc; + dest.SafeMoveBy(aDestOffset); + dest = IntRect(IntPoint(0, 0), aDestBounds).SafeIntersect(dest); + + aSrc = aSrc.SafeIntersect(dest - aDestOffset); + return aSrc + aDestOffset; +} + nsresult CanvasRenderingContext2D::GetImageDataArray(JSContext* aCx, int32_t aX, @@ -5785,9 +5797,11 @@ CanvasRenderingContext2D::GetImageDataArray(JSContext* aCx, return NS_OK; } - IntRect srcRect(0, 0, mWidth, mHeight); - IntRect destRect(aX, aY, aWidth, aHeight); - IntRect srcReadRect = srcRect.Intersect(destRect); + IntRect dstWriteRect(0, 0, aWidth, aHeight); + IntRect srcReadRect = ClipImageDataTransfer(dstWriteRect, + IntPoint(aX, aY), + IntSize(mWidth, mHeight)); + RefPtr readback; DataSourceSurface::MappedSurface rawData; if (!srcReadRect.IsEmpty()) { @@ -5815,9 +5829,6 @@ CanvasRenderingContext2D::GetImageDataArray(JSContext* aCx, } } - IntRect dstWriteRect = srcReadRect; - dstWriteRect.MoveBy(-aX, -aY); - JS::AutoCheckCannotGC nogc; bool isShared; uint8_t* data = JS_GetUint8ClampedArrayData(darray, &isShared, nogc); @@ -6018,10 +6029,10 @@ CanvasRenderingContext2D::PutImageData_explicit(int32_t aX, int32_t aY, uint32_t dirtyRect = imageDataRect; } - dirtyRect.MoveBy(IntPoint(aX, aY)); - dirtyRect = IntRect(0, 0, mWidth, mHeight).Intersect(dirtyRect); - - if (dirtyRect.Width() <= 0 || dirtyRect.Height() <= 0) { + IntRect srcRect = dirtyRect; + dirtyRect = ClipImageDataTransfer(srcRect, IntPoint(aX, aY), + IntSize(mWidth, mHeight)); + if (dirtyRect.IsEmpty()) { return NS_OK; } diff --git a/system/graphics/2d/BaseRect.h b/system/graphics/2d/BaseRect.h index b1eed9ddb..a4e5e4c1d 100644 --- a/system/graphics/2d/BaseRect.h +++ b/system/graphics/2d/BaseRect.h @@ -107,6 +107,10 @@ struct BaseRect { // (including edges) of *this and aRect. If there are no points in that // intersection, returns an empty rectangle with x/y set to the std::max of the x/y // of *this and aRect. + // + // Intersection with an empty Rect may not produce an empty Rect if overflow + // occurs. e.g. {INT_MIN, 0, 0, 20} Intersect { 5000, 0, 500, 20 } gives: + // the non-emtpy {5000, 0, 500, 20 } instead of {5000, 0, 0, 0} MOZ_MUST_USE Sub Intersect(const Sub& aRect) const { Sub result; @@ -119,6 +123,26 @@ struct BaseRect { } return result; } + // Gives the same results as Intersect() but handles integer overflow + // better. This comes at a tiny cost in performance. + // e.g. {INT_MIN, 0, 0, 20} Intersect { 5000, 0, 500, 20 } gives: + // {5000, 0, 0, 0} + MOZ_MUST_USE Sub SafeIntersect(const Sub& aRect) const + { + Sub result; + result.x = std::max(x, aRect.x); + result.y = std::max(y, aRect.y); + T right = std::min(x + width, aRect.x + aRect.width); + T bottom = std::min(y + height, aRect.y + aRect.height); + if (right < result.x || bottom < result.y) { + result.width = 0; + result.height = 0; + } else { + result.width = right - result.x; + result.height = bottom - result.y; + } + return result; + } // Sets *this to be the rectangle containing the intersection of the points // (including edges) of *this and aRect. If there are no points in that // intersection, sets *this to be an empty rectangle with x/y set to the std::max @@ -211,6 +235,40 @@ struct BaseRect { void MoveTo(const Point& aPoint) { x = aPoint.x; y = aPoint.y; } void MoveBy(T aDx, T aDy) { x += aDx; y += aDy; } void MoveBy(const Point& aPoint) { x += aPoint.x; y += aPoint.y; } + + // Variant of MoveBy that ensures that even after translation by a point that + // the rectangle coordinates will still fit within numeric limits. The origin + // and size will be clipped within numeric limits to ensure this. + void SafeMoveByX(T aDx) { + T x2 = XMost(); + if (aDx >= T(0)) { + T limit = std::numeric_limits::max(); + x = limit - aDx < x ? limit : x + aDx; + width = (limit - aDx < x2 ? limit : x2 + aDx) - x; + } else { + T limit = std::numeric_limits::min(); + x = limit - aDx > x ? limit : x + aDx; + width = (limit - aDx > x2 ? limit : x2 + aDx) - x; + } + } + void SafeMoveByY(T aDy) { + T y2 = YMost(); + if (aDy >= T(0)) { + T limit = std::numeric_limits::max(); + y = limit - aDy < y ? limit : y + aDy; + height = (limit - aDy < y2 ? limit : y2 + aDy) - y; + } else { + T limit = std::numeric_limits::min(); + y = limit - aDy > y ? limit : y + aDy; + height = (limit - aDy > y2 ? limit : y2 + aDy) - y; + } + } + void SafeMoveBy(T aDx, T aDy) { + SafeMoveByX(aDx); + SafeMoveByY(aDy); + } + void SafeMoveBy(const Point& aPoint) { SafeMoveBy(aPoint.x, aPoint.y); } + void SizeTo(T aWidth, T aHeight) { width = aWidth; height = aHeight; } void SizeTo(const SizeT& aSize) { width = aSize.width; height = aSize.height; } -- cgit v1.2.3