project: a hostile ROI cannot overflow the crop math

The roi quad is stored raw from the control channel (crop.useRoi passes
serde_json's [f64;4] straight through, unlike the sibling edge fields,
which are clamped to plus/minus 20 000) and nothing repairs it on load.
Crop::edges converted it with round() as i32 and subtracted raw: an x of
-1e12 saturates to i32::MIN and w as i32 - l overflows in debug builds,
wrapping to a wrong-but-bounded crop in release.

Clamp each converted edge to the same plus/minus 20 000 the sibling
fields use and subtract with saturating_sub in edges() and frame_size().
Regression test: hostile_roi_cannot_overflow_crop_math — panics at
crates/project/src/render_queue.rs:774 on main; with the fix the hostile
quad degrades to a 1x1 frame and the sane-ROI behavior is unchanged.
This commit is contained in:
midasmatterhorn
2026-10-15 16:18:10 +03:00
parent 0311bdc34d
commit 643dcf55c6
+26 -6
View File
@@ -769,10 +769,14 @@ impl Crop {
if self.use_roi
&& let Some([x, y, rw, rh]) = self.roi
{
let l = (x * scale).round() as i32;
let t = (y * scale).round() as i32;
let r = w as i32 - ((x + rw) * scale).round() as i32;
let b = h as i32 - ((y + rh) * scale).round() as i32;
// The quad arrives unvalidated (control channel or a hand-edited project):
// clamp like the sibling edge fields (plus/minus 20 000) and subtract
// without overflow.
let conv = |v: f64| v.round().clamp(-20_000.0, 20_000.0) as i32;
let l = conv(x * scale);
let t = conv(y * scale);
let r = (w as i32).saturating_sub(conv((x + rw) * scale));
let b = (h as i32).saturating_sub(conv((y + rh) * scale));
return (t, l, b, r);
}
(self.top, self.left, self.bottom, self.right)
@@ -1094,8 +1098,8 @@ impl OutputModule {
/// scale `scale`: after Crop and Resize (before codec rounding).
pub fn frame_size(&self, w: u32, h: u32, scale: f64) -> (u32, u32) {
let (t, l, b, r) = self.crop.edges(w, h, scale);
let cw = (w as i32 - l - r).max(1) as u32;
let ch = (h as i32 - t - b).max(1) as u32;
let cw = (w as i32).saturating_sub(l).saturating_sub(r).max(1) as u32;
let ch = (h as i32).saturating_sub(t).saturating_sub(b).max(1) as u32;
if !self.resize.enabled {
return (cw, ch);
}
@@ -1643,6 +1647,22 @@ mod tests {
assert!(s.frame_time(&c, 0) >= Tick::from_seconds_f64(1.0));
}
#[test]
fn hostile_roi_cannot_overflow_crop_math() {
// `-1e12` saturates to `i32::MIN` on conversion: `w as i32 - l` must saturate,
// not overflow, and still yield a usable (degenerate) frame size.
let c = Comp::new(1920, 1080, FrameRate::new(30, 1), Tick::from_seconds_f64(1.0));
let rs = RenderSettings::default();
let mut m = OutputModule::for_format(OutputFormat::PngSequence);
m.crop = Crop { enabled: true, use_roi: true, roi: Some([-1e12, -1e12, 200.0, 200.0]), ..Default::default() };
let (w, h) = m.output_size(&c, &rs);
assert!((w, h) == (1, 1), "degenerate but valid: {w}x{h}");
// Sane ROIs are unaffected (existing behavior).
m.crop = Crop { enabled: true, use_roi: true, roi: Some([100.0, 50.0, 800.0, 400.0]), ..Default::default() };
let half = RenderSettings { resolution: 0.5, ..Default::default() };
assert_eq!(m.output_size(&c, &half), (400, 200));
}
#[test]
fn uncompressed_quicktime_format() {
let f = OutputFormat::from_name("uncompressed").unwrap();