From 35c14f1e3cad75ecd4b47126bb17e7de6cb51139 Mon Sep 17 00:00:00 2001 From: Lilith River Date: Tue, 30 Jun 2026 19:53:17 -0600 Subject: [PATCH] decoder: reconstruct extended-depth samples modulo declared bit depth Promoted extended-depth grayscale (9-15 bits -> u16, 17-31 bits -> u32) can carry bits above the declared depth after the horizontal predictor, which reconstructs prev+delta in the promoted storage width rather than modulo 2^BitsPerSample. This underflowed `max - v` in invert_colors (the image-rs/image-tiff#396 panic in debug, wrong pixel in release) and left junk in the high bits of the non-inverted output (#396 review, @197g: "clamp the channel values in the non-inverted color case"). An n-bit predictor delta is stored in exactly n bits, i.e. modulo 2^n, so reconstruction is defined modulo 2^n as well. Since 2^n divides the u16/u32 storage modulus, masking each promoted sample with (1< 2406 bytes ...epth-invert-overflow-9f2f479685f84884.tiff | Bin 0 -> 2775 bytes tests/predict.rs | 164 ++++++++++++++++++ 5 files changed, 254 insertions(+) create mode 100644 tests/fuzz_images/oor_panic/graydepth-invert-overflow-5a0aad5024cc4ff3.tiff create mode 100644 tests/fuzz_images/oor_panic/graydepth-invert-overflow-9f2f479685f84884.tiff diff --git a/src/decoder/image.rs b/src/decoder/image.rs index b90e25c..3cfaa48 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -1204,6 +1204,14 @@ impl Image { scratch, ); + // An n-bit predictor delta is stored modulo 2^n, but the + // reconstruction above accumulated it in the promoted u16/u32 + // storage width. Reduce back modulo the declared depth so both + // the inverted and non-inverted paths see exact, in-range + // values (2^n divides the storage modulus, so one mask after + // the row is exact). + super::mask_samples_to_bit_depth(out_row, tiff_bps, out_bps, self.sample_format); + if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { super::invert_colors(out_row, color_type, self.sample_format)?; } @@ -1224,6 +1232,16 @@ impl Image { ); } + // No-op in this branch: promoted layouts (declared depth < storage + // width) are handled exclusively by the unpacking branch above. + // Called in every integer branch so they cannot diverge. + super::mask_samples_to_bit_depth( + tile, + layout.tiff_bits_per_sample, + layout.output_bits_per_sample, + self.sample_format, + ); + if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { super::invert_colors(tile, color_type, self.sample_format)?; } @@ -1250,6 +1268,8 @@ impl Image { } _ => unreachable!(), } + // No masking here: the floating point predictor requires + // IEEEFP samples, which are never depth-promoted. if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { super::invert_colors(row, color_type, self.sample_format)?; } @@ -1279,6 +1299,16 @@ impl Image { scratch, ); + // No-op in this branch: promoted layouts only occur in the + // unpacking branch. Called so the integer branches cannot + // diverge. + super::mask_samples_to_bit_depth( + row, + layout.tiff_bits_per_sample, + layout.output_bits_per_sample, + self.sample_format, + ); + if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { super::invert_colors(row, color_type, self.sample_format)?; } @@ -1321,6 +1351,16 @@ impl Image { scratch, ); + // No-op in this branch: promoted layouts only occur in the + // unpacking branch. Called so the integer branches cannot + // diverge. + super::mask_samples_to_bit_depth( + row, + layout.tiff_bits_per_sample, + layout.output_bits_per_sample, + self.sample_format, + ); + if photometric_interpretation == PhotometricInterpretation::WhiteIsZero { super::invert_colors(row, color_type, self.sample_format)?; } diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index f2f39af..b242a33 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -568,6 +568,56 @@ fn fix_endianness_and_predict( } } +/// Reduce promoted extended-depth samples (9–15 bits → `u16`, 17–31 bits → +/// `u32`) modulo their declared bit depth, i.e. mask with `(1 << n) - 1`. +/// +/// An n-bit horizontal-predicted sample stores its delta in exactly n bits, +/// i.e. modulo `2^n`, so reconstruction is also defined modulo `2^n`. The +/// predictor accumulates `prev + delta` in the promoted storage width +/// (`u16`/`u32`, see `rev_hpredict_nsamp`); since `2^n` divides the storage +/// modulus, masking once after the predictor yields exactly the mod-`2^n` +/// value for every sample — including legitimate wrap-arounds (a 9-bit +/// `511, 0` pair is stored as delta `1`, and `511 + 1 = 512` masks back to +/// `0`). Masking also bounds any out-of-range sample in malformed input, so +/// all downstream consumers (`invert_colors`' `max - v` and the non-inverted +/// output path) see in-range values. For samples already in range it is a +/// no-op. +/// +/// This runs in every integer decode branch of `expand_chunk` so the +/// branches cannot diverge. Outside the unpacking branch it reduces to the +/// two guards below and touches nothing: promoted layouts (`declared < +/// storage`) exist only there — standard depths have `declared == storage`, +/// and sub-byte depths keep byte-granular storage. +#[inline] +fn mask_samples_to_bit_depth( + buf: &mut [u8], + declared_bits: u8, + storage_bits: u8, + sample_format: SampleFormat, +) { + // Only unsigned samples are promoted with zero-extension; signed + // extended depths would need sign extension instead and are not + // supported by the unpacking path. + if sample_format != SampleFormat::Uint { + return; + } + match storage_bits { + 16 if declared_bits < 16 => { + let mask = (1u16 << declared_bits) - 1; + for chunk in buf.as_chunks_mut::<2>().0 { + *chunk = (u16::from_ne_bytes(*chunk) & mask).to_ne_bytes(); + } + } + 32 if declared_bits < 32 => { + let mask = (1u32 << declared_bits) - 1; + for chunk in buf.as_chunks_mut::<4>().0 { + *chunk = (u32::from_ne_bytes(*chunk) & mask).to_ne_bytes(); + } + } + _ => {} + } +} + fn invert_colors( buf: &mut [u8], color_type: ColorType, diff --git a/tests/fuzz_images/oor_panic/graydepth-invert-overflow-5a0aad5024cc4ff3.tiff b/tests/fuzz_images/oor_panic/graydepth-invert-overflow-5a0aad5024cc4ff3.tiff new file mode 100644 index 0000000000000000000000000000000000000000..21c7a2c29bd3362bcfe801cd8d4893330f76bcd0 GIT binary patch literal 2406 zcmc&$J!lkB5dP*iZq}$V7m`hJA}6Bgc|r_Wh=wGh!XmMWRtgGM+6WdFf*7!|6csE* zun;>z6jN$vsTVCJVj~Koq9Nkm_|1E}cbChLK@o?$+qdt{o1bs~E=ejl0i&7M`tpZr zA0&nVBG7e^Np#xbuy$hqPuoYTq`H0!Bt@A!sq#m4mQ1`kijPQ=O365jf3KAg>oUEz@&?%uV<1au)!^YY;Q4(&tRYZ#l=m5^2=X(?iiqh%NYDw}+hYsdy z258?`@|YTlWq`s%zHYnj5O^_@ko9D&wCO4bbZ*2J;aU`n^=739mw!Hb-z2_;@8^yr*9Pg)Q6X|BH zQbk$9vS}J4OPVBh-rL)I^&N>%(mT4b<$#0*-w)75MkBkAZsmibrCXHGZflcHAghGy`uf_&R$#EC-(`FJM{-e9s@ z6sXykLQLBXla;oVc|xK_OKGhq$c3YIhw$ur0xuJ;dHK$jNVsH=nUXUGNZSO;`w5S% zpR|Q`C~I8p{Ui^Mg$M{Nw5C_Fv7+@4KU-$Q5|fYpW`Ln~W7G}bVU%*9;vM~xWl)!& z$=>d~6*+b0Xj9$Hw43P#ie2dA3U%>cz-C@n-9S%DbGs~o`sMoznu5>NRggrsFN3bv z+*X5)G#G2-0dpbp?iqGIl2)pIbC@e7dx literal 0 HcmV?d00001 diff --git a/tests/fuzz_images/oor_panic/graydepth-invert-overflow-9f2f479685f84884.tiff b/tests/fuzz_images/oor_panic/graydepth-invert-overflow-9f2f479685f84884.tiff new file mode 100644 index 0000000000000000000000000000000000000000..86824ecd45bf84aee7f0ac705a9edab1487d436b GIT binary patch literal 2775 zcmeHJziSjx5dP-wvbO{;KhC%TV-Y1_h=FL4bVm}wV(lbaX^~E`38WA##FPP>fQ6+b z1pfrVAXr#wYod)Hu}Pi~{p$`5w;?R)cP=FOXL-izZg+<;;GodT)@ zkTL`ifz}{PsM=sdtGFoEk3=1x%aUN2J&E$A`jtW)p1}^{cx=cdH>`!%Plbxn7?PUE zGqxZhB8f}_A?;X;k4V6Op-nvE$R_d0We^4&jn1KhN$;rBw)aeAPtY+2Q7{R7l#&On0_|HgXZ@kEMgC=Lau+yZDDP|(M6`uvk4_~0 zQcV}K8OLvR>QXbBgPB8FGzaNwKoOCHfHy^E(}24OZI9N?X$u+5u!q`1(&APmvjGi{ zG3?wrh;0a^LWHUxESjdm{I*`sOats$z}_8GYZA;0m*S@z-&AaM^pu)azt~thkMG~x zn|bQ^+n6tiS4V;ltJ#kiV|PlDc1Sfar?;HX9dnvF;GAzsdfFLpN&0(bOh4waeN)t`=(<^h$mYhht!#WK>!k)1C2^gPr#nCdD1e)7*y z+^5cx45*hq-YAMyjFy7I`q0RDXFJp6AUg;BFw605^v~dKhPVZutbR|iyNbizH~OMt z=4RPQtYPa)#!;7nx&*uqumtq*Q##Kva^L8$?CoJc#hYh)7j9-3h}wwRs$zXTodCTq zJol&UZ;g{3YlL1F^b3wT9(bEQs&{`0S}u4>*ar3("tiled-jpeg-ycbcr.tif", ColorType::YCbCr(8)); } + +// --------------------------------------------------------------------------- +// Extended bit depths (9-15 promoted to u16, 17-31 promoted to u32). +// +// A horizontal-predicted n-bit sample stores its delta in exactly n bits, +// i.e. modulo 2^n, so reconstruction must also be modulo 2^n. The decoder +// accumulates in the promoted storage width and reduces back to n bits; +// these tests pin that reduction down with exact pixel values, including +// deltas that wrap modulo 2^n. The encoder does not support extended +// depths, so the TIFFs are hand-built. +// --------------------------------------------------------------------------- + +/// Hand-build a minimal little-endian, single-strip, uncompressed grayscale +/// TIFF with `Predictor::Horizontal` and the given (possibly non-standard) +/// bit depth. `pixels` are the intended sample values; they are converted to +/// horizontal differences modulo `2^bits` and bit-packed MSB-first with each +/// row padded to a byte boundary — the only representation an n-bit field +/// can hold. +fn build_gray_hpredict_tiff( + width: u32, + height: u32, + bits: u8, + photometric: u16, + pixels: &[u32], +) -> Vec { + let max = ((1u64 << bits) - 1) as u32; + let row_samples = width as usize; + assert_eq!(pixels.len(), row_samples * height as usize); + assert!(pixels.iter().all(|&v| v <= max)); + + // Per-row horizontal differencing modulo 2^bits. + let mut deltas = Vec::with_capacity(pixels.len()); + for row in pixels.chunks(row_samples) { + for (i, &v) in row.iter().enumerate() { + deltas.push(if i == 0 { + v + } else { + v.wrapping_sub(row[i - 1]) & max + }); + } + } + + // Bit-pack MSB-first, each row padded to a byte boundary. + let row_bytes = (row_samples * bits as usize).div_ceil(8); + let mut data = vec![0u8; row_bytes * height as usize]; + for (r, row) in deltas.chunks(row_samples).enumerate() { + for (i, &d) in row.iter().enumerate() { + let start = r * row_bytes * 8 + i * bits as usize; + for (off, b) in (start..).zip((0..bits).rev()) { + if (d >> b) & 1 == 1 { + data[off / 8] |= 0x80 >> (off % 8); + } + } + } + } + + // Classic little-endian TIFF: header, strip data, then a single IFD. + let mut out = Vec::new(); + out.extend_from_slice(b"II"); + out.extend_from_slice(&42u16.to_le_bytes()); + let data_offset = 8u32; + let ifd_offset = data_offset + data.len() as u32; + out.extend_from_slice(&ifd_offset.to_le_bytes()); + out.extend_from_slice(&data); + + const SHORT: u16 = 3; + const LONG: u16 = 4; + let entries: &[(u16, u16, u32)] = &[ + (256, LONG, width), // ImageWidth + (257, LONG, height), // ImageLength + (258, SHORT, u32::from(bits)), // BitsPerSample + (259, SHORT, 1), // Compression: none + (262, SHORT, u32::from(photometric)), // PhotometricInterpretation + (273, LONG, data_offset), // StripOffsets + (277, SHORT, 1), // SamplesPerPixel + (278, LONG, height), // RowsPerStrip + (279, LONG, data.len() as u32), // StripByteCounts + (317, SHORT, 2), // Predictor: horizontal + (339, SHORT, 1), // SampleFormat: unsigned integer + ]; + out.extend_from_slice(&(entries.len() as u16).to_le_bytes()); + for &(tag, ty, value) in entries { + out.extend_from_slice(&tag.to_le_bytes()); + out.extend_from_slice(&ty.to_le_bytes()); + out.extend_from_slice(&1u32.to_le_bytes()); + // A count-1 SHORT payload occupies the low bytes of the value field. + out.extend_from_slice(&value.to_le_bytes()); + } + out.extend_from_slice(&0u32.to_le_bytes()); // no next IFD + out +} + +fn decode_gray_u16(tiff: Vec, bits: u8) -> Vec { + let mut decoder = Decoder::open(Cursor::new(tiff)).expect("Cannot create decoder"); + decoder.next_image().unwrap(); + assert_eq!(decoder.colortype().unwrap(), ColorType::Gray(bits)); + match decoder.read_image().unwrap() { + DecodingSampleBuffer::U16(res) => res, + _ => panic!("Wrong data type"), + } +} + +fn decode_gray_u32(tiff: Vec, bits: u8) -> Vec { + let mut decoder = Decoder::open(Cursor::new(tiff)).expect("Cannot create decoder"); + decoder.next_image().unwrap(); + assert_eq!(decoder.colortype().unwrap(), ColorType::Gray(bits)); + match decoder.read_image().unwrap() { + DecodingSampleBuffer::U32(res) => res, + _ => panic!("Wrong data type"), + } +} + +#[test] +fn test_gray12_hpredict_wraparound_exact() { + // Pixel row [4095, 0, 1] encodes as deltas [4095, 1, 1] (mod 2^12). + // u16 accumulation yields [4095, 4096, 4097]; reduction modulo 2^12 + // must restore [4095, 0, 1]. Saturating instead would corrupt the two + // wrapped samples to 4095. + let tiff = build_gray_hpredict_tiff(3, 1, 12, 1, &[4095, 0, 1]); + assert_eq!(decode_gray_u16(tiff, 12), vec![4095, 0, 1]); +} + +#[test] +fn test_gray12_hpredict_wraparound_white_is_zero_exact() { + // Same stored samples with PhotometricInterpretation = WhiteIsZero: + // after the mod-2^12 reduction the decoder inverts as (2^12 - 1) - v. + let tiff = build_gray_hpredict_tiff(3, 1, 12, 0, &[4095, 0, 1]); + assert_eq!(decode_gray_u16(tiff, 12), vec![0, 4095, 4094]); +} + +#[test] +fn test_gray20_hpredict_wraparound_exact() { + // The same wrap-around shape on the u32 storage path (17-31 bits). + let tiff = build_gray_hpredict_tiff(3, 1, 20, 1, &[0xF_FFFF, 0, 1]); + assert_eq!(decode_gray_u32(tiff, 20), vec![0xF_FFFF, 0, 1]); +} + +#[test] +fn test_extended_depth_hpredict_random_roundtrip() { + // Deterministic pseudo-random pixels across a spread of extended depths; + // encode-then-decode must be the identity for every sample, including + // every mod-2^n wrap-around the differencing produces. Three rows verify + // that prediction restarts per row. + let mut state = 0x243F_6A88_85A3_08D3u64; + let mut next = move || { + state = state + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + (state >> 33) as u32 + }; + for &bits in &[9u8, 11, 12, 15] { + let max = (1u32 << bits) - 1; + let pixels: Vec = (0..16 * 3).map(|_| next() & max).collect(); + let tiff = build_gray_hpredict_tiff(16, 3, bits, 1, &pixels); + let expected: Vec = pixels.iter().map(|&v| v as u16).collect(); + assert_eq!(decode_gray_u16(tiff, bits), expected, "bits={bits}"); + } + for &bits in &[17u8, 20, 25, 31] { + let max = ((1u64 << bits) - 1) as u32; + let pixels: Vec = (0..16 * 3).map(|_| next() & max).collect(); + let tiff = build_gray_hpredict_tiff(16, 3, bits, 1, &pixels); + assert_eq!(decode_gray_u32(tiff, bits), pixels, "bits={bits}"); + } +}