-
Notifications
You must be signed in to change notification settings - Fork 35
[M-1] Bitmask shift can panic on 32-bit targets #172
Copy link
Copy link
Closed
Description
Context: crates/backend/fiat-shamir/src/challenger.rs
Description
The sample_in_range() computes ((1 << bits) - 1) using usize. While it asserts bits < F::bits(), that does not guarantee bits < usize::BITS. On 32-bit targets, bits may be between 32 and 63 since PrimeField64 commonly has F::bits() == 64, causing 1 << bits to overflow/panic.
If bits is influenced by untrusted inputs or configuration, this becomes a denial-of-service vector via panic.
pub fn sample_in_range(&mut self, bits: usize, n_samples: usize) -> Vec<usize> {
assert!(bits < F::bits());
let sampled_fe = self.sample_many(n_samples.div_ceil(RATE)).into_iter().flatten();
let mut res = Vec::new();
for fe in sampled_fe.take(n_samples) {
let rand_usize = fe.as_canonical_u64() as usize;
res.push(rand_usize & ((1 << bits) - 1));
}
res
}Recommendation
Constrain bits to bits < usize::BITS as usize and use checked shifting: let mask = (1usize.checked_shl(bits as u32).ok_or(...)?.wrapping_sub(1)),
Or compute the mask in u64 and downcast only after validation. Avoid assert! for input-dependent conditions in library code, return an error instead.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels