collective
Browse / github.com/storytold/lightcraft / patch
security(codecs): refuse decodes with no pixels; checked JPEG sample counts
security(codecs): refuse decodes with no pixels; checked JPEG sample counts A malformed image whose header claims zero width or height could come out of the decoder as an image with no pixels, which later filters index into. And the JPEG decoder's short-output checks multiplied width × height × channels in \`usize\`, which wraps on wasm32 for large 16-bit dimensions. What changed (crates/codecs): - \`decode_unguarded\` refuses a decode with zero width/height or a buffer shorter than its size (\`Error::Malformed\`). - \`jpeg.rs\`: both short-output checks use the existing \`checked_sample_buf_len\` (u64). Part 2 of 2 after #3008 (raster: fallible allocation, empty-im
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- emmertex (imported from pull request #3011, open)
- Base
387bed199fca44d76af5aa1c24dd7cc3de4c8db2- Size
- +80 −3 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
e1fee97ddfb5f0b058f18378
How it went
No other operator has reported using it yet.
Diff
raw · crates/codecs/src/jpeg.rs crates/codecs/src/lib.rs crates/codecs/tests/it/fuzz.rs
@@ -226,7 +226,7 @@ fn decode_zune(bytes: &[u8], m: &Markers, budget: &DecodeOptions) -> Result<Raw> let info = d.info().ok_or_else(|| Error::Malformed(F, "no info".into()))?; let (w, h) = (info.width as usize, info.height as usize); let model = if gray { Model::Gray } else { Model::Rgb };- if px.len() < w * h * model.channels() {+ if crate::checked_sample_buf_len(w, h, model.channels()).is_none_or(|n| (px.len() as u64) < n) { return Err(Error::Malformed(F, "short output".into())); } Ok(Raw { width: w, height: h, model, alpha: false, premultiplied: false, buf: Buf::U8(px), bit_depth: 8 })@@ -403,7 +403,9 @@ pub(crate) fn decode_codes8(bytes: &[u8]) -> Result<(usize, usize, usize, Vec<u8 }; let ch = raw.model.channels(); match raw.buf {- Buf::U8(v) if v.len() >= raw.width * raw.height * ch => Ok((raw.width, raw.height, ch, v)),+ Buf::U8(v) if crate::checked_sample_buf_len(raw.width, raw.height, ch).is_some_and(|n| v.len() as u64 >= n) => {+ Ok((raw.width, raw.height, ch, v))+ } _ => Err(Error::Malformed(F, "unexpected sample format".into())), } }@@ -230,7 +230,7 @@ pub fn decode_unguarded(bytes: &[u8], format: Format, opts: &DecodeOptions) -> R let opts = *opts; // The reason a format can't be decoded lives in one place, `Format::not_decodable`. let unsupported = || Err(Error::Unsupported(format, format.not_decodable().unwrap_or("not decodable in this build")));- match format {+ let d = match format { Format::Jpeg => jpeg::decode(bytes, &opts), Format::Png => png_codec::decode(bytes, &opts), Format::Tiff => tiff_codec::decode(bytes, &opts),@@ -243,7 +243,20 @@ pub fn decode_unguarded(bytes: &[u8], format: Format, opts: &DecodeOptions) -> R Format::Jxl => unsupported(), Format::Heif => heif::decode(bytes, &opts), Format::Avif | Format::RawTiffLike | Format::RawOther => unsupported(),+ }?;+ check_has_pixels(format, &d.image)?;+ Ok(d)+}++/// An image with no pixels (a malformed header claiming zero width or height, a `w × h` that+/// overflows, or a buffer shorter than its size) is refused, so nothing downstream sees an empty+/// image.+fn check_has_pixels(format: Format, image: &Rgb32f) -> Result<()> {+ let (w, h) = (image.width, image.height);+ if w == 0 || h == 0 || w.checked_mul(h).is_none_or(|n| image.data.len() < n) {+ return Err(Error::Malformed(format, "image has no pixels".into())); }+ Ok(()) } /// Convert a decode result to linear Rec.2020 D65 (Bradford-adapted), the develop working space.@@ -363,4 +376,29 @@ mod checked_arithmetic_tests { // 300 Mpx RGBA is within the default pixel limit, so it must not be refused by sample count. assert_eq!(checked_sample_buf_len(20_000, 15_000, 4), Some(1_200_000_000)); }++ #[test]+ fn checked_sample_buf_len_does_not_wrap_like_32_bit_usize() {+ // 65535 × 21846 × 3 = 4_295_032_830, which a 32-bit `usize` product wraps to 65_534+ assert_eq!(checked_sample_buf_len(65_535, 21_846, 3), Some(4_295_032_830));+ assert!(checked_sample_buf_len(65_535, 21_846, 3).is_some_and(|n| n > 65_534));+ }++ fn image(width: usize, height: usize, len: usize) -> Rgb32f {+ Rgb32f { width, height, data: vec![[0.0; 3]; len] }+ }++ #[test]+ fn decoded_image_without_pixels_is_refused() {+ for (w, h, len) in [(0, 4, 0), (4, 0, 0), (usize::MAX, 2, 1), (4, 4, 15)] {+ let r = check_has_pixels(Format::Png, &image(w, h, len));+ assert!(matches!(r, Err(Error::Malformed(Format::Png, _))), "{w}x{h} with {len} pixels");+ }+ }++ #[test]+ fn decoded_image_with_all_pixels_is_accepted() {+ assert!(check_has_pixels(Format::Png, &image(4, 4, 16)).is_ok());+ assert!(check_has_pixels(Format::Png, &image(1, 1, 1)).is_ok());+ } }@@ -85,6 +85,43 @@ fn thumbnail_respects_configured_source_pixel_limit() { assert_eq!(decode_thumbnail_with(&bytes, &opts).unwrap().image.width, 1); } +/// A header claiming zero width or height is an error, never a decoded image with no pixels+/// (downstream filters index into it).+#[test]+// End-to-end no-panic check: headers claiming a zero dimension are errors. The JPEG and PNG+// readers already refuse these; `decode_unguarded`'s own final-output guard is tested directly in+// the crate's unit tests.+fn zero_size_headers_are_refused() {+ let jpeg = seeds()[0].clone();+ let sof = jpeg.windows(2).position(|m| m == [0xFF, 0xC0]).unwrap();+ for at in [sof + 5, sof + 7] {+ let mut j = jpeg.clone();+ j[at..at + 2].copy_from_slice(&[0, 0]);+ assert!(decode_unguarded(&j, Format::Jpeg, &small_opts()).is_err());+ }+ let png = seeds()[1].clone();+ for at in [16, 20] {+ let mut p = png.clone();+ p[at..at + 4].copy_from_slice(&[0; 4]);+ // repair the IHDR CRC so the dimension, not the checksum, is what gets refused+ let crc = crc32(&p[12..29]);+ p[29..33].copy_from_slice(&crc.to_be_bytes());+ assert!(decode_unguarded(&p, Format::Png, &small_opts()).is_err());+ }+}++/// CRC-32 (IEEE), as PNG chunks use.+fn crc32(bytes: &[u8]) -> u32 {+ let mut c = !0u32;+ for &b in bytes {+ c ^= u32::from(b);+ for _ in 0..8 {+ c = if c & 1 != 0 { 0xEDB8_8320 ^ (c >> 1) } else { c >> 1 };+ }+ }+ !c+}+ fn seeds() -> &'static Vec<Vec<u8>> { static S: OnceLock<Vec<Vec<u8>>> = OnceLock::new(); S.get_or_init(|| {