The vulnerability analysis is based on the detailed description provided, which includes the root cause, a demonstration of the vulnerability, and an ASan log. The vulnerability is a heap-based buffer overflow in the WriteBMPImage function in coders/bmp.c, caused by an integer overflow when calculating the bytes_per_line for a BMP image. The provided commit 2c55221f4d38193adcb51056c14cf238fbcc35d7 addresses a similar issue in ReadBMPImage but does not contain the fix for WriteBMPImage. The vulnerability description, however, provides a clear explanation and a suggested patch for WriteBMPImage, which was used to identify the vulnerable function and the exact line of code. The ASan log further confirms that the crash occurs within WriteBMPImage, making the identification of this function as vulnerable a high-confidence finding.
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 tinybytes_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 belowINT_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)
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
“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.
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
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);
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;
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