From 827d949af9266a6d1b8173a779d7fa90cccc5d20 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:13:12 -0400 Subject: [PATCH 1/2] security: preflight PCX and XPM output allocations --- crates/codecs/src/codecs/pcx.rs | 13 ++++++++++++- crates/codecs/src/codecs/xpm.rs | 13 +++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/crates/codecs/src/codecs/pcx.rs b/crates/codecs/src/codecs/pcx.rs index f89a781e9d..aefbd0a968 100644 --- a/crates/codecs/src/codecs/pcx.rs +++ b/crates/codecs/src/codecs/pcx.rs @@ -72,8 +72,19 @@ pub(crate) fn decode(b: &[u8], limits: &Limits) -> Result { return Err(bad("8-bit indexed PCX has no 256-colour palette")); } let image_end = if vga { b.len() - 769 } else { b.len() }; + // Each RLE packet emits at most 63 bytes and occupies at least two bytes. + // Reject inputs that cannot possibly contain all scanlines before allocating + // the decoded image buffer. + let min_packets = stride.div_ceil(63).checked_mul(2).ok_or_else(|| bad("compressed row size overflow"))?; + let min_encoded = min_packets.checked_mul(h as usize).and_then(|n| n.checked_mul(planes)).ok_or_else(|| bad("compressed image size overflow"))?; + let encoded_len = image_end.saturating_sub(HEADER); + if encoded_len < min_encoded { + return Err(bad("PCX pixel data is too short for the declared image")); + } let mut row = vec![0u8; row_len]; - let mut pixels = vec![0u8; total]; + let mut pixels = Vec::new(); + pixels.try_reserve_exact(total).map_err(|_| bad("PCX pixel buffer is too large"))?; + pixels.resize(total, 0); let mut p = HEADER; let width = w as usize; for y in 0..h as usize { diff --git a/crates/codecs/src/codecs/xpm.rs b/crates/codecs/src/codecs/xpm.rs index 82543d8867..c20f3c58e9 100644 --- a/crates/codecs/src/codecs/xpm.rs +++ b/crates/codecs/src/codecs/xpm.rs @@ -206,11 +206,16 @@ fn decode_lines(lines: &[Vec], limits: &Limits) -> Result } } let row_len = width.checked_mul(cpp).ok_or_else(|| bad("row byte count overflow"))?; - let mut pixels = vec![0u8; width.checked_mul(height).and_then(|n| n.checked_mul(4)).ok_or_else(|| bad("pixel size overflow"))?]; + // Validate all source rows before reserving the decoded RGBA image. A malformed + // short row must not be able to make a large header-sized allocation first. + if lines[1 + colours..required].iter().any(|line| line.len() != row_len) { + return Err(bad("XPM row has the wrong width")); + } + let total = width.checked_mul(height).and_then(|n| n.checked_mul(4)).ok_or_else(|| bad("pixel size overflow"))?; + let mut pixels = Vec::new(); + pixels.try_reserve_exact(total).map_err(|_| bad("XPM pixel buffer is too large"))?; + pixels.resize(total, 0); for (y, line) in lines[1 + colours..required].iter().enumerate() { - if line.len() != row_len { - return Err(bad("XPM row has the wrong width")); - } for x in 0..width { let key = &line[x * cpp..(x + 1) * cpp]; let rgba = palette.get(key).ok_or_else(|| bad("unknown palette key"))?; From 1bd922f6d1857318531accbc48e17ea245034f9a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 22 Oct 2026 19:24:24 -0400 Subject: [PATCH 2/2] fix(codecs): use exact minimum encoded row length for PCX preflight --- crates/codecs/src/codecs/pcx.rs | 18 +++++++++++++++--- crates/codecs/tests/it/pcx.rs | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/codecs/src/codecs/pcx.rs b/crates/codecs/src/codecs/pcx.rs index aefbd0a968..48dbf1169e 100644 --- a/crates/codecs/src/codecs/pcx.rs +++ b/crates/codecs/src/codecs/pcx.rs @@ -72,11 +72,23 @@ pub(crate) fn decode(b: &[u8], limits: &Limits) -> Result { return Err(bad("8-bit indexed PCX has no 256-colour palette")); } let image_end = if vga { b.len() - 769 } else { b.len() }; - // Each RLE packet emits at most 63 bytes and occupies at least two bytes. + // A 63-byte RLE packet takes 2 bytes; a trailing remainder of 1 byte below 0xC0 + // can be encoded as a 1-byte literal, while any larger remainder takes at least 2 bytes. + // Because RLE packets may span planes within a scanline (`row_len`), compute the + // minimum encoded bytes across the full scanline (`row_len`) and multiply by `h`. // Reject inputs that cannot possibly contain all scanlines before allocating // the decoded image buffer. - let min_packets = stride.div_ceil(63).checked_mul(2).ok_or_else(|| bad("compressed row size overflow"))?; - let min_encoded = min_packets.checked_mul(h as usize).and_then(|n| n.checked_mul(planes)).ok_or_else(|| bad("compressed image size overflow"))?; + let min_packets = (row_len / 63) + .checked_mul(2) + .and_then(|n| { + n.checked_add(match row_len % 63 { + 0 => 0, + 1 => 1, + _ => 2, + }) + }) + .ok_or_else(|| bad("compressed row size overflow"))?; + let min_encoded = min_packets.checked_mul(h as usize).ok_or_else(|| bad("compressed image size overflow"))?; let encoded_len = image_end.saturating_sub(HEADER); if encoded_len < min_encoded { return Err(bad("PCX pixel data is too short for the declared image")); diff --git a/crates/codecs/tests/it/pcx.rs b/crates/codecs/tests/it/pcx.rs index a5796473e6..792a41021c 100644 --- a/crates/codecs/tests/it/pcx.rs +++ b/crates/codecs/tests/it/pcx.rs @@ -87,6 +87,25 @@ fn rejects_truncated_and_overshooting_rle_without_panicking() { let mut b = header(8, 3, 2, 1, 2); b[66..68].copy_from_slice(&0u16.to_le_bytes()); assert_eq!(detect(&b), None, "invalid stride is not a PCX signature"); + + // A solid image with stride of the form 63k + 1 (64, 127) encodes as k full 63-byte runs + // plus a 1-byte literal (2k + 1 bytes per plane row), which is valid and must decode. + for width in [64u16, 127] { + let mut solid = header(8, 3, width, 1, width); + for _plane in 0..3 { + let mut rem = usize::from(width); + while rem >= 63 { + solid.extend_from_slice(&[0xff, 0x00]); // 63-byte run of 0x00 + rem -= 63; + } + if rem == 1 { + solid.push(0x00); // 1-byte literal 0x00 + } + } + let im = decode(&solid).unwrap(); + assert_eq!(im.dimensions(), (u32::from(width), 1)); + assert!(im.data().iter().all(|&v| v == 0)); + } } #[test]