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)

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:

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)

• 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):

• 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)

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, 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);

• 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)