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: A potential vulnerability in Skia’s Graphite DrawBufferManager can lead to a Use-After-Free or wild write in the GPU process. When a buffer allocation fails mid-recording, stack-local allocators are destructed and can stash stale raw pointers that are subsequently reused. In release builds, failure-handling paths fail to clear these stale allocators, allowing subsequent invalid memory operations.

Affected files:

• third_party/skia/src/gpu/graphite/BufferManager.cpp
• third_party/skia/src/gpu/graphite/BufferManager.h

Estimated timestamp from git blame: 2025-10-13

Root Cause Analysis

In Skia’s Graphite implementation, BufferSubAllocator::reset() (defined in third_party/skia/src/gpu/graphite/BufferManager.cpp) handles returning or stashing a GPU buffer when a sub-allocator goes out of scope. However, it does not properly handle the state where the owning DrawBufferManager has encountered an allocation or mapping failure (fMappingFailed is true).

When an allocation failure occurs, DrawBufferManager::onFailedBuffer() is called. It marks fMappingFailed = true and clears state.fAvailableBuffer (lines 370-375). However, active stack-local BufferSubAllocator instances (held by utilities like UniformTracker or DrawWriter) are unaffected at this stage.

When the failed recording is aborted and the stack unwinds, the destructors of these stack-local allocators are called, invoking BufferSubAllocator::reset(). Inside reset() (lines 282-284), the following condition is evaluated to determine whether to stash the buffer or move it to fUsedBuffers:

else if (state.fAvailableBuffer.fBuffer.get() == fBuffer.get() ||
           this->remainingBytes() < state.fAvailableBuffer.remainingBytes() ||
           this->remainingBytes() < state.fMinAlignment)

Because onFailedBuffer() recently cleared state.fAvailableBuffer, its underlying fBuffer is null, meaning state.fAvailableBuffer.remainingBytes() is 0. Consequently, this->remainingBytes() < state.fAvailableBuffer.remainingBytes() evaluates to false. Execution falls into the else block (line 295):

state.fAvailableBuffer = std::move(*this);

This move assignment copies the raw CPU mapped pointer (fMappedPtr) and the raw transfer buffer pointer (fTransferBuffer) completely unchanged into state.fAvailableBuffer.

In release builds, the safety assert in DrawBufferManager::transferToRecording() is compiled out, allowing transferToRecording() to return false and reset fMappingFailed to false while silently leaving the stale allocator inside fAvailableBuffer (lines 391-403). On a subsequent successful recording session, getBuffer() retrieves this stale allocator and returns it to the caller, leading to a wild write via the dangling fMappedPtr and a potential heap Use-After-Free (UAF) read/downcast on the deleted srcBuffer inside DawnCommandBuffer::onCopyBufferToBuffer (line 1232).

Additionally, any stack-local allocators that execute the else if branch of reset() will push their stale references to fUsedBuffers (line 291). Because transferToRecording() returns early on fMappingFailed without clearing fUsedBuffers, these stale buffers contaminate the manager’s state into the next recording session.

Suggested/Potential Trigger Path

Note: These steps are suggested and potential; our automated analysis tools do not currently have the capability to run code or execute a live proof-of-concept.

1. Trigger Transient Failure: An attacker forces a transient allocation/mapping failure in the GPU process (for instance, by scheduling extremely large canvas or WebGPU operations to hit memory limits).
2. OnFailedBuffer Invocation: The allocation failure invokes onFailedBuffer(), which marks fMappingFailed = true and clears fAvailableBuffer.
3. Destructor Stashing: The failed recording aborts, and stack unwinding invokes the destructor of stack-local allocators (e.g., UniformTracker). The destructor executes the else block of BufferSubAllocator::reset(), moving raw, stale pointers (fMappedPtr, fTransferBuffer) into state.fAvailableBuffer.
4. Recording Discard: transferToRecording() returns early, resetting fMappingFailed but leaving the stale allocator in fAvailableBuffer untouched.
5. Re-use and Corruption: A subsequent successful recording begins. The allocator retrieves the stale cache from fAvailableBuffer. Writing to fMappedPtr results in a wild write. When the allocator goes out of scope, the stale transfer buffer is pushed to fUsedBuffers, causing a heap Use-After-Free inside DawnCommandBuffer::onCopyBufferToBuffer when the command copy is executed.

Suggested Fix

To prevent both stashing and contamination of fUsedBuffers during an active allocation failure, BufferSubAllocator::reset() should check if fOwner->fMappingFailed is true and immediately discard its resources:

void BufferSubAllocator::reset() {
    if (fBuffer) {
        SkASSERT(fOwner);
        if (fOwner->fMappingFailed) {
            fBuffer.reset();
            fRemaining = 0;
            return;
        }
        // ... existing reset logic ...

Evaluated with Chrome root at commit: b1520ef4a76878853a31f0943b565e42060edec8

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.