Merge pull request #1221 from dotCooCoo/fix/hdr-isize-bound

craft-hdr: refuse buffers past isize::MAX bytes when limits are raised

crates/hdr/src/lib.rs

@@ -11,7 +11,9 @@
 //!
 //! Input is treated as hostile: the header length and the picture size are capped by [`Limits`],
 //! and every scan line is validated before the float picture is allocated, so a short file cannot
-//! ask for a large allocation.
+//! ask for a large allocation. Whatever the limits, no buffer is requested past the target's
+//! largest allocation (`isize::MAX` bytes), so raised limits on a 32-bit target such as `wasm32`
+//! give [`Error::LimitExceeded`] instead of a panic.
 
 #[cfg(test)]
 mod tests;
@@ -42,6 +44,7 @@ pub type Result<T> = std::result::Result<T, Error>;
 #[non_exhaustive]
 pub struct Limits {
     /// Most pixels (width × height) a picture may have; each takes 12 bytes decoded. Default 2^26.
+    /// A picture over the target's largest allocation is refused at any setting.
     pub max_pixels: u64,
     /// Longest header read, in bytes (real ones are a few hundred). Default 64 KiB.
     pub max_header: usize,
@@ -62,6 +65,27 @@ pub struct Picture {
     pub rgb: Vec<f32>,
 }
 
+/// The largest allocation in bytes that a `Vec` or `String` can make on the target.
+const MAX_ALLOC_BYTES: u64 = isize::MAX as u64;
+
+/// Bytes taken by `count` elements of `element_bytes` each, or `None` when the count overflows `u64`.
+fn checked_byte_len(count: u64, element_bytes: usize) -> Option<u64> {
+    count.checked_mul(element_bytes as u64)
+}
+
+/// `count` as a length when `count` elements of `element_bytes` each fit in `max_alloc_bytes`.
+fn alloc_len(count: u64, element_bytes: usize, max_alloc_bytes: u64) -> Option<usize> {
+    checked_byte_len(count, element_bytes).filter(|&bytes| bytes <= max_alloc_bytes).and_then(|_| usize::try_from(count).ok())
+}
+
+/// Bytes of `String::from_utf8_lossy(bytes)`: each invalid sequence becomes one U+FFFD (3 bytes).
+fn lossy_len(bytes: &[u8]) -> Option<u64> {
+    bytes.utf8_chunks().try_fold(0u64, |n, chunk| {
+        let invalid = if chunk.invalid().is_empty() { 0 } else { '\u{FFFD}'.len_utf8() };
+        n.checked_add(chunk.valid().len() as u64)?.checked_add(invalid as u64)
+    })
+}
+
 /// Whether `bytes` starts like a Radiance picture.
 pub fn sniff(bytes: &[u8]) -> bool {
     bytes.starts_with(b"#?RADIANCE") || bytes.starts_with(b"#?RGBE")
@@ -158,6 +182,11 @@ fn scanline(b: &[u8], pos: &mut usize, line: &mut [[u8; 4]]) -> Result<()> {
 
 /// Decodes a Radiance picture to linear float RGB, top row first.
 pub fn decode(b: &[u8], limits: &Limits) -> Result<Picture> {
+    decode_within(b, limits, MAX_ALLOC_BYTES)
+}
+
+/// [`decode`] with `max_alloc_bytes` as the largest allocation the target allows.
+fn decode_within(b: &[u8], limits: &Limits, max_alloc_bytes: u64) -> Result<Picture> {
     if !sniff(b) {
         return Err(Error::NotHdr);
     }
@@ -166,7 +195,11 @@ pub fn decode(b: &[u8], limits: &Limits) -> Result<Picture> {
     let line = |pos: &mut usize| -> Result<String> {
         let rest = b.get(*pos..).ok_or(Error::Malformed("truncated header"))?;
         let end = rest.iter().take(limits.max_header).position(|&c| c == b'\n').ok_or(Error::Malformed("truncated header"))?;
-        let s = String::from_utf8_lossy(rest.get(..end).unwrap_or_default()).trim_end_matches('\r').to_string();
+        let raw = rest.get(..end).unwrap_or_default();
+        if lossy_len(raw).is_none_or(|n| n > max_alloc_bytes) {
+            return Err(Error::LimitExceeded("header length"));
+        }
+        let s = String::from_utf8_lossy(raw).trim_end_matches('\r').to_string();
         *pos += end + 1;
         Ok(s)
     };
@@ -188,8 +221,9 @@ pub fn decode(b: &[u8], limits: &Limits) -> Result<Picture> {
     }
     let res = line(&mut pos)?;
     let bad = Error::Malformed("bad resolution line");
-    let t: Vec<&str> = res.split_whitespace().collect();
-    let &[a1, n1, a2, n2] = t.as_slice().first_chunk::<4>().ok_or(bad.clone())?;
+    // Tokens after the fourth are ignored, so they are not collected.
+    let mut t = res.split_whitespace();
+    let (Some(a1), Some(n1), Some(a2), Some(n2)) = (t.next(), t.next(), t.next(), t.next()) else { return Err(bad) };
     let ((x1, p1), (x2, p2)) = (axis(a1).ok_or(bad.clone())?, axis(a2).ok_or(bad.clone())?);
     if x1 == x2 {
         return Err(Error::Malformed("resolution line names one axis twice"));
@@ -205,6 +239,10 @@ pub fn decode(b: &[u8], limits: &Limits) -> Result<Picture> {
     if pixels > limits.max_pixels {
         return Err(Error::LimitExceeded("pixel count"));
     }
+    // One scan line of RGBE pixels and the float picture, each within the target's largest allocation.
+    let scan_len = alloc_len(u64::from(if x1 { height } else { width }), size_of::<[u8; 4]>(), max_alloc_bytes);
+    let rgb_len = pixels.checked_mul(3).and_then(|floats| alloc_len(floats, size_of::<f32>(), max_alloc_bytes));
+    let (Some(scan_len), Some(total)) = (scan_len, rgb_len) else { return Err(Error::LimitExceeded("pixel count")) };
     // A scan line takes at least a few bits of file, so a header cannot ask for more than the data has.
     if (b.len().saturating_sub(pos) as u64).saturating_mul(4096) < pixels {
         return Err(Error::Malformed("truncated pixel data"));
@@ -212,12 +250,11 @@ pub fn decode(b: &[u8], limits: &Limits) -> Result<Picture> {
     // Validate every encoded scan line before allocating the float picture: a short file could
     // otherwise ask for the whole limit of zeroed pixels and only then fail on its first line.
     let pixel_data = pos;
-    let mut scan = vec![[0u8; 4]; n2];
+    let mut scan = vec![[0u8; 4]; scan_len];
     for _ in 0..n1 {
         scanline(b, &mut pos, &mut scan)?;
     }
 
-    let total = w.checked_mul(h).and_then(|n| n.checked_mul(3)).ok_or(Error::LimitExceeded("pixel count"))?;
     let mut rgb = vec![0f32; total];
     pos = pixel_data;
     for i in 0..n1 {

crates/hdr/src/tests.rs

@@ -142,3 +142,57 @@ fn limits_are_checked_before_allocating() {
     assert_eq!(decode(&long, &Limits { max_header: 8, ..Limits::default() }), Err(Error::Malformed("truncated header")));
     assert_eq!((Limits::default().max_pixels, Limits::default().max_header), (1 << 26, 64 << 10));
 }
+
+/// `isize::MAX` on a 32-bit target such as `wasm32`.
+const BOUND_32: u64 = i32::MAX as u64;
+
+#[test]
+fn buffer_lengths_respect_target_isize_bound() {
+    // A scan line of RGBE pixels, 4 bytes each.
+    assert_eq!(alloc_len(536_870_911, 4, BOUND_32), Some(536_870_911));
+    assert_eq!(alloc_len(536_870_912, 4, BOUND_32), None);
+    assert_eq!(alloc_len(u64::from(u32::MAX), 4, BOUND_32), None);
+    // The float picture: 3 floats of 4 bytes a pixel.
+    assert_eq!(alloc_len(178_956_970 * 3, 4, BOUND_32), Some(178_956_970 * 3));
+    assert_eq!(alloc_len(178_956_971 * 3, 4, BOUND_32), None);
+    // The default pixel limit fits a 32-bit target.
+    assert_eq!(alloc_len(Limits::default().max_pixels * 3, 4, BOUND_32), Some(3 << 26));
+    assert_eq!(checked_byte_len(u64::MAX, 4), None);
+    assert_eq!(alloc_len(u64::MAX, 1, MAX_ALLOC_BYTES), None);
+}
+
+#[test]
+fn raised_pixel_limit_stops_at_the_target_bound() {
+    let open = Limits { max_pixels: u64::MAX, ..Limits::default() };
+    // 2 × 2 needs an 8-byte scan line and a 48-byte float picture.
+    let ok = file("-Y 2 +X 2", &[0; 16]);
+    assert!(decode_within(&ok, &open, 48).is_ok());
+    assert_eq!(decode_within(&ok, &open, 47), Err(Error::LimitExceeded("pixel count")));
+    // A 1 × 536,870,912 picture is past the 32-bit bound in both buffers; refused before
+    // anything is allocated, ahead of the scan-line validation.
+    let wide = file("-Y 1 +X 536870912", &[0; 131_072]);
+    assert_eq!(decode_within(&wide, &open, BOUND_32), Err(Error::LimitExceeded("pixel count")));
+    // 178,956,971 one-pixel scan lines fit the scan buffer but not the float picture.
+    let tall = file("-Y 178956971 +X 1", &[0; 43_692]);
+    assert_eq!(decode_within(&tall, &open, BOUND_32), Err(Error::LimitExceeded("pixel count")));
+}
+
+#[test]
+fn header_lines_are_measured_after_lossy_conversion() {
+    // `#` and ten bytes that are not UTF-8: 11 bytes in the file, 31 as text.
+    let mut f = b"#?RADIANCE\n#".to_vec();
+    f.extend_from_slice(&[0xff; 10]);
+    f.extend_from_slice(b"\n\n-Y 1 +X 1\n\0\0\0\0");
+    assert_eq!(lossy_len(&f[11..22]), Some(31));
+    assert!(decode_within(&f, &Limits::default(), 31).is_ok());
+    assert_eq!(decode_within(&f, &Limits::default(), 30), Err(Error::LimitExceeded("header length")));
+    assert_eq!(lossy_len(b"ok \xe2\x82"), Some(6));
+    assert_eq!(lossy_len(b""), Some(0));
+}
+
+#[test]
+fn resolution_tokens_after_the_fourth_are_ignored() {
+    let f = dec(&file("-Y 1 +X 1 a b c d e f", &[0; 4])).unwrap();
+    assert_eq!((f.width, f.height), (1, 1));
+    assert_eq!(dec(&file("-Y 1 +X", &[0; 4])), Err(Error::Malformed("bad resolution line")));
+}