Merge pull request #8405 from dylanmtaylor/security/codec-allocation-bounds

security: preflight PCX and XPM allocations
This commit is contained in:
Dylan M. Taylor
2026-10-22 19:42:34 -04:00
committed by GitHub
3 changed files with 52 additions and 5 deletions
+24 -1
View File
@@ -72,8 +72,31 @@ pub(crate) fn decode(b: &[u8], limits: &Limits) -> Result<Image, CodecError> {
return Err(bad("8-bit indexed PCX has no 256-colour palette"));
}
let image_end = if vga { b.len() - 769 } else { b.len() };
// 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 = (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"));
}
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 {
+9 -4
View File
@@ -206,11 +206,16 @@ fn decode_lines(lines: &[Vec<u8>], limits: &Limits) -> Result<Image, CodecError>
}
}
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"))?;
+19
View File
@@ -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]