CVE-2026-5870
Overview
Background
- `SkReadPixelsRec` / `SkWritePixelsRec`
- Skia helper structs that package and trim the parameters of
readPixels()/writePixels()(anSkImageInfo, a pixel pointer,fRowBytes, and anfX/fYoffset into the source/destination). - `trim()`
- the method that clamps the requested rectangle to the valid src/dst bounds; on success it may advance
fPixelsand shrinkfInfo, and it explicitly supports negativefX/fYfor partially off-screen rectangles. - Negative-offset handling
- a negative
fX/fYmeans the rectangle starts off-screen, sotrim()negates the offset to compute how far to advance the pixel pointer (-fX,-fY) and by how much to reduce the sub-rectangle. - `INT_MIN` negation
- negating
INT_MIN(-2147483648) has no representable positive counterpart, so the operation is signed-integer overflow / undefined behavior in C++.
Root Cause Analysis
trim() validates fInfo dimensions (0 >= fInfo.width() || 0 >= fInfo.height()) but then proceeds to use fX/fY, and for negative offsets the subsequent logic negates them to compute the pixel-pointer adjustment and the reduced fInfo. When fX or fY is INT_MIN, that negation is undefined behavior and, combined with the multiplication by fRowBytes/bytes-per-pixel, produces a wrapped offset rather than a correctly rejected out-of-bounds rectangle. The clamp that is supposed to keep the adjusted fPixels inside the buffer is therefore computed from a corrupted value, so the resulting record can point outside the allocation and readPixels/writePixels then performs an out-of-bounds pixel copy (inferred for the exact access).
The fix adds, in both SkReadPixelsRec::trim and SkWritePixelsRec::trim, an early if (fX == INT_MIN || fY == INT_MIN) { return false; } guard placed before the negation, rejecting the pathological inputs; the comment states exactly “negating the largest negative integer (below) is UB”. The accompanying header doc and new PixelsRecTest.cpp (including an “Explicitly trigger y_offset overflow” case using INT_MIN) document and lock in the rejection.
trim() negates a caller-supplied fX/fY to handle off-screen offsets, but INT_MIN cannot be negated without overflow (UB), so the computed offset wrapped and defeated the bounds clamp; guarding against fX/fY == INT_MIN and returning false removes the undefined operation.Attack Path
- Reach a pixel-rect operation
Content drives a Skia
readPixels/writePixelspath (e.g. via canvas image-data operations) that constructs anSkReadPixelsRec/SkWritePixelsRecwith attacker-influencedfX/fYoffsets. - Supply `INT_MIN`
The offset
fXorfYis set toINT_MIN, which passes thefInfodimension check and enters the negative-offset branch. - Overflow the offset math
trim()negatesINT_MIN(UB) and multiplies byfRowBytes, producing a wrappedy_offset/pointer adjustment instead of a correct clamp (the test forces this with a largefRowBytesandfY = INT_MIN). - Out-of-bounds copy
The record ends up describing a region outside the pixel buffer, so the subsequent
readPixels/writePixelsperforms an out-of-bounds read or write(inferred); the fix now returnsfalsebefore any of this runs.
Impact Assessment
x/y offset to a Skia readPixels/writePixels operation can trigger signed-integer-overflow UB in trim() that defeats the bounds clamp, leading to an out-of-bounds pixel access in whichever process runs Skia (renderer or GPU). The vendor advisory rates this high severity and labels it an integer overflow in Skia; it does not report in-the-wild exploitation. The patch demonstrates the overflow/UB (INT_MIN negation); the resulting out-of-bounds access and any escalation to a controlled read/write or code execution are not shown by the diff (inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
ifsrc/core/SkReadPixelsRec.cpp |
modified | |
ifsrc/core/SkWritePixelsRec.cpp |
modified | |
DEF_TESTtests/PixelsRecTest.cpp |
modified |
Files Changed
gn/tests.gnisrc/core/SkReadPixelsRec.cppsrc/core/SkReadPixelsRec.hsrc/core/SkWritePixelsRec.cppsrc/core/SkWritePixelsRec.htests/PixelsRecTest.cpp
Audit Directions
- Unary negation of signed inputsFlag any
-x/std::abs-style negation of an externally suppliedintwhose domain includesINT_MIN, especially when the result then scales a size or pointer offset. - Clamp/trim routines accepting negative offsetsAudit other Skia (and graphics) rectangle-clamping helpers that treat negative coordinates as “off-screen” and derive pointer advances from them, confirming the extreme values are rejected before arithmetic.
- Offset-times-stride computationsReview
offset * rowBytes/offset * bytesPerPixelpatterns for overflow, since a wrapped product can yield an in-range-looking pointer that actually escapes the allocation.
Patch
From d50fd578871fe2cf01efc001b95739c9e53402bb Mon Sep 17 00:00:00 2001 From: Kaylee Lubick <[email protected]> Date: Mon, 30 Mar 2026 14:58:18 +0000 Subject: [PATCH] Add tests and docs to Sk*PixelsRec Follow-up to https://review.skia.org/1194336 Bug: b/495534710 Change-Id: I1d15a9e3c995612aa136570b72b74529e92d29f9 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1198456 Reviewed-by: Florin Malita <[email protected]> Commit-Queue: Kaylee Lubick <[email protected]> --- diff --git a/gn/tests.gni b/gn/tests.gni index 4318311..fb49acf 100644 --- a/gn/tests.gni +++ b/gn/tests.gni @@ -167,6 +167,7 @@ "$_tests/PictureShaderTest.cpp", "$_tests/PictureTest.cpp", "$_tests/PixelRefTest.cpp", + "$_tests/PixelsRecTest.cpp", "$_tests/Point3Test.cpp", "$_tests/PointTest.cpp", "$_tests/PolyUtilsTest.cpp", diff --git a/src/core/SkReadPixelsRec.cpp b/src/core/SkReadPixelsRec.cpp index f7e9661..f3d0a91 100644 --- a/src/core/SkReadPixelsRec.cpp +++ b/src/core/SkReadPixelsRec.cpp @@ -18,6 +18,10 @@ if (0 >= fInfo.width() || 0 >= fInfo.height()) { return false; } + // negating the largest negative integer (below) is UB + if (fX == INT_MIN || fY == INT_MIN) { + return false; + } int x = fX; int y = fY; diff --git a/src/core/SkReadPixelsRec.h b/src/core/SkReadPixelsRec.h index 959b51b..34701ba 100644 --- a/src/core/SkReadPixelsRec.h +++ b/src/core/SkReadPixelsRec.h @@ -17,6 +17,11 @@ * Helper class to package and trim the parameters passed to readPixels() */ struct SkReadPixelsRec { + /** + * @param x, y The offset into the source. Negative values are supported; the portion + * of the rectangle that is "off-screen" (negative x or y) will leave the + * corresponding area in the destination pixels untouched. + */ SkReadPixelsRec(const SkImageInfo& info, void* pixels, size_t rowBytes, int x, int y) : fPixels(pixels) , fRowBytes(rowBytes) @@ -41,7 +46,8 @@ /* * On true, may have modified its fields (except fRowBytes) to make it a legal subset - * of the specified src width/height. + * of the specified src width/height. Negative fX or fY will cause fPixels to be + * incremented and fInfo to be reduced to account for the portion that is "off-screen". * * On false, leaves self unchanged, but indicates that it does not overlap src, or * is not valid (e.g. bad fInfo) for readPixels(). diff --git a/src/core/SkWritePixelsRec.cpp b/src/core/SkWritePixelsRec.cpp index d1bad0f5..9bd09e5 100644 --- a/src/core/SkWritePixelsRec.cpp +++ b/src/core/SkWritePixelsRec.cpp @@ -19,6 +19,10 @@ if (0 >= fInfo.width() || 0 >= fInfo.height()) { return false; } + // negating the largest negative integer (below) is UB + if (fX == INT_MIN || fY == INT_MIN) { + return false; + } int x = fX; int y = fY; diff --git a/src/core/SkWritePixelsRec.h b/src/core/SkWritePixelsRec.h index 6a30498..bfceabf 100644 --- a/src/core/SkWritePixelsRec.h +++ b/src/core/SkWritePixelsRec.h @@ -17,6 +17,11 @@ * Helper class to package and trim the parameters passed to writePixels() */ struct SkWritePixelsRec { + /** + * @param x, y The offset into the destination. Negative values are supported; the portion + * of the rectangle that is "off-screen" (negative x or y) will cause the + * corresponding area in the source pixels to be ignored. + */ SkWritePixelsRec(const SkImageInfo& info, const void* pixels, size_t rowBytes, int x, int y) : fPixels(pixels) , fRowBytes(rowBytes) @@ -41,7 +46,8 @@ /* * On true, may have modified its fields (except fRowBytes) to make it a legal subset - * of the specified dst width/height. + * of the specified dst width/height. Negative fX or fY will cause fPixels to be + * incremented and fInfo to be reduced to account for the portion that is "off-screen". * * On false, leaves self unchanged, but indicates that it does not overlap dst, or * is not valid (e.g. bad fInfo) for writePixels(). diff --git a/tests/PixelsRecTest.cpp b/tests/PixelsRecTest.cpp new file mode 100644 index 0000000..aaf6afd --- /dev/null +++ b/tests/PixelsRecTest.cpp @@ -0,0 +1,222 @@ +/* + * Copyright 2026 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "include/core/SkImageInfo.h" +#include "src/core/SkReadPixelsRec.h" +#include "src/core/SkWritePixelsRec.h" +#include "tests/Test.h" + +#include <climits> +#include <cstddef> +#include <cstdint> +#include <vector> + +DEF_TEST(ReadPixelsRec_trim, reporter) { + constexpr int W = 100; + constexpr int H = 100; + const SkImageInfo info = SkImageInfo::MakeN32Premul(W, H); + const size_t rowBytes = info.minRowBytes(); + std::vector<char> storage(H * rowBytes); + void* pixels = storage.data(); + + { + skiatest::ReporterContext ctx(reporter, "Normal valid trim"); + SkReadPixelsRec rec(info, pixels, rowBytes, 0, 0); + REPORTER_ASSERT(reporter, rec.trim(W, H)); + REPORTER_ASSERT(reporter, rec.fPixels == pixels); + REPORTER_ASSERT(reporter, rec.fInfo.width() == W); + REPORTER_ASSERT(reporter, rec.fInfo.height() == H); + } + + { + skiatest::ReporterContext ctx(reporter, "Trim with negative x, y"); + SkReadPixelsRec rec(info, pixels, rowBytes, -10, -10); + REPORTER_ASSERT(reporter, rec.trim(W, H)); + // fPixels should be adjusted: pixels + 10 * rowBytes + 10 * 4 + REPORTER_ASSERT(reporter, rec.fPixels == + (char*)pixels + 10 * rowBytes + 10 * info.bytesPerPixel()); + REPORTER_ASSERT(reporter, rec.fInfo.width() == W - 10); + REPORTER_ASSERT(reporter, rec.fInfo.height() == H - 10); + REPORTER_ASSERT(reporter, rec.fX == 0); + REPORTER_ASSERT(reporter, rec.fY == 0); + } + + { + skiatest::ReporterContext ctx(reporter, "Trim with x, y partially outside"); + SkReadPixelsRec rec(info, pixels, rowBytes, 50, 50); + REPORTER_ASSERT(reporter, rec.trim(W, H)); + REPORTER_ASSERT(reporter, rec.fPixels == pixels); + REPORTER_ASSERT(reporter, rec.fInfo.width() == 50); + REPORTER_ASSERT(reporter, rec.fInfo.height() == 50); + REPORTER_ASSERT(reporter, rec.fX == 50); + REPORTER_ASSERT(reporter, rec.fY == 50); + } + + { + skiatest::ReporterContext ctx(reporter, "Trim with x, y completely outside (positive)"); + SkReadPixelsRec rec(info, pixels, rowBytes, 150, 150); + REPORTER_ASSERT(reporter, !rec.trim(W, H)); + } + + { + skiatest::ReporterContext ctx(reporter, "Trim with x, y completely outside (negative)"); + SkReadPixelsRec rec(info, pixels, rowBytes, -150, -150); + REPORTER_ASSERT(reporter, !rec.trim(W, H)); + } + + { + skiatest::ReporterContext ctx(reporter, "Explicitly trigger y_offset overflow"); + size_t hugeRowBytes = 0; + int hugeY = 0; + if (sizeof(size_t) == 8) { + hugeRowBytes = (size_t)1 << 35; + hugeY = INT_MIN; + } else { + hugeRowBytes = (size_t)1 << 19; + hugeY = INT_MIN; + } + SkReadPixelsRec rec(info, pixels, hugeRowBytes, 0, hugeY); + REPORTER_ASSERT(reporter, !rec.trim(W, H)); + } + + {
Original Bug Report
Potential Heap OOB Write in Skia `trim` via Canvas `getImageData` Integer Overflow
Project Fortify, an experimental security project, has identified the following potential security issue.
Overview: An integer overflow in Skia’s trim functions allows a negative pointer adjustment when using pixel formats with large bytes-per-pixel values (like rgba-float32). This can be triggered via the Canvas 2D API to achieve a precise, attacker-controlled out-of-bounds memory write in the renderer process.
Affected files:
third_party/skia/src/core/SkReadPixelsRec.cppthird_party/skia/src/core/SkWritePixelsRec.cppthird_party/skia/src/core/SkImageInfoPriv.hthird_party/skia/include/core/SkImageInfo.hthird_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.ccthird_party/blink/renderer/core/html/canvas/image_data.cc
Estimated timestamp from git blame: 2023-02-15
Summary
An integer overflow vulnerability exists in SkReadPixelsRec::trim() and SkWritePixelsRec::trim() in Skia (third_party/skia/src/core/SkReadPixelsRec.cpp and SkWritePixelsRec.cpp). When processing image data with more than 4 bytes per pixel (BPP), such as rgba-float32 (16 BPP), an attacker can supply large negative coordinates that bypass bounds checks and overflow during pointer arithmetic. This results in an out-of-bounds (OOB) write to memory located before the allocated pixel buffer in the sandboxed renderer process.
Vulnerability Details
The vulnerability relies on two key flaws:
-
Guard Bypass in
minRowBytes(): InSkReadPixelsRec::trim(), the first safety check isif (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()). If the attacker requests an extremely wideImageData(e.g., width =268,435,456),SkImageInfo::minRowBytes()calculateswidth * bytesPerPixel(268,435,456 * 16 = 4,294,967,296). Because this exceedsINT32_MAX, an internalSkTFitsIn<int32_t>check fails, andminRowBytes()returns0as an error sentinel. SincefRowBytesis asize_t,fRowBytes < 0evaluates tofalse, completely bypassing the safety guard. -
Pointer Adjustment Integer Overflow: Further down in
trim(), to handle negative coordinates (meaning the read/write starts outside the canvas bounds), the pointer is advanced to the first intersecting pixel:fPixels = ((char*)fPixels + -y*fRowBytes + -x*fInfo.bytesPerPixel());The expression
-x * fInfo.bytesPerPixel()performs signed 32-bit integer multiplication. Ifxis a carefully chosen large negative value (e.g.,-268,435,455),-xis positive268,435,455. Multiplied by16(the BPP ofrgba-float32), the mathematical result is4,294,967,280. In 32-bit two’s complement arithmetic, this overflows and wraps to-16. Adding-16tofPixelsmoves the destination pointer backwards by 16 bytes, placing it out of bounds before the allocated buffer.
Note: This exploit requires allocating an ArrayBuffer larger than 2GB (up to ~4GB for the specific offsets mentioned), which restricts this vulnerability to 64-bit systems where v8::TypedArray::kMaxByteLength allows such massive allocations.
Potential Attack Steps
(Note: These are suggested theoretical steps as our setup cannot execute code to verify the exploit end-to-end.)
An attacker could trigger this vulnerability via JavaScript using the following steps:
- Setup Canvas: Create a 1x1 HTML
<canvas>element to restrict the read/write size to a single pixel (16 bytes), preventing massive memory corruption and ensuring a stable exploit. - Prepare Payload: Fill the canvas with a controlled payload using
ctx.fillStyle = 'rgba(...)'; ctx.fillRect(0, 0, 1, 1);. - Trigger OOB Write: Call
getImageDatawith carefully chosen parameters:// sx = -268435455, sy = 0, sw = 268435456, sh = 1 ctx.getImageData(-268435455, 0, 268435456, 1, { pixelFormat: 'rgba-float32' });swis chosen so thatsx + sw = 1, satisfying the canvas intersection check in Blink.sxis chosen such that-sx * 16wraps around to exactly-16in 32-bit math.
- Result:
getImageDataallocates a ~4GB backing store for theFloat32Array. Skia calculates the-16offset and copies the 16-byte attacker-controlled payload from the canvas exactly 16 bytes before the allocated backing store in the V8 heap (which is highly likely a PartitionAlloc DirectMap allocation). By targeting metadata or adjacent objects, this underflow could be leveraged for renderer RCE.
Suggested Fix
- Fix the pointer arithmetic: Cast the operands to
ptrdiff_torsize_tbefore multiplication in bothSkReadPixelsRec::trim()andSkWritePixelsRec::trim()to prevent 32-bit integer overflow:fPixels = ((char*)fPixels + (ptrdiff_t)-y * fRowBytes + (ptrdiff_t)-x * fInfo.bytesPerPixel()); - Fix the sentinel bypass:
SkImageInfo::minRowBytes()returning0on overflow is dangerous if callers assume it returns a valid minimum size. Update the guards intrim()to explicitly check for the0sentinel or bounds-checkxandyearlier against safer maximums.
Evaluated with Chrome root at commit: 9760e6c70cd33a320713361f17c6dcca85648c0f
Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; data from false positives will be used to improve accuracy over time. Please feel free to reach out to me if you have concerns or feedback.