collective
Browse / github.com/storytold/lightcraft / patch
security(raster): fallible image allocation; no panic on empty images or zip_map size mismatch
security(raster): fallible image allocation; no panic on empty images or zip_map size mismatch A raw or image whose header claims a huge picture could abort the whole app when the buffer couldn't be allocated (an abort, not a catchable panic, so the user loses unsaved work). Two raster helpers could also panic: \`get_clamped\` on an image with no pixels, and \`zip_map\` on two images of different sizes. What changed (crates/raster, crates/raw): - New \`try_vec\`, \`Image::try_new\`, \`Image::try_filled\`: return \`AllocError\` when the size overflows, passes 64 GiB (\`MAX_IMAGE_BYTES\`), or the allocator refuses (\`try_reserve_exact\`). - \`RawImage::develop\` checks up front that the
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- emmertex (imported from pull request #3008, open)
- Base
387bed199fca44d76af5aa1c24dd7cc3de4c8db2- Size
- +142 −9 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
3dc1a6f03fcfa5dae9ede65d
How it went
No other operator has reported using it yet.
Diff
raw · crates/raster/src/lib.rs crates/raw/src/lib.rs
@@ -47,6 +47,40 @@ pub fn par_join<A: Send, B: Send>(a: impl FnOnce() -> A + Send, b: impl FnOnce() } } +/// An image buffer that could not be allocated (its size overflows, passes [`MAX_IMAGE_BYTES`], or the+/// allocator refused it): returned by [`Image::try_new`] instead of aborting the process.+#[derive(Clone, Copy, Debug, PartialEq, Eq)]+pub struct AllocError {+ pub width: usize,+ pub height: usize,+}++impl std::fmt::Display for AllocError {+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {+ write!(f, "not enough memory for a {}×{} image", self.width, self.height)+ }+}++impl std::error::Error for AllocError {}++/// The largest single image buffer [`Image::try_new`] allocates (64 GiB, or `isize::MAX` bytes on+/// 32-bit targets such as wasm32, where 64 GiB doesn't fit in `usize`).+pub const MAX_IMAGE_BYTES: usize = if usize::BITS >= 64 { 1 << 36 } else { isize::MAX as usize };++/// A vector of `n` copies of `v`, or an error when it can't be had (overflow, past [`MAX_IMAGE_BYTES`],+/// allocator failure) instead of an abort. `width`/`height` only label the error.+pub fn try_vec<T: Copy>(n: usize, v: T, width: usize, height: usize) -> Result<Vec<T>, AllocError> {+ let err = AllocError { width, height };+ let bytes = n.checked_mul(std::mem::size_of::<T>().max(1)).ok_or(err)?;+ if bytes > MAX_IMAGE_BYTES {+ return Err(err);+ }+ let mut data = Vec::new();+ data.try_reserve_exact(n).map_err(|_| err)?;+ data.resize_with(n, || v);+ Ok(data)+}+ /// Interleaved row-major image. #[derive(Clone, Debug, PartialEq)] pub struct Image<T> {@@ -63,6 +97,15 @@ impl<T: Copy + Default> Image<T> { pub fn new(width: usize, height: usize) -> Self { Self { width, height, data: vec![T::default(); width * height] } }+ /// [`Self::new`] that returns an error instead of aborting when the buffer can't be allocated.+ pub fn try_new(width: usize, height: usize) -> Result<Self, AllocError> {+ Self::try_filled(width, height, T::default())+ }+ /// [`Self::filled`] that returns an error instead of aborting when the buffer can't be allocated.+ pub fn try_filled(width: usize, height: usize, v: T) -> Result<Self, AllocError> {+ let n = width.checked_mul(height).ok_or(AllocError { width, height })?;+ Ok(Self { width, height, data: try_vec(n, v, width, height)? })+ } pub fn filled(width: usize, height: usize, v: T) -> Self { Self { width, height, data: vec![v; width * height] } }@@ -79,12 +122,16 @@ impl<T: Copy + Default> Image<T> { pub fn get(&self, x: usize, y: usize) -> T { self.data[y * self.width + x] }- /// Clamped access (edge extension).+ /// Clamped access (edge extension). An image with no pixels (zero width or height, or a+ /// buffer shorter than its size) reads as `T::default()` instead of panicking. #[inline] pub fn get_clamped(&self, x: isize, y: isize) -> T {- let x = x.clamp(0, self.width as isize - 1) as usize;- let y = y.clamp(0, self.height as isize - 1) as usize;- self.data[y * self.width + x]+ if self.width == 0 || self.height == 0 {+ return T::default();+ }+ let x = x.clamp(0, (self.width - 1).min(isize::MAX as usize) as isize) as usize;+ let y = y.clamp(0, (self.height - 1).min(isize::MAX as usize) as isize) as usize;+ y.checked_mul(self.width).and_then(|i| i.checked_add(x)).and_then(|i| self.data.get(i).copied()).unwrap_or_default() } #[inline] pub fn set(&mut self, x: usize, y: usize, v: T) {@@ -144,18 +191,31 @@ impl<T: Copy + Default> Image<T> { }); out }- /// Per-pixel combination of two images of the same size (row-parallel).+ /// Per-pixel combination of two images of the same size (row-parallel). The output has `self`'s size;+ /// if `other` is smaller (a caller bug), its missing pixels read as `U::default()` instead of panicking. pub fn zip_map<U: Copy + Default + Sync, V: Copy + Default + Send>(&self, other: &Image<U>, f: impl Fn(T, U) -> V + Sync + Send) -> Image<V> where T: Sync, {- assert_eq!((self.width, self.height), (other.width, other.height), "zip_map: size mismatch"); let mut out = Image::<V>::new(self.width, self.height); let w = self.width;+ let same = (self.width, self.height) == (other.width, other.height) && other.data.len() >= w * self.height; par_rows(&mut out.data, w, |y, row| {- let (a, b) = (&self.data[y * w..(y + 1) * w], &other.data[y * w..(y + 1) * w]);- for ((o, s), t) in row.iter_mut().zip(a).zip(b) {- *o = f(*s, *t);+ let a = self.data.get(y * w..(y + 1) * w).unwrap_or(&[]);+ if same {+ let b = other.data.get(y * w..(y + 1) * w).unwrap_or(&[]);+ for ((o, s), t) in row.iter_mut().zip(a).zip(b) {+ *o = f(*s, *t);+ }+ } else {+ for (x, (o, s)) in row.iter_mut().zip(a).enumerate() {+ let t = if x < other.width && y < other.height {+ y.checked_mul(other.width).and_then(|i| i.checked_add(x)).and_then(|i| other.data.get(i)).copied().unwrap_or_default()+ } else {+ U::default()+ };+ *o = f(*s, t);+ } } }); out@@ -298,9 +358,80 @@ impl Rgba8 { #[cfg(test)] mod tests {+ #[test]+ fn try_vec_fills_without_calling_clone() {+ use std::sync::atomic::{AtomicUsize, Ordering};+ static CLONES: AtomicUsize = AtomicUsize::new(0);+ #[derive(Copy, PartialEq, Debug)]+ struct Px(u8);+ #[allow(clippy::non_canonical_clone_impl)]+ impl Clone for Px {+ fn clone(&self) -> Self {+ CLONES.fetch_add(1, Ordering::SeqCst);+ *self+ }+ }+ let v = super::try_vec(4, Px(7), 2, 2).unwrap();+ assert!(v.len() == 4 && v.iter().all(|p| p.0 == 7));+ assert_eq!(CLONES.load(Ordering::SeqCst), 0, "filling must not invoke Clone");+ }++ #[test]+ fn get_clamped_on_empty_image_does_not_panic() {+ for (w, h) in [(0, 0), (0, 5), (5, 0)] {+ let img = Plane::new(w, h);+ for (x, y) in [(0, 0), (-3, 2), (7, -1), (isize::MIN, isize::MAX)] {+ assert_eq!(img.get_clamped(x, y), 0.0);+ }+ let rgb = Rgb32f::new(w, h);+ assert_eq!(rgb.sample_bilinear(1.5, 0.5), [0.0; 3]);+ }+ }++ #[test]+ fn get_clamped_short_buffer_reads_default() {+ let img = Plane { width: 4, height: 4, data: vec![1.0; 3] };+ assert_eq!(img.get_clamped(1, 0), 1.0);+ assert_eq!(img.get_clamped(3, 3), 0.0);+ }++ #[test]+ fn get_clamped_still_extends_edges() {+ let img = Plane::from_fn(3, 2, |x, y| (x + 10 * y) as f32);+ assert_eq!(img.get_clamped(-5, -5), 0.0);+ assert_eq!(img.get_clamped(9, 9), 12.0);+ assert_eq!(img.get_clamped(1, 1), 11.0);+ }+ use super::*; use lightcraft_geom::Orientation; + #[test]+ fn huge_images_are_an_error_not_an_abort() {+ assert!(Rgb32f::try_new(usize::MAX / 2, 4).is_err());+ assert!(Rgb32f::try_new(1 << 20, 1 << 20).is_err());+ let ok = Plane::try_filled(3, 2, 0.5).unwrap();+ assert_eq!(ok, Plane::filled(3, 2, 0.5));+ }++ #[test]+ fn zip_map_with_mismatched_sizes_does_not_panic() {+ let a = Plane::filled(4, 3, 1.0);+ let b = Plane::filled(2, 2, 2.0);+ let c = a.zip_map(&b, |x, y| x + y);+ assert_eq!((c.width, c.height), (4, 3));+ assert_eq!(c.get(1, 1), 3.0);+ assert_eq!(c.get(3, 2), 1.0);+ // a hostile `other` whose width times row overflows reads defaults instead of panicking+ let wide = Plane { width: usize::MAX, height: 3, data: vec![5.0; 2] };+ let narrow = Plane::filled(1, 3, 1.0);+ let d = narrow.zip_map(&wide, |x, y| x + y);+ assert_eq!(d.get(0, 0), 6.0);+ assert_eq!(d.get(0, 2), 1.0);+ let same = a.zip_map(&a, |x, y| x + y);+ assert!(same.data.iter().all(|&v| v == 2.0));+ }+ /// A sample at an infinite coordinate (the inverse mapping of a crop of no size) cast to /// isize::MAX, and its right / lower neighbour overflowed (a panic where overflow is checked): /// the neighbours saturate and the edge pixel comes back.@@ -866,6 +866,8 @@ impl RawImage { /// The result is camera RGB (not white balanced), white level = 1.0, not oriented. pub fn develop(&self, method: Method) -> Result<Rgb32f> { let n = self.normalized()?;+ // the demosaiced picture is the biggest buffer: refuse it with an error rather than abort on allocation+ drop(lightcraft_raster::Rgb32f::try_new(n.width, n.height).map_err(|_| RawError::Limit("not enough memory to develop this raw"))?); let rgb = demosaic(&n, method); drop(n); Ok(self.finish_demosaiced(rgb))