0fe9ff8c67  [ANGLE] Metal backend: Fix crash on OOB vertex attribute offset in syncDirtyAttrib

The bug at a glance

An integer-underflow in the ANGLE Metal backend’s vertex-count math lets an out-of-bounds vertex attribute offset flow into VertexArrayMtl::setupDraw and crash (or read out-of-bounds GPU-side) when the binding offset exceeds the buffer size. WebGL is a broadly reachable web-exposed surface and the GPU process is a valuable target, so an OOB in vertex processing is high severity. It is scored high rather than critical because the immediate observed effect is a crash / robustness violation on the non-conversion path rather than a demonstrated controlled write.

GetVertexCount computed GLint64 bytes = srcBuffer->size() - binding.getOffset() with a signed subtraction and then divided by binding.getStride(); a huge or negative offset (and a zero stride) produced a bogus vertex count, and the non-conversion branch of syncDirtyAttrib stored binding.getOffset() unchecked, so setupDraw later indexed past the buffer.

Root cause

VertexArrayMtl::syncDirtyAttrib configures Metal vertex buffers for a draw. Two helpers, GetVertexCount and GetVertexCountWithConversion, estimate how many vertices fit in the source buffer given the binding’s byte offset and stride. The original math used signed 64-bit arithmetic: GLint64 bytes = srcBuffer->size() - binding.getOffset(); followed by if (bytes < srcFormatSize) return 0; and then numVertices += static_cast<size_t>(bytes) / binding.getStride();. This is fragile in two ways. First, binding.getOffset() is an application-controlled GLintptr that can be larger than the buffer size (via glVertexAttribPointer with a large offset) — the subtraction can go negative, but the negative is only caught if it dips below srcFormatSize; a carefully chosen offset can pass the check while still being out of bounds relative to real usable space. Second, binding.getStride() can be zero (tightly packed attributes report zero stride to mean “use format size”), so the division bytes / binding.getStride() is a division by zero, or with the wrong operand produces a wildly wrong count.

Crucially, the guarding of the result only happened on the conversion path. In syncDirtyAttrib, when needConversion was false, the code did mCurrentArrayBufferOffsets[attribIndex] = binding.getOffset(); directly — storing the raw, unvalidated offset with no vertex-count sanity check. The zero-count fallback (clamp offset to 0, stride to 16) lived only inside convertVertexBuffer, which is reached only on the conversion branch. So for a plain, non-converting attribute with an out-of-bounds offset, the raw offset propagated to mCurrentArrayBufferOffsets and then to VertexArrayMtl::setupDraw, which set the Metal vertex buffer offset beyond the buffer’s length and crashed (Metal validation / out-of-bounds).

The fix restructures both the math and the control flow. GetVertexCount/GetVertexCountWithConversion now reject negative bindingOffset up front (if (bindingOffset < 0) return 0;), use angle::CheckedNumeric<size_t> for bytes = srcBuffer->size(); bytes -= bindingOffset; bytes -= srcFormatSize; with an if (!bytes.IsValid()) return 0; to catch any underflow, and replace the raw stride divisor with effectiveStride = binding.getStride() > 0 ? binding.getStride() : srcFormatSize; to eliminate division-by-zero. More importantly the zero-count check is hoisted in syncDirtyAttrib to before the needConversion branch: size_t numVertices = GetVertexCount(bufferMtl, binding, srcFormatSize); if (numVertices == 0) { ...clamp offset to 0, stride to 16... } else if (needConversion) {...} else {...}. Now both the conversion and non-conversion paths are guarded, and an OOB offset yields the safe KHR_robust_buffer_access_behavior fallback (undefined-but-safe values) instead of an out-of-bounds Metal buffer binding. The redundant check inside convertVertexBuffer is removed since the caller now handles it.

Key code

VertexArrayMtl.mm: GetVertexCount hardened with signed-offset and CheckedNumeric guards

    GLintptr bindingOffset = binding.getOffset();
    if (bindingOffset < 0)
        return 0;

    // Bytes usable for vertex data.
    angle::CheckedNumeric<size_t> bytes = srcBuffer->size();
    bytes -= static_cast<size_t>(bindingOffset);
    // Count the last vertex.  It may occupy less than a full stride.
    bytes -= srcFormatSize;
    if (!bytes.IsValid())
        return 0;

    // Count how many strides fit remaining space.
    size_t effectiveStride = binding.getStride() > 0 ? binding.getStride() : srcFormatSize;
    return 1 + static_cast<size_t>(bytes.ValueOrDie()) / effectiveStride;

Patch walkthrough

• Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/VertexArrayMtl.mm — GetVertexCount and GetVertexCountWithConversion are rewritten: they reject negative binding offsets, compute usable bytes with angle::CheckedNumeric<size_t> and bail on invalid (underflow), and divide by an effectiveStride that falls back to srcFormatSize when stride is 0. syncDirtyAttrib now calls GetVertexCount and checks numVertices==0 before the needConversion branch, applying the robust-access clamp (offset 0, stride 16) on both paths; the previously-only-on-conversion fallback and the redundant GetVertexCount call inside convertVertexBuffer are removed. Adds #include “common/mathutil.h” for CheckedNumeric.
• Source/ThirdParty/ANGLE/src/tests/gl_tests/VertexAttributeTest.cpp — Adds VertexAttributeTest.OutOfBoundsOffsetNoFormatConversion: a valid draw, then a draw with glVertexAttribPointer offset 1024 far exceeding the buffer (UB in ES, no assertion), then another valid draw asserting the backend recovered and state was not corrupted — exercising the non-conversion OOB path that previously crashed.

Background

VertexArrayMtl::syncDirtyAttrib — ANGLE Metal-backend routine that, for each dirty vertex attribute, decides whether format conversion is needed and records the source buffer, offset and stride Metal will use for the draw.

gl::VertexBinding offset/stride — Application-controlled values set by glVertexAttribPointer/glBindVertexBuffer. Offset is a GLintptr; stride 0 conventionally means tightly-packed (use the format size).

angle::CheckedNumeric — ANGLE’s checked-arithmetic wrapper (common/mathutil.h). Operations set an invalid flag on overflow/underflow; IsValid()/ValueOrDie() let callers reject out-of-range results instead of wrapping.

KHR_robust_buffer_access_behavior — GL guarantee that out-of-bounds vertex/buffer accesses return undefined-but-safe values rather than crashing or reading arbitrary memory. The zero-vertex clamp implements this contract.

VertexArrayMtl::setupDraw — Consumes mCurrentArrayBufferOffsets/Strides to bind Metal vertex buffers; an offset past the buffer length triggers a Metal out-of-bounds/validation crash — the observed failure site.

Vulnerability window

1. Setup — Web content issues glVertexAttribPointer with an offset (e.g. 1024) far larger than the bound buffer.
2. Sync — syncDirtyAttrib runs; on the non-conversion path the raw binding.getOffset() is stored into mCurrentArrayBufferOffsets without a vertex-count check.
3. Bad count — GetVertexCount’s signed subtraction / zero-stride division produced a meaningless count that was never consulted on this path anyway.
4. Draw — setupDraw binds the Metal vertex buffer at the OOB offset.
5. Crash / OOB — Metal reads beyond the buffer, crashing the GPU/content process (robustness violation).
6. Fixed — With the patch numVertices==0 is detected before branching and the offset is clamped to 0 with a safe stride, satisfying robust-access.

Proof of concept

The GL test binds a small 4-vertex buffer, performs a valid draw, then sets a vertex attribute offset of 1024 (far past the buffer) and draws — the case that previously stored the raw OOB offset and crashed in setupDraw. The final valid draw asserts the backend recovered, proving the clamp fallback fires on the non-conversion path.

TEST_P(VertexAttributeTest, OutOfBoundsOffsetNoFormatConversion)
{
    constexpr char kVS[] =
        R"(attribute vec4 a_position;
void main()
{
    gl_Position = a_position;
    gl_PointSize = 1.0;
})";

    ANGLE_GL_PROGRAM(program, kVS, essl1_shaders::fs::Red());
    glUseProgram(program);

    GLint posLoc = glGetAttribLocation(program, "a_position");
    ASSERT_NE(-1, posLoc);

    const GLfloat vertices[] = {
        -1.0f, -1.0f,
         1.0f, -1.0f,
         1.0f,  1.0f,
        -1.0f,  1.0f,
    };
    GLBuffer buffer;
    glBindBuffer(GL_ARRAY_BUFFER, buffer);
    glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);

    glEnableVertexAttribArray(posLoc);

    // Valid draw: prove setup is correct.
    glVertexAttribPointer(posLoc, 2, GL_FLOAT, GL_FALSE, 0, 0);
    glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
    glClear(GL_COLOR_BUFFER_BIT);
    glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
    ASSERT_GL_NO_ERROR();
    EXPECT_PIXEL_COLOR_EQ(32, 32, GLColor::red);

    // OOB draw: offset far exceeds buffer. Result is UB in ES — no assertion.
    glVertexAttribPointer(posLoc, 2, GL_FLOAT, GL_FALSE, 0,
                          reinterpret_cast<const void *>(1024));
    glDrawArrays(GL_POINTS, 0, 1);

    // Valid draw again: prove the backend recovered and state is not corrupted.
    glVertexAttribPointer(posLoc, 2, GL_FLOAT, GL_FALSE, 0, 0);
    glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
    glClear(GL_COLOR_BUFFER_BIT);
    glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
    ASSERT_GL_NO_ERROR();
    EXPECT_PIXEL_COLOR_EQ(32, 32, GLColor::red);
}

Exploitation

1. Reach — Serve a WebGL page; from JS call glVertexAttribPointer with an offset larger than the bound buffer on a non-converting float attribute.
2. Trigger — Issue a draw; syncDirtyAttrib forwards the raw offset and setupDraw binds a Metal buffer past its end.
3. OOB read — On builds without Metal validation, the GPU reads adjacent memory as vertex data — an information-disclosure / robustness break; with validation it is a GPU-process crash / DoS.
4. Amplify — Combine with zero-stride and large negative-appearing offsets to maximize the misread region across draws.

Detection & hunting

For defenders and SOC / detection engineers:

• GPU-process crashes on draw —
• Offset > buffer size —
• Zero-stride divisions —

Audit directions

• Signed buffer math —
• Path-asymmetric guards —
• Zero stride —
• Offset propagation —