GHSA-mxvv-97wh-cfmm: ImageMagick (WriteBMPImage): 32-bit integer overflow when writing BMP scanline stride → heap buffer overflow
SummaryA 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)Per-row base and 24-bpp loop (writer)Allocation (writer)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: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 DemonstrationChosen 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 stridePer-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)Binary: **ELF 32-bit i386**, Q8, non-HDRIResources 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):Runtime limits to exercise writer:**One-liner trigger (no input file):****Expected:** ASan heap-buffer-overflow in `WriteBMPImage` (will be provided in a private gist link).**Alternate PoC (raw PPM generator):**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 (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**Reporter:** Lumina Mescuwa**Tested on:** i686 Linux container (details in Repro)**Timeline:** August 19th, 2025AppendicesAppendix 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.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)
CVSS v3.1: 7.5