From 09b9ebca3468cc05eda531d14e5bed4ad8a96120 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 08:59:06 -0400 Subject: [PATCH] roto: bound frozen matte decoding and serialization (#2194) * roto: bound frozen matte decoding and serialization * roto: address review (fallible RLE growth, pre-check plane bytes, saturate 32-bit limits) --- crates/effects/src/object_matte.rs | 8 +++- crates/effects/src/roto.rs | 10 ++-- crates/track/src/roto/mod.rs | 52 +++++++++++++++++---- crates/track/src/roto/rle.rs | 74 ++++++++++++++++++++++++------ crates/track/src/roto/tests.rs | 15 ++++++ 5 files changed, 132 insertions(+), 27 deletions(-) diff --git a/crates/effects/src/object_matte.rs b/crates/effects/src/object_matte.rs index 32a196af..d98ec73a 100644 --- a/crates/effects/src/object_matte.rs +++ b/crates/effects/src/object_matte.rs @@ -284,9 +284,13 @@ pub fn frozen_alpha(params: &Params, layer_size: [f64; 2], fps: f64, f: i64, w: return None; } let fr = fc.frames.get(&f)?; - let a = rle::decode(&fr.a)?; let [fw, fh] = fc.size; - if fw == 0 || fh == 0 || a.len() != fw.checked_mul(fh)? { + let expected = fw.checked_mul(fh)?; + if fw == 0 || fh == 0 || expected > rle::MAX_PLANE_BYTES { + return None; + } + let a = rle::decode_bounded(&fr.a, expected)?; + if a.len() != expected { return None; } let af: Vec = a.iter().map(|v| *v as f32 / 255.0).collect(); diff --git a/crates/effects/src/roto.rs b/crates/effects/src/roto.rs index a6c80e74..4917cc23 100644 --- a/crates/effects/src/roto.rs +++ b/crates/effects/src/roto.rs @@ -560,10 +560,14 @@ fn matte_for(ctx: &EffectCtx, f: i64, img: &Arc, scale: f64) -> Option<(V && fc.key == frozen_key(ctx.params, ctx.layer_size, ctx.fps()) && let Some(fr) = fc.frames.get(&f) { - let a = rle::decode(&fr.a)?; - let bnd = rle::decode(&fr.b)?; let [fw, fh] = fc.size; - if a.len() == fw * fh && bnd.len() == fw * fh { + let expected = fw.checked_mul(fh)?; + if fw == 0 || fh == 0 || expected > rle::MAX_PLANE_BYTES { + return None; + } + let a = rle::decode_bounded(&fr.a, expected)?; + let bnd = rle::decode_bounded(&fr.b, expected)?; + if a.len() == expected && bnd.len() == expected { let af: Vec = a.iter().map(|v| *v as f32 / 255.0).collect(); let (alpha, band) = if fw == w && fh == h { (af, bnd) diff --git a/crates/track/src/roto/mod.rs b/crates/track/src/roto/mod.rs index 4efb4335..305065d2 100644 --- a/crates/track/src/roto/mod.rs +++ b/crates/track/src/roto/mod.rs @@ -224,13 +224,19 @@ pub struct FrameSeg { impl FrameSeg { pub fn empty(w: usize, h: usize) -> FrameSeg { - FrameSeg { w, h, matte: vec![0; w * h], refine: vec![0; w * h], ..Default::default() } + let Some(n) = w.checked_mul(h).filter(|n| *n <= rle::MAX_PLANE_BYTES) else { + return FrameSeg::default(); + }; + FrameSeg { w, h, matte: vec![0; n], refine: vec![0; n], ..Default::default() } } pub fn area(&self) -> usize { self.matte.iter().filter(|v| **v != 0).count() } /// Centroid of the matte (pixels), if any. pub fn centroid(&self) -> Option<[f64; 2]> { + if self.w == 0 || self.w.checked_mul(self.h)? > self.matte.len() { + return None; + } let (mut sx, mut sy, mut n) = (0.0, 0.0, 0.0); for (i, v) in self.matte.iter().enumerate() { if *v != 0 { @@ -246,8 +252,18 @@ impl FrameSeg { /// Worker to the page): `ECSG`, width, height (u32 LE), the colour models as JSON (length /// prefixed), then the matte and the Refine Edge band run-length encoded (u16 LE run, byte). pub fn to_bytes(&self) -> Vec { + let Some(pixels) = self.w.checked_mul(self.h).filter(|n| *n <= rle::MAX_PLANE_BYTES) else { + return Vec::new(); + }; + if self.matte.len() != pixels || self.refine.len() != pixels || self.w > u32::MAX as usize || self.h > u32::MAX as usize { + return Vec::new(); + } let models = serde_json::to_vec(&(&self.fg, &self.bg)).unwrap_or_default(); - let mut out = Vec::with_capacity(16 + models.len() + 64); + let Some(header_len) = 16usize.checked_add(models.len()) else { return Vec::new() }; + let mut out = Vec::new(); + if out.try_reserve(header_len.saturating_add(64)).is_err() { + return Vec::new(); + } out.extend_from_slice(b"ECSG"); out.extend_from_slice(&(self.w as u32).to_le_bytes()); out.extend_from_slice(&(self.h as u32).to_le_bytes()); @@ -261,6 +277,9 @@ impl FrameSeg { while i + n < plane.len() && plane[i + n] == v && n < u16::MAX as usize { n += 1; } + if out.try_reserve(3).is_err() { + return Vec::new(); + } out.extend_from_slice(&(n as u16).to_le_bytes()); out.push(v); i += n; @@ -277,23 +296,40 @@ impl FrameSeg { let u = |i: usize| u32::from_le_bytes([b[i], b[i + 1], b[i + 2], b[i + 3]]) as usize; let (w, h, ml) = (u(4), u(8), u(12)); let n = w.checked_mul(h)?; - let models = b.get(16..16 + ml)?; + if n > rle::MAX_PLANE_BYTES || w == 0 || h == 0 { + return None; + } + let models_end = 16usize.checked_add(ml)?; + let models = b.get(16..models_end)?; let (fg, bg): (Gmm, Gmm) = serde_json::from_slice(models).ok()?; - let mut rest = &b[16 + ml..]; + let mut rest = b.get(models_end..)?; + // Each run covers at most u16::MAX pixels; refuse before reserving a plane the input can't fill. + if rest.len() < n.div_ceil(usize::from(u16::MAX)).saturating_mul(6) { + return None; + } let mut plane = || -> Option> { - let mut v = Vec::with_capacity(n); + let mut v = Vec::new(); + v.try_reserve_exact(n).ok()?; while v.len() < n { if rest.len() < 3 { return None; } - let run = u16::from_le_bytes([rest[0], rest[1]]) as usize; - v.resize(v.len() + run, rest[2]); - rest = &rest[3..]; + let run = u16::from_le_bytes([*rest.first()?, *rest.get(1)?]) as usize; + let next = v.len().checked_add(run)?; + if run == 0 || next > n { + return None; + } + let value = *rest.get(2)?; + v.resize(next, value); + rest = rest.get(3..)?; } (v.len() == n).then_some(v) }; let matte = plane()?; let refine = plane()?; + if !rest.is_empty() { + return None; + } Some(FrameSeg { w, h, matte, refine, fg, bg }) } } diff --git a/crates/track/src/roto/rle.rs b/crates/track/src/roto/rle.rs index 308b2689..914bcad8 100644 --- a/crates/track/src/roto/rle.rs +++ b/crates/track/src/roto/rle.rs @@ -17,6 +17,10 @@ pub fn base64_encode(data: &[u8]) -> String { } pub fn base64_decode(s: &str) -> Option> { + base64_decode_bounded(s, usize::MAX) +} + +fn base64_decode_bounded(s: &str, max_bytes: usize) -> Option> { let val = |c: u8| -> Option { Some(match c { b'A'..=b'Z' => (c - b'A') as u32, @@ -27,13 +31,25 @@ pub fn base64_decode(s: &str) -> Option> { _ => return None, }) }; - let bytes: Vec = s.bytes().filter(|c| !c.is_ascii_whitespace()).collect(); - if !bytes.len().is_multiple_of(4) { + let significant = s.bytes().filter(|c| !c.is_ascii_whitespace()).count(); + let max_encoded = max_bytes.checked_add(2).and_then(|n| n.checked_div(3)).and_then(|n| n.checked_mul(4)).unwrap_or(usize::MAX); + if !significant.is_multiple_of(4) || significant > max_encoded { return None; } - let mut out = Vec::with_capacity(bytes.len() / 4 * 3); + let mut bytes = Vec::new(); + bytes.try_reserve_exact(significant).ok()?; + bytes.extend(s.bytes().filter(|c| !c.is_ascii_whitespace())); + let decoded_len = bytes.len().checked_div(4)?.checked_mul(3)?; + if decoded_len > max_bytes.saturating_add(2) { + return None; + } + let mut out = Vec::new(); + out.try_reserve_exact(decoded_len.min(max_bytes)).ok()?; for c in bytes.chunks(4) { let pad = c.iter().rev().take_while(|b| **b == b'=').count(); + if pad > 2 || c[..4 - pad].contains(&b'=') { + return None; + } let mut n = 0u32; for (i, b) in c.iter().enumerate() { n |= if *b == b'=' { 0 } else { val(*b)? } << (18 - 6 * i); @@ -96,36 +112,66 @@ pub fn encode(values: &[u8]) -> String { base64_encode(&out) } -/// Decode [`encode`]'s output (None when malformed). -pub fn decode(s: &str) -> Option> { - let data = base64_decode(s)?; - let mut pos = 0; - let n = get(&data, &mut pos)? as usize; - if n > 1 << 30 { +/// Decode [`encode`]'s output (None when malformed or larger than the caller's validated plane). +pub fn decode_bounded(s: &str, max_len: usize) -> Option> { + let encoded_limit = max_len.saturating_mul(16).saturating_add(128); + if s.len() > encoded_limit { return None; } - let mut out = Vec::with_capacity(n); + // An alternating byte plane takes at most one value byte and a ten-byte varint per + // pixel, plus the declared-length varint. + let decoded_limit = max_len.saturating_mul(11).saturating_add(10); + let data = base64_decode_bounded(s, decoded_limit)?; + let mut pos = 0; + let n = usize::try_from(get(&data, &mut pos)?).ok()?; + if n > max_len { + return None; + } + let mut out = Vec::new(); + out.try_reserve_exact(n).ok()?; while out.len() < n { let v = *data.get(pos)?; pos += 1; - let run = get(&data, &mut pos)? as usize; - if out.len() + run > n { + let run = usize::try_from(get(&data, &mut pos)?).ok()?; + let next = out.len().checked_add(run)?; + if run == 0 || next > n { return None; } - out.resize(out.len() + run, v); + out.resize(next, v); } Some(out) } +/// Decode [`encode`]'s output, capped at the standard matte-plane limit. +pub fn decode(s: &str) -> Option> { + decode_bounded(s, MAX_PLANE_BYTES) +} + +/// Maximum frozen matte plane size (256 MiB at one byte per pixel). +pub const MAX_PLANE_BYTES: usize = 256 * 1024 * 1024; + #[cfg(test)] mod tests { use super::*; + #[test] + fn refuses_oversized_and_zero_runs_before_expansion() { + let mut declared = Vec::new(); + put(&mut declared, 1024); + declared.extend_from_slice(&[255, 0x80, 0x08]); // 1024-byte run + assert!(decode_bounded(&base64_encode(&declared), 16).is_none()); + let mut zero_run = Vec::new(); + put(&mut zero_run, 1); + zero_run.extend_from_slice(&[0, 0]); + assert!(decode_bounded(&base64_encode(&zero_run), 16).is_none()); + assert!(decode_bounded(&"A".repeat(100_000), 16).is_none()); + } + #[test] fn round_trips() { for n in [0usize, 1, 2, 3, 4, 5, 100, 1000] { let v: Vec = (0..n).map(|i| ((i / 7) % 3) as u8 * 100).collect(); - assert_eq!(decode(&encode(&v)).unwrap(), v); + assert_eq!(decode(&encode(&v)).unwrap(), v, "RLE n={n}"); let raw: Vec = (0..n).map(|i| (i * 31) as u8).collect(); assert_eq!(base64_decode(&base64_encode(&raw)).unwrap(), raw); } diff --git a/crates/track/src/roto/tests.rs b/crates/track/src/roto/tests.rs index 65d1743f..3c4645cb 100644 --- a/crates/track/src/roto/tests.rs +++ b/crates/track/src/roto/tests.rs @@ -95,6 +95,21 @@ fn strokes_segment_the_base_frame() { assert!(bytes.len() < seg.w * seg.h / 4, "{} bytes", bytes.len()); assert!(FrameSeg::from_bytes(&bytes[..bytes.len() - 1]).is_none()); assert!(FrameSeg::from_bytes(b"nope").is_none()); + + // Untrusted plane dimensions and run lengths are rejected before allocation/resize. + let mut hostile = b"ECSG".to_vec(); + hostile.extend_from_slice(&u32::MAX.to_le_bytes()); + hostile.extend_from_slice(&u32::MAX.to_le_bytes()); + hostile.extend_from_slice(&0u32.to_le_bytes()); + assert!(FrameSeg::from_bytes(&hostile).is_none()); + let mut overrun = bytes.clone(); + let plane_at = 16 + u32::from_le_bytes(bytes[12..16].try_into().unwrap()) as usize; + overrun[plane_at..plane_at + 2].copy_from_slice(&u16::MAX.to_le_bytes()); + assert!(FrameSeg::from_bytes(&overrun).is_none()); + let mut trailing = bytes.clone(); + trailing.push(0); + assert!(FrameSeg::from_bytes(&trailing).is_none()); + assert_eq!(FrameSeg::empty(usize::MAX, 2), FrameSeg::default()); } #[test]