diff --git a/crates/meta/src/xmp.rs b/crates/meta/src/xmp.rs --- a/crates/meta/src/xmp.rs +++ b/crates/meta/src/xmp.rs @@ -234,7 +234,18 @@ pub(crate) fn unescape_attr(s: &str) -> String { while let Some(i) = rest.find('&') { out.push_str(&rest[..i]); rest = &rest[i..]; - match rest.find(';').and_then(|j| entity(&rest[1..j]).map(|c| (c, j))) { + // the longest reference is `􏿿` plus leading zeros: skip the zeros (each byte once, since a + // miss resumes after the `&`), then search a short window, not the rest of the value per `&` + let b = rest.as_bytes(); + let digits = match (b.get(1), b.get(2)) { + (Some(b'#'), Some(b'x' | b'X')) => 3, + (Some(b'#'), _) => 2, + _ => 1, + }; + let zeros = b.iter().skip(digits).take_while(|&&c| c == b'0').count(); + let skip = digits + zeros.saturating_sub(1); + let end = b.iter().skip(skip).take(12).position(|&c| c == b';').map(|j| j + skip); + match end.and_then(|j| entity(&rest[1..j]).map(|c| (c, j))) { Some((c, j)) => { out.push(c); rest = &rest[j + 1..]; @@ -741,6 +752,24 @@ pub fn write_xmp_lc(meta: &Metadata, lc: &[(&str, &str)]) -> String { mod tests { use super::*; + /// References resolve, anything else (unknown, unterminated, or a `;` too far away to end one) stays as + /// written, and a value that is nothing but `&` is handled in one pass. + #[test] + fn attribute_references_resolve_and_hostile_ones_stay_verbatim() { + assert_eq!(unescape_attr("a&bAB<>"'"), "a&bAB<>\"'"); + assert_eq!(unescape_attr("&bogus; & &#xZZ; tail&"), "&bogus; & &#xZZ; tail&"); + let far = format!("&{};", "x".repeat(40)); + assert_eq!(unescape_attr(&far), far); + let many = "&".repeat(1 << 20); + assert_eq!(unescape_attr(&many), many); + // multi-byte text around a reference is never cut inside a character + assert_eq!(unescape_attr("é&日&é;"), "é&日&é;"); + // leading zeros are valid in numeric references, however many there are + assert_eq!(unescape_attr("AB"), "AB"); + let zeros = format!("&#{};", "0".repeat(1 << 20)); + assert_eq!(unescape_attr(&zeros), zeros); + } + fn full() -> Metadata { Metadata { // write_xmp doesn't emit regions yet (read-only interchange; see docs/xmp-interop.md), so the diff --git a/crates/raw/src/vendor/cr2.rs b/crates/raw/src/vendor/cr2.rs --- a/crates/raw/src/vendor/cr2.rs +++ b/crates/raw/src/vendor/cr2.rs @@ -226,16 +226,19 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let widths: Vec = match &slices { Some(s) => { let n = s[0].min(64) as usize; - let mut v = vec![s[1] as usize; n]; - v.push(s[2] as usize); + // `as usize` would truncate a 64-bit width on 32-bit targets (wasm32, x86) and hide the overflow + let width = |w: u64| usize::try_from(w).map_err(|_| RawError::Corrupt("CR2 slice widths overflow".into())); + let mut v = vec![width(s[1])?; n]; + v.push(width(s[2])?); v } None => { let w = (fw as u64).checked_mul(nc as u64).ok_or_else(|| RawError::Corrupt("bad CR2 frame size".into()))?; - vec![w as usize] + vec![usize::try_from(w).map_err(|_| RawError::Corrupt("bad CR2 frame size".into()))?] } }; - let width: usize = widths.iter().sum(); + // slice widths come from the file: a sum that overflows is a corrupt file, not a wrapped width + let width = widths.iter().try_fold(0usize, |acc, &w| acc.checked_add(w)).ok_or_else(|| RawError::Corrupt("CR2 slice widths overflow".into()))?; if width == 0 || total % width != 0 { return Err(RawError::Corrupt(format!("CR2 slices ({width}) do not divide the frame ({total} samples)"))); } @@ -247,7 +250,11 @@ pub(crate) fn decode(bytes: &[u8], mode: Mode) -> Result { let mut x0 = 0; for &sw in &widths { for y in 0..height { - data[y * width + x0..y * width + x0 + sw].copy_from_slice(&frame.data[i..i + sw]); + // sums to `width`, so these are in range; `get` keeps a surprise an error, not a panic + let (Some(dst), Some(src)) = (data.get_mut(y * width + x0..y * width + x0 + sw), frame.data.get(i..i + sw)) else { + return Err(RawError::Corrupt("CR2 slices outside the frame".into())); + }; + dst.copy_from_slice(src); i += sw; } x0 += sw; diff --git a/crates/raw/tests/it/vendor_robust.rs b/crates/raw/tests/it/vendor_robust.rs --- a/crates/raw/tests/it/vendor_robust.rs +++ b/crates/raw/tests/it/vendor_robust.rs @@ -145,16 +145,21 @@ fn pef(compression: u16) -> Vec { /// A Canon CR2: three placeholder IFDs and IFD3 with a 2-component lossless JPEG frame. fn cr2() -> Vec { + cr2_with_slices(Value::Short(vec![1, 16, 16]), false) +} + +/// The same CR2 with its slice-width tag (0xc640) replaced, optionally in a BigTIFF (for 64-bit widths). +fn cr2_with_slices(slices: Value, bigtiff: bool) -> Vec { let img: Vec = (0..(W * H) as usize).map(|i| 1024 + (i * 7919 % 12000) as u16).collect(); let enc = lightcraft_raw::ljpeg::encode(&img, W as usize / 2, H as usize, 2, 14, 1, 0); let mut ifd0 = IfdBuilder::new(); ifd0.set(t::MAKE, Value::Ascii("Canon".into())); let mut ifd3 = IfdBuilder::new(); ifd3.set(t::COMPRESSION, Value::Short(vec![6])); - ifd3.set(0xc640, Value::Short(vec![1, 16, 16])); + ifd3.set(0xc640, slices); ifd3.set_image(ImageData::Strips { rows_per_strip: H, strips: vec![enc] }); let blank = || IfdBuilder::new().with(1, Value::Short(vec![0])); - TiffWriter::new(ByteOrder::Little, false).write(&[ifd0, blank(), blank(), ifd3]).unwrap() + TiffWriter::new(ByteOrder::Little, bigtiff).write(&[ifd0, blank(), blank(), ifd3]).unwrap() } /// A Fujifilm RAF: header, records, raw block (LE TIFF whose IFD0 points to a 0xf000 raw IFD). @@ -241,6 +246,8 @@ fn samples() -> Vec> { rw2(), cr2(), raf(), + cr2_with_slices(Value::Long8(vec![64, u64::MAX, u64::MAX]), true), + cr2_with_slices(Value::Long8(vec![1, 1 << 40, 16]), true), ] .into_iter() .chain(rw2_formats()) @@ -283,7 +290,7 @@ proptest! { #![proptest_config(ProptestConfig { cases: 2000, .. ProptestConfig::default() })] #[test] - fn mutated_vendor_files_never_panic(kind in 0usize..18, flips in proptest::collection::vec((any::(), any::()), 1..16), cut in any::()) { + fn mutated_vendor_files_never_panic(kind in 0usize..20, flips in proptest::collection::vec((any::(), any::()), 1..16), cut in any::()) { let mut data = samples().swap_remove(kind); let n = data.len(); for (i, v) in flips { @@ -302,3 +309,12 @@ fn rw2_samples_decode_unmutated() { assert_eq!((r.width, r.height), (W as usize, H as usize)); } } + +/// Slice widths come from the file: widths whose sum overflows (or exceeds the frame) are an error, not a +/// wrapped width that indexes outside the frame. +#[test] +fn cr2_slice_widths_that_overflow_are_an_error() { + for slices in [vec![64, u64::MAX, u64::MAX], vec![1, 1 << 40, 16], vec![2, u64::MAX, 1]] { + assert!(lightcraft_raw::decode(&cr2_with_slices(Value::Long8(slices.clone()), true)).is_err(), "{slices:?}"); + } +}