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 compromised renderer can potentially deserialize a forged SDFTSubRun with a mask format of kARGB, which is normally reserved for color bitmap text. This causes a stride mismatch between the geometry processor (which expects a 16-byte or 20-byte stride) and the CPU-side vertex filler (which writes 12-byte or 16-byte structs). As a result, uninitialized memory from a shared GPU-process staging buffer pool is uploaded and rendered to the GPU, potentially leaking cross-origin graphics data.

Affected files:

• third_party/skia/src/text/gpu/SubRunContainer.cpp
• third_party/skia/src/gpu/ganesh/ops/AtlasTextOp.cpp
• third_party/skia/src/gpu/ganesh/text/GlyphData.cpp

Estimated timestamp from git blame: 2022-02-23

Detailed Description

A security vulnerability potentially exists in Skia’s Ganesh text-rendering pipeline when deserializing Slug objects from a compromised renderer. Specifically, the deserialization of SDFTSubRun does not validate that the maskFormat is restricted to kA8. This leads to a mismatch between the GPU vertex stride (defined by the Geometry Processor) and the CPU-side vertex fill stride, resulting in an uninitialized memory disclosure from the GPU process.

Analysis of the Root Cause

1. Deserialization Validation Deficit: In third_party/skia/src/text/gpu/SubRunContainer.cpp, SDFTSubRun::MakeFromBuffer is used to deserialize SDF subruns from an untrusted renderer:

   static SubRunOwner MakeFromBuffer(SkReadBuffer& buffer, ...)
   

   It reads the VertexFiller using make_vertex_filler_from_buffer:

   MaskFormat maskFormat = buffer.read32LE(MaskFormat::kLast);
   

   Since MaskFormat::kLast is MaskFormat::kARGB (value 2), a compromised renderer can supply kARGB and pass deserialization. The constructor of SDFTSubRun contains only a debug-only assertion (SkASSERT) checking that maskFormat == MaskFormat::kA8, which is compiled out of release/production builds of Chrome.

2. GPU Allocation (16-Byte Stride): When drawing the subrun, AtlasTextOp::Make executes. Because glyphParams().isSDF is unconditionally true for an SDFTSubRun, it routes to the distance-field path. AtlasTextOp builds a GrDistanceFieldA8TextGeoProc whose vertex layout is unconditionally composed of:

   • inPosition: 8 bytes (kFloat2_GrVertexAttribType for non-perspective)
   • inColor: 4 bytes (kUByte4_norm_GrVertexAttribType)
   • inTextureCoords: 4 bytes (kUShort2_GrVertexAttribType)

   This results in a total vertex stride of exactly 16 bytes (or 20 bytes if perspective is active). The vertex buffer is then allocated from the shared pool GrVertexBufferAllocPool using this 16-byte stride.

3. CPU Fill (12-Byte Stride): During the draw preparation, GlyphData::fillVertexData handles the vertex data population on the CPU side. It determines the struct size based on vf.maskFormat():

   if (!positionMatrix.hasPerspective()) {
       if (vf.maskFormat() == MaskFormat::kARGB) {
           using Quad = ARGB2DVertex[4];
           ...
   

   Because the attacker forged maskFormat to kARGB, the code uses ARGB2DVertex, which is only 12 bytes in size (since the GrColor field is discarded). Thus, Skia writes only 12 bytes per vertex but advances the buffer allocation index using the 16-byte stride requested by the geometry processor.

4. Uninitialized Memory Leak: This mismatch leaves 4 bytes (or 8 bytes under perspective variations) uninitialized at the tail of each vertex. Because the vertex buffers are allocated with ZeroInit::kNo from a shared staging pool reused across multiple contexts and origins, these uninitialized bytes contain stale graphics data from prior draws. This stale data is uploaded to the GPU, rendered via the distance field shader, and can potentially be retrieved by the attacker via canvas readback (getImageData) or texture copying.

Potential Steps to Reproduce

Since our automated tooling does not currently have the capability to execute proof-of-concept code, these are theoretical steps demonstrating how an attacker could trigger the mismatch:

1. From a compromised renderer, construct a serialized DrawSlugOp paint-op.
2. In the SDFTSubRun payload, set the VertexFiller’s maskFormat field to 2 (MaskFormat::kARGB).
3. Submit the paint op over the IPC channel to the GPU process to be rasterized into an OOP-R tile backing or an accelerated 2D canvas.
4. Read back the rasterized output from the renderer side (e.g., via CanvasRenderingContext2D.getImageData) and examine the returned pixel data for artifacts corresponding to leaked cross-origin GPU memory.

Suggested Fix

Validate the maskFormat inside SDFTSubRun::MakeFromBuffer in third_party/skia/src/text/gpu/SubRunContainer.cpp. If the format is not MaskFormat::kA8, the buffer should be marked invalid and deserialization should fail:

    static SubRunOwner MakeFromBuffer(SkReadBuffer& buffer,
                                      SubRunAllocator* alloc,
                                      const SkStrikeClient* client) {
        ...
        auto vertexFiller = make_vertex_filler_from_buffer(buffer, alloc, &glyphVector.value(),
                                                           skglyph::kSDFT, kGlyphInsetting);
        if (!buffer.validate(vertexFiller.has_value())) { return nullptr; }
        
        // Ensure the deserialized mask format is kA8 for SDFT subruns
        if (!buffer.validate(vertexFiller->maskFormat() == MaskFormat::kA8)) { return nullptr; }

        return alloc->makeUnique<SDFTSubRun>(useLCD,
                                             isAntiAliased,
                                             matrixRange,
                                             std::move(*vertexFiller),
                                             std::move(*glyphVector));
    }

Evaluated with Chrome root at commit: e9507a33bb4148ee071aaaf8a7e9ad68770359bf

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.