Skip to content

calc_optical_flow_pyramid_lk: win_size area overflows i32 and panics for huge windows #155

Description

@kalwalt

Fact

TrackingWindow::area (src/video/optical_flow.rs) multiplies the two i32 window sides unchecked:

fn area(self) -> usize {
    (self.width * self.height) as usize
}

calc_optical_flow_pyramid_lk validates only the lower bound (win_size.width < 3 || win_size.height < 3 → InvalidInput), so any accepted size with width × height > i32::MAX (e.g. 1_073_741_824 × 3, or 46341 × 46341) reaches it. Call sites on dev (ef13ed8):

  • lk_single_level: let win_area = win.area() as f64; runs on every path, scalar and SIMD.
  • SIMD path: let n_win = win.area(); → Vec::with_capacity(n_win) ×3.
  • compute_tracking_error: error / win.area() as f32.

Debug builds: attempt to multiply with overflow panic. Release builds: the product wraps; a negative result cast to usize becomes enormous, so the SIMD path's Vec::with_capacity panics with a capacity overflow, and the scalar path divides by a garbage area.

This predates #144; the old (2*half_w + 1) * (2*half_h + 1) had the same unchecked multiply.

Impact

Low in practice, since it needs absurd window sizes. But it's a panic reachable from a public API with a plain Size2i argument, and CLAUDE.md requires library code to return Result<T, PureCvError> rather than panic.

Suggested fix

Validate the product up front, next to the existing minimum-size check, and return InvalidInput if it isn't representable:

if win_size.width.checked_mul(win_size.height).is_none() { /* InvalidInput */ }

Then TrackingWindow::area can rely on it (or compute in usize from the already-validated sides). Regression test: calc_optical_flow_pyramid_lk with Size2i::new(1 << 30, 3) returns Err(InvalidInput) instead of panicking. The test must not allocate, so it should hit the validation before any sampling.

Optionally, consider whether windows larger than the image should be rejected or documented. OpenCV doesn't reject them, so this is a design choice, not a parity fix.

Found in Qodo's review of the v0.9.0 release PR (#153, finding 1).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions