Skip to content

Commit

Permalink
Fix Uint::shr(_vartime) and Uint::shl(_vartime) (#550)
Browse files Browse the repository at this point in the history
The amount to shift was being miscomputed. It should've been a
saturating-like operation, where any shift larger than the precision
returns zero, but instead a modulus (which isn't constant time) was
being applied instead.

This also adds proptests to ensure correct behavior.
  • Loading branch information
tarcieri authored Jan 10, 2024
1 parent 0083e34 commit 5909271
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 23 deletions.
14 changes: 6 additions & 8 deletions src/uint/shl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,16 @@ impl<const LIMBS: usize> Uint<LIMBS> {
}
}

/// Computes `self << shift` in a panic-free manner, masking off bits of `shift` which would cause the shift to
/// exceed the type's width.
/// Computes `self << shift` in a panic-free manner, returning zero if the shift exceeds the
/// precision.
pub const fn wrapping_shl(&self, shift: u32) -> Self {
self.overflowing_shl(shift % Self::BITS)
.expect("shift within range")
self.overflowing_shl(shift).unwrap_or(Self::ZERO)
}

/// Computes `self << shift` in variable-time in a panic-free manner, masking off bits of `shift` which would cause
/// the shift to exceed the type's width.
/// Computes `self << shift` in variable-time in a panic-free manner, returning zero if the
/// shift exceeds the precision.
pub const fn wrapping_shl_vartime(&self, shift: u32) -> Self {
self.overflowing_shl_vartime(shift % Self::BITS)
.expect("shift within range")
self.overflowing_shl_vartime(shift).unwrap_or(Self::ZERO)
}

/// Computes `self << shift` where `0 <= shift < Limb::BITS`,
Expand Down
14 changes: 6 additions & 8 deletions src/uint/shr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,16 @@ impl<const LIMBS: usize> Uint<LIMBS> {
}
}

/// Computes `self >> shift` in a panic-free manner, masking off bits of `shift` which would cause the shift to
/// exceed the type's width.
/// Computes `self >> shift` in a panic-free manner, returning zero if the shift exceeds the
/// precision.
pub const fn wrapping_shr(&self, shift: u32) -> Self {
self.overflowing_shr(shift % Self::BITS)
.expect("shift within range")
self.overflowing_shr(shift).unwrap_or(Self::ZERO)
}

/// Computes `self >> shift` in variable-time in a panic-free manner, masking off bits of `shift` which would cause
/// the shift to exceed the type's width.
/// Computes `self >> shift` in variable-time in a panic-free manner, returning zero if the
/// shift exceeds the precision.
pub const fn wrapping_shr_vartime(&self, shift: u32) -> Self {
self.overflowing_shr_vartime(shift % Self::BITS)
.expect("shift within range")
self.overflowing_shr_vartime(shift).unwrap_or(Self::ZERO)
}

/// Computes `self >> 1` in constant-time.
Expand Down
7 changes: 0 additions & 7 deletions tests/uint.proptest-regressions

This file was deleted.

29 changes: 29 additions & 0 deletions tests/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,35 @@ proptest! {
assert_eq!(expected, actual);
}

#[test]
fn wrapping_shl(n in uint(), shift in any::<u32>()) {
let n_bi = to_biguint(&n);

let expected = if shift < U256::BITS {
to_uint(n_bi << shift)
} else {
U256::ZERO
};

let actual_ct = n.wrapping_shl(shift);
assert_eq!(expected, actual_ct);

let actual_vartime = n.wrapping_shl_vartime(shift);
assert_eq!(expected, actual_vartime);
}

#[test]
fn wrapping_shr(n in uint(), shift in any::<u32>()) {
let n_bi = to_biguint(&n);
let expected = to_uint(n_bi >> shift);

let actual_ct = n.wrapping_shr(shift);
assert_eq!(expected, actual_ct);

let actual_vartime = n.wrapping_shr_vartime(shift);
assert_eq!(expected, actual_vartime);
}

#[test]
fn encoding(a in uint()) {
assert_eq!(a, U256::from_be_bytes(a.to_be_bytes()));
Expand Down

0 comments on commit 5909271

Please sign in to comment.