Medium chrome Integer Overflow 📄 Reporter bug report 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInteger overflow in Skia
DescriptionInteger overflow in Skia
ComponentSkia
Bug ClassInteger Overflow
Tracker500505046
Fix commit82e20c2da974 (skia) +26/-7
CISA KEVNot listed
CreditedGoogle
Disclosed2026-06-30

Changed Functions

FunctionChangeNotes
if
src/gpu/ganesh/SurfaceContext.cpp
modified
for
src/gpu/ganesh/SurfaceContext.cpp
modified

Files Changed

  • src/gpu/ganesh/SurfaceContext.cpp
From 82e20c2da974f5eeceb06ef92b2a404b7a74ce47 Mon Sep 17 00:00:00 2001
From: Greg Daniel <egdaniel@google.com>
Date: Mon, 04 May 2026 16:02:40 +0000
Subject: [PATCH] Fix potential integer overflows in SurfaceContext using SkSafeMath

Bug: b/500505046
Change-Id: I68462f3a73f2e1491151c7b9c5fc68fadb9d04a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1223518
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
---

diff --git a/src/gpu/ganesh/SurfaceContext.cpp b/src/gpu/ganesh/SurfaceContext.cpp
index 6eaa922..c6fa47f 100644
--- a/src/gpu/ganesh/SurfaceContext.cpp
+++ b/src/gpu/ganesh/SurfaceContext.cpp
@@ -26,6 +26,7 @@
 #include "include/private/base/SkTemplates.h"
 #include "include/private/base/SkTo.h"
 #include "include/private/gpu/ganesh/GrTypesPriv.h"
+#include "src/base/SkSafeMath.h"
 #include "src/core/SkColorSpaceXformSteps.h"
 #include "src/core/SkMipmap.h"
 #include "src/core/SkTraceEvent.h"
@@ -276,7 +277,11 @@
                             this->colorInfo().refColorSpace(),
                             dst.dimensions());
         size_t tmpRB = tmpInfo.minRowBytes();
-        size_t size = tmpRB * tmpInfo.height();
+        SkSafeMath safe;
+        size_t size = safe.mul(tmpRB, tmpInfo.height());
+        if (!safe.ok()) {
+            return false;
+        }
         // Chrome MSAN bots require the data to be initialized (hence the ()).
         tmpPixels = std::make_unique<char[]>(size);
         tmp = {tmpInfo, tmpPixels.get(), tmpRB};
@@ -536,16 +541,24 @@
     bool mustBeTight = !caps->writePixelsRowBytesSupport();
     size_t tmpSize = 0;
     if (mustBeTight || convertAll) {
+        SkSafeMath safe;
         for (int i = 0; i < numLevels; ++i) {
             if (convertAll || (mustBeTight && src[i].rowBytes() != src[i].info().minRowBytes())) {
-                tmpSize += src[i].info().makeColorType(allowedColorType).minRowBytes()*
-                           src[i].height();
+                size_t minRowBytes = src[i].info().makeColorType(allowedColorType).minRowBytes();
+                size_t levelSize = safe.mul(minRowBytes, src[i].height());
+                tmpSize = safe.add(tmpSize, levelSize);
             }
         }
+        if (!safe.ok()) {
+            return false;
+        }
     }
 
     auto tmpData = tmpSize ? SkData::MakeUninitialized(tmpSize) : nullptr;
-    void*    tmp = tmpSize ? tmpData->writable_data()           : nullptr;
+    if (tmpSize && !tmpData) {
+        return false;
+    }
+    void* tmp = tmpSize ? tmpData->writable_data() : nullptr;
     AutoSTArray<15, GrMipLevel> srcLevels(numLevels);
     bool ownAllStorage = true;
     for (int i = 0; i < numLevels; ++i) {
@@ -1353,9 +1366,15 @@
         return {};
     }
 
-    size_t rowBytes = GrColorTypeBytesPerPixel(supportedRead.fColorType) * rect.width();
-    rowBytes = SkAlignTo(rowBytes, this->caps()->transferBufferRowBytesAlignment());
-    size_t size = rowBytes * rect.height();
+    SkSafeMath safe;
+    size_t bytesPerPixel = GrColorTypeBytesPerPixel(supportedRead.fColorType);
+    size_t rowBytes = safe.mul(bytesPerPixel, rect.width());
+    size_t maxTransAlignment = this->caps()->transferBufferRowBytesAlignment();
+    rowBytes = safe.alignUp(rowBytes, maxTransAlignment);
+    size_t size = safe.mul(rowBytes, rect.height());
+    if (!safe.ok()) {
+        return {};
+    }
     // By using kStream_GrAccessPattern here, we are not able to cache and reuse the buffer for
     // multiple reads. Switching to kDynamic_GrAccessPattern would allow for this, however doing
     // so causes a crash in a chromium test. See skbug.com/40042671
Loading diff…

Original Bug Report

reported by vm...@google.com

Potential Integer Overflow in Skia SurfaceContext::readPixels leads to Heap OOB Access

Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the security team.

Overview: A 32-bit integer overflow in Skia’s Ganesh backend can trigger a zero-sized heap allocation during pixel readback color conversion. This leads to a massive out-of-bounds read or write of up to 4 GiB on 32-bit platforms. This issue could potentially be exploited by a compromised renderer to leak GPU process memory or cause memory corruption.

Affected files:

  • third_party/skia/src/gpu/ganesh/SurfaceContext.cpp

Estimated timestamp from git blame: 2021-04-02

Summary

A potential integer overflow vulnerability exists in SurfaceContext::readPixels within Skia’s Ganesh backend. When reading pixels from a large surface that requires an intermediate color conversion, the calculation for the temporary CPU buffer size can overflow on 32-bit architectures. This results in a zero-sized heap allocation followed by a massive 4 GiB out-of-bounds (OOB) memory access, which can lead to a Denial of Service, potential Information Leak, or OOB write in the GPU process.

Technical Details

In third_party/skia/src/gpu/ganesh/SurfaceContext.cpp, if a pixel readback requires format conversion, a temporary buffer is allocated:

size_t tmpRB = tmpInfo.minRowBytes();
size_t size = tmpRB * tmpInfo.height();
// Chrome MSAN bots require the data to be initialized (hence the ()).
tmpPixels = std::make_unique<char[]>(size);

On 32-bit systems, size_t is 32 bits wide. If the readback requires a high-precision fallback format (e.g., kRGBA_F32, which is 16 bytes per pixel) and the surface dimensions are extremely large (e.g., 16384x16384), the size calculation overflows:

  • tmpRB = 16 bytes/pixel * 16384 = 262,144 bytes.
  • size = 262,144 * 16384 = 4,294,967,296.
  • 4,294,967,296 % 2^32 = 0 (on 32-bit systems).

std::make_unique<char[]>(0) allocates a minimal valid heap buffer. Following this, GrGLGpu::readPixels is called to write 4 GiB of data into this tiny allocation.

If the OpenGL driver blindly attempts to copy the data, it causes a 4 GiB OOB write. If the driver rejects the massive read and returns an error, Skia ignores the OpenGL error state and proceeds to GrConvertPixels. GrConvertPixels then attempts to read 4 GiB of data from this 0-sized tmpPixels buffer to convert and write 1 GiB into the renderer-supplied SharedMemory, causing a massive OOB read and Information Leak.

Potential Attacker Steps

(Note: These are suggested steps based on code analysis; we do not currently have a working proof-of-concept.)

  1. Allocate Surface: A compromised renderer process creates a SharedImage with dimensions 16384 x 16384 and format R16F (16-bit float red). This requires ~512 MiB of GPU memory and is permitted on GPUs where GL_MAX_TEXTURE_SIZE >= 16384.
  2. Request Readback: The renderer requests a pixel readback of this SharedImage to RGBA_8888 (4 bytes per pixel) via IPC, providing a 1 GiB SharedMemory region for the destination.
  3. Bypass Decoder Checks: The GPU process validates the 1 GiB destination size against UINT32_MAX (4 GiB). The check passes.
  4. Trigger Overflow: The request reaches Skia. Because reading R16F to RGBA_8888 requires an intermediate fallback buffer of RGBA_F32 (16 bpp) on GLES, Skia attempts to allocate 16384 * 16384 * 16 bytes. This overflows to 0 on 32-bit systems.
  5. Exploit: The subsequent GPU read or CPU conversion operates on the 0-sized buffer, causing a massive OOB access across the GPU process heap.

Because the linear memory traversal is exactly 4 GiB, it will inevitably hit unmapped memory on a 32-bit system and cause a deterministic crash (DoS). However, an attacker might be able to exfiltrate leaked GPU memory through the SharedMemory before the crash occurs.

Suggested Fix

The size calculation in SurfaceContext::readPixels and related functions (like SurfaceContext::internalWritePixels) must be protected against integer overflow.

Instead of manual multiplication, Skia should use safe math or rely on SkImageInfo::computeByteSize, which already contains overflow checks:

size_t tmpRB = tmpInfo.minRowBytes();
size_t size = tmpInfo.computeByteSize(tmpRB);
if (size == SIZE_MAX) {
    return false; // Overflow occurred
}
tmpPixels = std::make_unique<char[]>(size);

This ensures that any dimensions resulting in an overflow are cleanly rejected before memory allocation occurs.

Evaluated with Chrome root at commit: 137d451a126685dd5010e6609db9f6d4a78d8234


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. And please feel free to reach out to me directly if you have concerns or feedback on the project.

View on issue tracker