Chrome · Compositing › Internals>Compositing
CVE-2026-103621
Integer Overflow in Compositing (Internals>Compositing)
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Files Changed
gpu/command_buffer/common/gles2_cmd_format_test_autogen.hgpu/command_buffer/raster_cmd_buffer_functions.txtgpu/command_buffer/common/raster_cmd_format_autogen.hgpu/command_buffer/service/raster_decoder.cccc/paint/paint_op_reader.ccgpu/command_buffer/service/raster_decoder_autogen.hgpu/command_buffer/build_cmd_buffer_lib.pygpu/command_buffer/client/raster_cmd_helper_autogen.hcc/paint/paint_op_writer.cccc/paint/paint_cache_unittest.ccgpu/command_buffer/client/raster_implementation.cccc/paint/paint_cache.hgpu/command_buffer/common/raster_cmd_format_test_autogen.h
Patch
From 7ff661871690f30cb1912652aaaab2eadc23903f Mon Sep 17 00:00:00 2001 From: Thomas Smith <[email protected]> Date: Wed, 23 Sep 2026 11:26:47 -0700 Subject: [PATCH] cc, gpu: Widen PaintCacheId and paint cache deletion IPC to uint64_t Skia is widening SkPath generation IDs from 32-bit to 64-bit. This CL prepares Chromium to consume 64-bit IDs by widening PaintCacheId and its associated serialization and GPU command buffer purging pipeline. Bug: 562038679 Bug: 556268833 Change-Id: I6546900f138050327229c14db45c3c70b1e9640e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8429709 Reviewed-by: Kyle Charbonneau <[email protected]> Reviewed-by: Robert Phillips <[email protected]> Commit-Queue: Thomas Smith <[email protected]> Cr-Commit-Position: refs/heads/main@{#1703606} --- diff --git a/cc/paint/paint_cache.h b/cc/paint/paint_cache.h index 6183f22..19349115 100644 --- a/cc/paint/paint_cache.h +++ b/cc/paint/paint_cache.h @@ -38,7 +38,7 @@ // since it avoids the need for cross-process ref-counting required by the // TransferCache. -using PaintCacheId = uint32_t; +using PaintCacheId = uint64_t; using PaintCacheIds = std::vector<PaintCacheId>; enum class PaintCacheDataType : uint32_t { kPath, diff --git a/cc/paint/paint_op_reader.cc b/cc/paint/paint_op_reader.cc index 006362e..53be3b7 100644 --- a/cc/paint/paint_op_reader.cc +++ b/cc/paint/paint_op_reader.cc @@ -300,8 +300,8 @@ } void PaintOpReader::Read(SkPath* path) { - uint32_t path_id; - ReadSimple(&path_id); + PaintCacheId path_id; + Read(&path_id); if (!valid_) return; diff --git a/cc/paint/paint_op_writer.cc b/cc/paint/paint_op_writer.cc index 706d314..b6e1151 100644 --- a/cc/paint/paint_op_writer.cc +++ b/cc/paint/paint_op_writer.cc @@ -298,7 +298,7 @@ } void PaintOpWriter::Write(const SkPath& path, UsePaintCache use_paint_cache) { - auto id = path.getGenerationID(); + PaintCacheId id = path.getGenerationID(); Write(id); DCHECK(use_paint_cache == UsePaintCache::kEnabled || diff --git a/gpu/command_buffer/build_cmd_buffer_lib.py b/gpu/command_buffer/build_cmd_buffer_lib.py index bd18d16..05addec 100644 --- a/gpu/command_buffer/build_cmd_buffer_lib.py +++ b/gpu/command_buffer/build_cmd_buffer_lib.py @@ -2482,10 +2482,12 @@ def WriteGetDataSizeCode(self, func, arg, f): """Overrriden from TypeHandler.""" code = """ uint32_t %(data_size)s; - if (!base::CheckMul(n, sizeof(GLuint)).AssignIfValid(&%(data_size)s)) { + if (!base::CheckMul(n, sizeof(%(element_type)s)) + .AssignIfValid(&%(data_size)s)) { return error::kOutOfBounds; } -""" % {'data_size': arg.GetReservedSizeId()} +""" % {'data_size': arg.GetReservedSizeId(), + 'element_type': arg.GetPointedType()} f.write(code) def WriteGLES2ImplementationUnitTest(self, func, f): @@ -2629,11 +2631,13 @@ """ f.write(code % args) - def WriteImmediateCmdComputeSize(self, _func, f): + def WriteImmediateCmdComputeSize(self, func, f): """Overrriden from TypeHandler.""" + element_type = func.GetLastOriginalArg().GetPointedType() f.write(" static uint32_t ComputeDataSize(GLsizei _n) {\n") f.write( - " return static_cast<uint32_t>(sizeof(GLuint) * _n); // NOLINT\n") + " return static_cast<uint32_t>(sizeof(%s) * _n); // NOLINT\n" % + element_type) f.write(" }\n") f.write("\n") f.write(" static uint32_t ComputeSize(GLsizei _n) {\n") @@ -2700,8 +2704,9 @@ def WriteImmediateFormatTest(self, func, f): """Overrriden from TypeHandler.""" + element_type = func.GetLastOriginalArg().GetPointedType() f.write("TEST_F(%sFormatTest, %s) {\n" % (_prefix, func.name)) - f.write(" static GLuint ids[] = { 12, 23, 34, };\n") + f.write(" static %s ids[] = { 12, 23, 34, };\n" % element_type) f.write(" cmds::%s& cmd = *GetBufferAs<cmds::%s>();\n" % (func.name, func.name)) f.write(" void* next_cmd = cmd.Set(\n") @@ -2710,12 +2715,15 @@ func.name) f.write(" cmd.header.command);\n") f.write(" EXPECT_EQ(sizeof(cmd) +\n") - f.write(" RoundSizeToMultipleOfEntries(cmd.n * 4u),\n") + f.write(" RoundSizeToMultipleOfEntries(cmd.n * sizeof(%s)),\n" % + element_type) f.write(" cmd.header.size * 4u);\n") f.write(" EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n);\n"); f.write(" CheckBytesWrittenMatchesExpectedSize(\n") f.write(" next_cmd, sizeof(cmd) +\n") - f.write(" RoundSizeToMultipleOfEntries(std::size(ids) * 4u));\n") + f.write(" RoundSizeToMultipleOfEntries(" + "std::size(ids) * sizeof(%s)));\n" % + element_type) f.write(" EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd),\n") f.write(" sizeof(ids))));\n") f.write("}\n") diff --git a/gpu/command_buffer/client/raster_implementation.cc b/gpu/command_buffer/client/raster_implementation.cc index 515a5baa..8edfa91 100644 --- a/gpu/command_buffer/client/raster_implementation.cc +++ b/gpu/command_buffer/client/raster_implementation.cc @@ -108,7 +108,7 @@ if (ids.size() <= IMMEDIATE_SIZE_CONSTANT) { \ helper_->IMMEDIATE_FUNCTION(ids.size(), ids.data()); \ } else { \ - size_t data_size = ids.size() * sizeof(GLuint); \ + size_t data_size = ids.size() * sizeof(cc::PaintCacheId); \ ScopedSharedMemoryPtr dest(data_size, transfer_buffer_, \ mapped_memory_.get(), helper()); \ if (dest.valid()) { \ diff --git a/gpu/command_buffer/raster_cmd_buffer_functions.txt b/gpu/command_buffer/raster_cmd_buffer_functions.txt index 717f4fe..75621f3 100644 --- a/gpu/command_buffer/raster_cmd_buffer_functions.txt +++ b/gpu/command_buffer/raster_cmd_buffer_functions.txt @@ -28,8 +28,8 @@ GL_APICALL void GL_APIENTRY glCreateTransferCacheEntryINTERNAL (GLuint entry_type, GLuint entry_id, GLuint handle_shm_id, GLuint handle_shm_offset, GLuint data_shm_id, GLuint data_shm_offset, GLuint data_size); GL_APICALL void GL_APIENTRY glDeleteTransferCacheEntryINTERNAL (GLuint entry_type, GLuint entry_id); GL_APICALL void GL_APIENTRY glUnlockTransferCacheEntryINTERNAL (GLuint entry_type, GLuint entry_id); -GL_APICALL void GL_APIENTRY glDeletePaintCachePathsINTERNAL (GLsizeiNotNegative n, const GLuint* ids); -GL_APICALL void GL_APIENTRY glDeletePaintCacheEffectsINTERNAL (GLsizeiNotNegative n, const GLuint* ids); +GL_APICALL void GL_APIENTRY glDeletePaintCachePathsINTERNAL (GLsizeiNotNegative n, const GLuint64* ids); +GL_APICALL void GL_APIENTRY glDeletePaintCacheEffectsINTERNAL (GLsizeiNotNegative n, const GLuint64* ids); GL_APICALL void GL_APIENTRY glClearPaintCacheINTERNAL (void); // TOOD(backer): Remove GL encoding. These are not GL functions. diff --git a/gpu/command_buffer/service/raster_decoder.cc b/gpu/command_buffer/service/raster_decoder.cc index b9a44b8..7fbcc40b 100644 --- a/gpu/command_buffer/service/raster_decoder.cc +++ b/gpu/command_buffer/service/raster_decoder.cc @@ -792,10 +792,10 @@ void RestoreStateForAttrib(GLuint attrib, bool restore_array_binding); void DeletePaintCachePathsINTERNALHelper( GLsizei n, - const volatile GLuint* paint_cache_ids); + const volatile GLuint64* paint_cache_ids); void DeletePaintCacheEffectsINTERNALHelper( GLsizei n, - const volatile GLuint* paint_cache_ids); + const volatile GLuint64* paint_cache_ids); void DoClearPaintCacheINTERNAL(); #if defined(NDEBUG) @@ -2785,13 +2785,13 @@ void RasterDecoderImpl::DeletePaintCachePathsINTERNALHelper( GLsizei n, - const volatile GLuint* paint_cache_ids) { + const volatile GLuint64* paint_cache_ids) { paint_cache_->Purge(cc::PaintCacheDataType::kPath, n, paint_cache_ids); } void RasterDecoderImpl::DeletePaintCacheEffectsINTERNALHelper( GLsizei n, - const volatile GLuint* paint_cache_ids) { + const volatile GLuint64* paint_cache_ids) { paint_cache_->Purge(cc::PaintCacheDataType::kSkRuntimeEffect, n, paint_cache_ids); } diff --git a/cc/paint/paint_cache_unittest.cc b/cc/paint/paint_cache_unittest.cc index 3e3f8df..81df359a 100644 --- a/cc/paint/paint_cache_unittest.cc +++ b/cc/paint/paint_cache_unittest.cc @@ -91,7 +91,7 @@ switch (GetType()) { case PaintCacheDataType::kPath: { auto path = CreatePath(); - auto id = path.getGenerationID(); + PaintCacheId id = path.getGenerationID(); SkPath cached_path; EXPECT_EQ(false, service_cache.GetPath(id, &cached_path)); service_cache.PutPath(id, path); @@ -104,7 +104,7 @@ } break; case PaintCacheDataType::kSkRuntimeEffect: { auto effect = GetEffect(); - auto id = base::PersistentHash(effect->source()); + PaintCacheId id = base::PersistentHash(effect->source()); sk_sp<SkRuntimeEffect> cached_effect = nullptr; EXPECT_FALSE(service_cache.GetEffect(id, &cached_effect)); service_cache.PutEffect(id, effect); diff --git a/gpu/command_buffer/client/raster_cmd_helper_autogen.h b/gpu/command_buffer/client/raster_cmd_helper_autogen.h index 002ed1d..66c4c687 100644 --- a/gpu/command_buffer/client/raster_cmd_helper_autogen.h +++ b/gpu/command_buffer/client/raster_cmd_helper_autogen.h @@ -158,7 +158,7 @@ } } -void DeletePaintCachePathsINTERNALImmediate(GLsizei n, const GLuint* ids) { +void DeletePaintCachePathsINTERNALImmediate(GLsizei n, const GLuint64* ids) { const uint32_t size = raster::cmds::DeletePaintCachePathsINTERNALImmediate::ComputeSize(n); raster::cmds::DeletePaintCachePathsINTERNALImmediate* c = @@ -179,7 +179,7 @@ } } -void DeletePaintCacheEffectsINTERNALImmediate(GLsizei n, const GLuint* ids) { +void DeletePaintCacheEffectsINTERNALImmediate(GLsizei n, const GLuint64* ids) { const uint32_t size = raster::cmds::DeletePaintCacheEffectsINTERNALImmediate::ComputeSize(n); raster::cmds::DeletePaintCacheEffectsINTERNALImmediate* c = diff --git a/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h b/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h index 7b342948..ee6026d 100644 --- a/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h +++ b/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h @@ -736,12 +736,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteBuffersImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -757,12 +757,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteFramebuffersImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -788,12 +788,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteRenderbuffersImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -809,12 +809,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteSamplersImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -850,12 +850,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteTexturesImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -872,12 +872,12 @@ EXPECT_EQ( static_cast<uint32_t>(cmds::DeleteTransformFeedbacksImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -3990,12 +3990,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteQueriesEXTImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } @@ -4137,12 +4137,12 @@ void* next_cmd = cmd.Set(&cmd, static_cast<GLsizei>(std::size(ids)), ids); EXPECT_EQ(static_cast<uint32_t>(cmds::DeleteVertexArraysOESImmediate::kCmdId), cmd.header.command); - EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * 4u), + EXPECT_EQ(sizeof(cmd) + RoundSizeToMultipleOfEntries(cmd.n * sizeof(GLuint)), cmd.header.size * 4u); EXPECT_EQ(static_cast<GLsizei>(std::size(ids)), cmd.n); CheckBytesWrittenMatchesExpectedSize( - next_cmd, - sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * 4u)); + next_cmd, sizeof(cmd) + RoundSizeToMultipleOfEntries(std::size(ids) * + sizeof(GLuint))); EXPECT_EQ(0, UNSAFE_TODO(memcmp(ids, ImmediateDataAddress(&cmd), sizeof(ids)))); } diff --git a/gpu/command_buffer/common/raster_cmd_format_autogen.h b/gpu/command_buffer/common/raster_cmd_format_autogen.h index 1142810..8a692789 100644 --- a/gpu/command_buffer/common/raster_cmd_format_autogen.h +++ b/gpu/command_buffer/common/raster_cmd_format_autogen.h @@ -702,7 +702,7 @@ static const uint8_t cmd_flags = CMD_FLAG_SET_TRACE_LEVEL(3); static uint32_t ComputeDataSize(GLsizei _n) { - return static_cast<uint32_t>(sizeof(GLuint) * _n); // NOLINT + return static_cast<uint32_t>(sizeof(GLuint64) * _n); // NOLINT } static uint32_t ComputeSize(GLsizei _n) { @@ -714,13 +714,13 @@ header.SetCmdByTotalSize<ValueType>(ComputeSize(_n)); } - void Init(GLsizei _n, const GLuint* _ids) { + void Init(GLsizei _n, const GLuint64* _ids) { SetHeader(_n); n = _n; UNSAFE_TODO(memcpy(ImmediateDataAddress(this), _ids, ComputeDataSize(_n))); } - void* Set(void* cmd, GLsizei _n, const GLuint* _ids) { + void* Set(void* cmd, GLsizei _n, const GLuint64* _ids) { static_cast<ValueType*>(cmd)->Init(_n, _ids); const uint32_t size = ComputeSize(_n); return NextImmediateCmdAddressTotalSize<ValueType>(cmd, size); @@ -790,7 +790,7 @@ static const uint8_t cmd_flags = CMD_FLAG_SET_TRACE_LEVEL(3); static uint32_t ComputeDataSize(GLsizei _n) { - return static_cast<uint32_t>(sizeof(GLuint) * _n); // NOLINT + return static_cast<uint32_t>(sizeof(GLuint64) * _n); // NOLINT } static uint32_t ComputeSize(GLsizei _n) { @@ -802,13 +802,13 @@ header.SetCmdByTotalSize<ValueType>(ComputeSize(_n)); ... (file diff truncated)
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/cc/paint/paint_cache_unittest.cc b/cc/paint/paint_cache_unittest.cc
index 3e3f8df..81df359a 100644
--- a/cc/paint/paint_cache_unittest.cc
+++ b/cc/paint/paint_cache_unittest.cc
@@ -91,7 +91,7 @@
switch (GetType()) {
case PaintCacheDataType::kPath: {
auto path = CreatePath();
- auto id = path.getGenerationID();
+ PaintCacheId id = path.getGenerationID();
SkPath cached_path;
EXPECT_EQ(false, service_cache.GetPath(id, &cached_path));
service_cache.PutPath(id, path);
@@ -104,7 +104,7 @@
} break;
case PaintCacheDataType::kSkRuntimeEffect: {
auto effect = GetEffect();
- auto id = base::PersistentHash(effect->source());
+ PaintCacheId id = base::PersistentHash(effect->source());
sk_sp<SkRuntimeEffect> cached_effect = nullptr;
EXPECT_FALSE(service_cache.GetEffect(id, &cached_effect));
service_cache.PutEffect(id, effect);
Loading diff…
Original Bug Report
reported by [email protected]
Cross-origin SVG leak via SkPath 30-bit generation ID wraparound in OOP raster cache
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. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: Skia generates SkPath generation IDs using a 30-bit counter without wraparound protection, which Chromium’s OOP-raster paint cache uses as the sole identifier across the renderer-to-GPU process boundary. After 2^30 path allocations, generation IDs collide with existing cached paths, potentially allowing stale path geometry in the GPU process to be substituted during subsequent rasterization. This could allow an unprivileged web page to extract cross-origin SVG vector geometry into an origin-clean canvas.
Affected files:
cc/paint/paint_op_reader.cccc/paint/paint_op_writer.cccc/paint/paint_cache.cccc/paint/paint_cache.hthird_party/skia/src/core/SkPathData.cpp
Estimated timestamp from git blame: 2018-11-08
Summary
Skia generates unique path generation IDs using a 30-bit atomic counter that truncates the top 2 bits. In Chromium, OOP-raster serialization (cc::PaintOpWriter / cc::PaintOpReader) uses this generation ID as the sole identity key for cached SkPath objects in both cc::ClientPaintCache (renderer) and cc::ServicePaintCache (GPU process).
Because the counter is 30-bit and lacks wraparound protection, allocating $2^{30}$ paths causes IDs to cycle. If a cached victim path is retained across the wraparound, subsequent paint ops utilizing a newly minted path with the colliding ID will hit the cache (kCached), causing the GPU process to rasterize the stale victim path instead. Because canvas origin-cleanliness is tracked strictly in Blink, drawing a colliding path into a clean canvas and reading it via getImageData() may permit unauthorized extraction of cross-origin SVG vector geometry.
Note: These findings are based on static code analysis. Automated execution and verification have not been performed by our tooling agent, so these steps should be considered potential attack vectors.
Root Cause Analysis
- 30-bit Skia Counter (
third_party/skia/src/core/SkPathData.cpp:32-45):
static uint32_t next_pathdata_unique_id() {
constexpr int kHighBitsToMakeRoomForFillType = 2;
static std::atomic<int32_t> nextID{1};
uint32_t id;
do {
id = nextID.fetch_add(1, std::memory_order_relaxed);
id <<= kHighBitsToMakeRoomForFillType; // clear the high bits
id >>= kHighBitsToMakeRoomForFillType;
} while (id == 0);
return id;
}
Every SkPathData allocation queries this counter (SkPathData.cpp:148). SkPath::getGenerationID() (SkPath.cpp:94) returns the bare 30-bit fUniqueID.
- ID as Sole Cache Key in OOP-Raster (
cc/paint/paint_op_writer.cc:292-302):
void PaintOpWriter::Write(const SkPath& path, UsePaintCache use_paint_cache) {
auto id = path.getGenerationID();
Write(id);
...
if (use_paint_cache == UsePaintCache::kEnabled &&
options_.paint_cache->Get(PaintCacheDataType::kPath, id)) {
Write(static_cast<uint32_t>(PaintCacheEntryState::kCached));
return; // Serializes only the ID, no path data or hash
}
- Fail-Open Service Cache Lookup (
cc/paint/paint_op_reader.cc:312-316andcc/paint/paint_cache.cc:103-109): When the GPU deserializesPaintCacheEntryState::kCached,ServicePaintCache::GetPath(path_id, path)retrieves the path fromcached_paths_(std::map<PaintCacheId, SkPath>) without verifying bounds, point counts, or content hashes.
Potential Trigger Steps
- Load Cross-Origin Target: An attacker page loads an authenticated cross-origin SVG image
<img src="https://victim.example/secret_chart.svg">and draws it onto an offscreen accelerated<canvas>$T$. Canvas $T$ is marked tainted by Blink (origin_clean_ = false), preventing direct script readback. - Path Caching: The SVG’s shape paths are recorded as
DrawPathOps (UsePaintCache::kEnabled) and serialized to the GPU process asPaintCacheEntryState::kInlined, populatingClientPaintCacheandServicePaintCachewith the victim path under generation ID $G_v$. - Counter Pumping: The attacker spawns Web Workers that repeatedly create dummy paths (e.g.,
new Path2D('M0 0h1')). BecausenextIDis process-global and shared across threads, $2^{30}$ allocations (~1.07 billion iterations) can be completed in parallel in a few minutes, causing the counter to wrap around to $1$ and approach $G_v$. - Clean Canvas Probe: The attacker creates a fresh canvas $F$ (
origin_clean_ = true) and draws a same-origin probe SVG containing paths. When a probe path is allocated with generation ID $G_v$,PaintOpWriter::Writedetects a cache hit inClientPaintCacheand sendskCachedwith ID $G_v$. - GPU Substitution: The GPU process retrieves the cached victim path associated with $G_v$ and rasterizes it into canvas $F$’s backing buffer.
- Readback: The attacker calls
ctxF.getImageData(0, 0, width, height). Because canvas $F$ only drew same-origin content at the Blink layer, the origin check inBaseRenderingContext2D::getImageDatapasses, returning the victim’s rasterized vector geometry.
Suggested Fix
- Use 64-bit Identifiers: Avoid using Skia’s 30-bit
getGenerationID()directly as the wire identity across the IPC boundary. Instead, assign unique 64-bit IDs inccfor cached paint objects. - Wraparound Guard in Skia: Add wraparound handling or a fallback in
next_pathdata_unique_id()to prevent silent cyclic reuse of IDs. - Content Validation: Validate basic structural characteristics (such as verb/point counts and bounding boxes) or compare content hashes in
ServicePaintCachebefore acceptingkCachedentries.
Evaluated with Chrome root at commit: 53c437ac4ea52cc89d0265eb9e6d68666fdb303e
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
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.
References
On This Page