You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The function sgrproj_box_ab_internal looks like this:
pub(crate) fn sgrproj_box_ab_internal<const BD: usize>(
r: usize, af: &mut [u32], bf: &mut [u32], iimg: &[u32], iimg_sq: &[u32],
iimg_stride: usize, start_x: usize, y: usize, stripe_w: usize, s: u32,
) {
let d: usize = r * 2 + 1;
let n: usize = d * d;
let one_over_n = if r == 1 { 455 } else { 164 };
assert!(iimg.len() > (y + d) * iimg_stride + stripe_w + d);
assert!(iimg_sq.len() > (y + d) * iimg_stride + stripe_w + d);
assert!(af.len() > stripe_w);
assert!(bf.len() > stripe_w);
for x in start_x..stripe_w + 2 {
// SAFETY: We perform the bounds checks above, once for the whole loop
unsafe {
let sum = get_integral_square(iimg, iimg_stride, x, y, d);
let ssq = get_integral_square(iimg_sq, iimg_stride, x, y, d);
let (reta, retb) =
sgrproj_sum_finish::<BD>(ssq, sum, n as u32, one_over_n, s);
*af.get_unchecked_mut(x) = reta;
*bf.get_unchecked_mut(x) = retb;
}
}
}
The safety comment claims that the bounds are checked above. The safety condition required by af.get_unchecked_mut(x) is x < af.len(). The maximum value x can have is stripe_w + 2 (exclusive) or stripe_w + 1 (inclusive).
However the assertion only checked that stripe_w < bf.len(), not stripe_w + 1 < bf.len(), so this function doesn't actually ensure its safety preconditions, and is thus unsound.
The same issue applies to all 4 of these assertions. Luckily the stricter assertions do not fail in the unit test suite, so it looks like a theoretical soundness issue, not a practical problem.
An additional complication are integer overflows. If the calculation inside the assertion overflows, the assertion can pass, while the index can still be out of bounds for a smaller x. This currently affects only the first two assertions, but would also affect a "fixed" assertion like assert!(af.len() > stripe_w + 1).
The text was updated successfully, but these errors were encountered:
The function
sgrproj_box_ab_internal
looks like this:The safety comment claims that the bounds are checked above. The safety condition required by
af.get_unchecked_mut(x)
isx < af.len()
. The maximum valuex
can have isstripe_w + 2
(exclusive) orstripe_w + 1
(inclusive).However the assertion only checked that
stripe_w < bf.len()
, notstripe_w + 1 < bf.len()
, so this function doesn't actually ensure its safety preconditions, and is thus unsound.The same issue applies to all 4 of these assertions. Luckily the stricter assertions do not fail in the unit test suite, so it looks like a theoretical soundness issue, not a practical problem.
An additional complication are integer overflows. If the calculation inside the assertion overflows, the assertion can pass, while the index can still be out of bounds for a smaller
x
. This currently affects only the first two assertions, but would also affect a "fixed" assertion likeassert!(af.len() > stripe_w + 1)
.The text was updated successfully, but these errors were encountered: