Chromium

Steps to reproduce

(Chromium)

1. Apply the chromium.diff renderer patch to chromium.
2. Run genskpic.py to generate drawable_picture.skp.hh, then move the generated file to src/gpu/command_buffer/client.
3. Build and start the browser.
4. Open index.html.
5. The GPU process will crash.

(Skia standalone)

1. Run genskpic.py to generate a .skp file.
2. You can now run it in skpbench using: ./out/asan/skpbench --src pic.skp --config gles.
3. UBSAN crash will happen.

Vulnerability Details

In Skia, when drawing Skia Picture with many DRAW_VERTICES_OBJECT operations, MeshOp::onCombineIfPossible will be called to test how many meshes are allowed to be combined into a single operation. This is done by concatenating vertices and indices counts of all possible meshes:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp;l=1216

However, while there are checks to prevent fVertexCount from overflowing, no checks are made to ensure that the addition of fIndexCount and that->fIndexCount in [1] will not overflow. So by drawing a triangular mesh with, for example, 3 vertices and 3000000 indices where each indice is an index to a vertice we can easily make fIndexCount (a 32-bit integer) overflow.

(The reason I'm keeping the number of vertices low (3) is to avoid the check at [2] when merging meshes and because I need to optimize size for PoC).

GrOp::CombineResult MeshOp::onCombineIfPossible(GrOp* t, SkArenaAlloc*, const GrCaps& caps) {
    auto that = t->cast<MeshOp>();
    ...
    if (SkToBool(fIndexCount) && fVertexCount > SkToInt(UINT16_MAX) - that->fVertexCount) { // <-- [2]
        return CombineResult::kCannotCombine;
    }
    ...
    fMeshes.move_back_n(that->fMeshes.size(), that->fMeshes.begin());
    fVertexCount += that->fVertexCount;
    fIndexCount  += that->fIndexCount; // <-- [1]
    return CombineResult::kMerged;
}

Later, MeshOp::onPrepareDraws will be called to allocate enough space for all index data using the overflowed fIndexCount [3], and then will copy each individual mesh to the allocated buffer [4] which ends up in an OOB write.

https://source.chromium.org/chromium/chromium/src/+/main:third_party/skia/src/gpu/ganesh/ops/DrawMeshOp.cpp;l=1119

void MeshOp::onPrepareDraws(GrMeshDrawTarget* target) {
    ...

    std::tie(indexBuffer, firstIndex) = fMeshes[0].gpuIB();
    if (fIndexCount && !indexBuffer) {
        uint16_t* indices = nullptr;
        indices = target->makeIndexSpace(fIndexCount, &indexBuffer, &firstIndex); // <-- [3]
        if (!indices) {
            SkDebugf("Could not allocate indices.\n");
            return;
        }
        // We can just copy the first mesh's indices. Subsequent meshes need their indices adjusted.
        std::copy_n(fMeshes[0].indices(), fMeshes[0].indexCount(), indices);
        int voffset = fMeshes[0].vertexCount();
        int ioffset = fMeshes[0].indexCount();
        for (int m = 1; m < fMeshes.size(); ++m) {
            for (int i = 0; i < fMeshes[m].indexCount(); ++i) {
                indices[ioffset++] = fMeshes[m].indices()[i] + voffset; // <-- [4]
            }
            voffset += fMeshes[m].vertexCount();
        }
        SkASSERT(voffset == fVertexCount);
        SkASSERT(ioffset == fIndexCount);
    } else if (indexBuffer) {
        SkASSERT(fMeshes.size() == 1);
        SkASSERT(firstIndex % sizeof(uint16_t) == 0);
        firstIndex /= sizeof(uint16_t);
    }

    ...
}

This could be achieved by a compromised renderer.

Having to keep the number of vertices low affects a bit the control of the attacker of the value being written since each indice must be an index to a vertice. I'm only doing this here for demonstration purposes (optimizing size of PoC), in a real attack scenario an attacker might prefer to have some bigger meshes to have better control of the value where one of the first meshes will have more vertices than the remaining meshes.

Fix:

diff --git a/src/gpu/ganesh/ops/DrawMeshOp.cpp b/src/gpu/ganesh/ops/DrawMeshOp.cpp
index eaa3c7d85f..f4f5625fff 100644
--- a/src/gpu/ganesh/ops/DrawMeshOp.cpp
+++ b/src/gpu/ganesh/ops/DrawMeshOp.cpp
@@ -1232,7 +1232,7 @@ GrOp::CombineResult MeshOp::onCombineIfPossible(GrOp* t, SkArenaAlloc*, const Gr
         return CombineResult::kCannotCombine;
     }
 
-    if (fVertexCount > INT32_MAX - that->fVertexCount) {
+    if (fVertexCount > INT32_MAX - that->fVertexCount || fIndexCount > INT32_MAX - that->fIndexCount) {
         return CombineResult::kCannotCombine;
     }
     if (SkToBool(fIndexCount) != SkToBool(that->fIndexCount)) {

Type of crash

GPU Process

Environment

Chrome version: 129.0.6661.0 (Developer Build) (64-bit).

OS: Linux.

Reporter Credit

Renan Rios (@hyhy_100)

Thanks!