hw/display/ati: integer overflow in Rage128 2D blit bounds check leads to OOB write

Summary

The VRAM bounds check in ati_2d_do_blt() computes (dst.y + dst.height) * dst_stride using signed 32-bit arithmetic. On Rage128, setup_2d_blt_ctx() multiplies dst_stride by bpp (up to 32), producing strides up to 524,256 bytes/row. With large dst.y + dst.height values, the signed int product overflows to a negative value, causing the bounds check to evaluate false when it should reject the operation.

The subsequent blit loop uses unsigned arithmetic for the same index computation, producing large positive offsets (up to ~4 GB past VRAM) that write guest-controlled data into host process memory.

CWE: CWE-190 (Integer Overflow) leading to CWE-787 (Out-of-bounds Write) CVSS 3.1: 8.8 (AV:L/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:H) Verified at: HEAD commit 3377670e

Root Cause

In setup_2d_blt_ctx() (ati_2d.c:122-124), Rage128 multiplies the stride by bpp:

if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
    ctx->dst_stride *= ctx->bpp;
}

The VRAM bounds check (ati_2d.c:169-173):

if (ctx->dst.x > 0x3fff || ctx->dst.y > 0x3fff ||
    ctx->dst_bits >= vram_end - bypp || ctx->dst_bits + ctx->dst.x * bypp +
    (ctx->dst.y + ctx->dst.height) * ctx->dst_stride >= vram_end - bypp) {

With dst_y=0, dst_height=16383, dst_stride=524256: 16383 * 524256 = 8,588,445,248, which wraps to -1,001,489,344 in signed 32-bit. The negative result makes dst_bits + negative_value point before the VRAM buffer, so the >= vram_end comparison is false and the check passes.

The blit loop (ati_2d.c:254-269, fallback path) uses unsigned promotion:

for (y = 0; y < vis_dst.height; y++) {
    i = vis_dst.x * bypp;
    if (ctx->top_to_bottom) {
        i += (vis_dst.y + y) * ctx->dst_stride;
    }
    memmove(&ctx->dst_bits[i], &ctx->src_bits[j], vis_dst.width * bypp);
}

(vis_dst.y + y) is int16_t + unsigned int -> unsigned. The multiplication produces large positive offsets writing past VRAM.

Trigger

-device ati-vga,rombar=0 (Rage128 is the default ATI model)

Guest writes via MMIO registers:

• DP_DATATYPE: set to 6 (32bpp)
• DST_PITCH: set to 16383 (0x3FFF, maximum)
• DST_Y: 0, DST_HEIGHT: 16383, DST_WIDTH: 16383
• DST_OFFSET: 0
• DP_CNTL: DST_X_LEFT_TO_RIGHT | DST_Y_TOP_TO_BOTTOM
• DP_MIX: ROP3_PATCOPY or ROP3_SRCCOPY
• Trigger: write to DST_HEIGHT_WIDTH register

Impact

Guest-to-host OOB write. Rows 129-8192 write at controlled offsets within ~4 GB of the VRAM buffer. Each row writes up to 65,532 bytes of guest-controlled data. PATCOPY writes from dp_brush_frgd_clr (guest-controlled); SRCCOPY writes from guest VRAM. QEMU allocates VRAM via mmap; adjacent heap contains device state, timer callbacks, and function pointers. The source bounds check has the same overflow, enabling OOB reads.

Rage128 only -- Radeon does not multiply stride by bpp, so its products stay within INT_MAX.

All three blit paths are vulnerable: fallback memmove, pixman_blt direct, and pixman_blt via tmp. The PATCOPY fill path (lines 273-319) is also affected.

Proposed Fix

Use uint64_t for the offset computation to prevent overflow. Apply the same fix to the source bounds check at line 209-210.

--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -155,6 +155,8 @@ static bool ati_2d_do_blt(const ATI2DCtx *ctx, uint8_t use_pixman)
 {
     QemuRect vis_src, vis_dst;
     unsigned int x, y, i, j, bypp = ctx->bpp / 8;
+    uint64_t dst_end;
+    uint64_t dst_off = ctx->dst_bits - ctx->vga->vram_ptr;
     const uint8_t *vram_end = ctx->vga->vram_ptr + ctx->vga->vram_size;
 
     if (!ctx->bpp) {
@@ -165,9 +167,11 @@ static bool ati_2d_do_blt(const ATI2DCtx *ctx, uint8_t use_pixman)
         qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n");
         return false;
     }
+    dst_end = dst_off + (uint64_t)ctx->dst.x * bypp +
+              (uint64_t)(ctx->dst.y + ctx->dst.height) * ctx->dst_stride;
     if (ctx->dst.x > 0x3fff || ctx->dst.y > 0x3fff ||
-        ctx->dst_bits >= vram_end - bypp || ctx->dst_bits + ctx->dst.x * bypp +
-        (ctx->dst.y + ctx->dst.height) * ctx->dst_stride >= vram_end - bypp) {
+        dst_off >= ctx->vga->vram_size ||
+        dst_end >= ctx->vga->vram_size) {
         qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
         return false;
     }