CVE-2026-5870
Overview
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
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.