From 6a8b2de7a86cc8d6efc857380fb940e153a5a151 Mon Sep 17 00:00:00 2001 From: c r Date: Tue, 18 Jan 2022 11:10:33 -0600 Subject: [PATCH] B4 fix: fixing by adding max index computation in bitfield validation (#1344) --- utils/bitfield/src/lib.rs | 9 ++++++++ utils/bitfield/src/rleplus/mod.rs | 38 +++++++++++++++++++++++++++++++ vm/actor/src/builtin/miner/mod.rs | 14 +++++++----- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/utils/bitfield/src/lib.rs b/utils/bitfield/src/lib.rs index 36cb625bb03..e0074d81c02 100644 --- a/utils/bitfield/src/lib.rs +++ b/utils/bitfield/src/lib.rs @@ -127,6 +127,15 @@ impl BitField { .find(|i| !self.unset.contains(i)) } + /// Returns the index of the highest bit present in the bit field. + /// Errors if no bits are set. Merges set/unset into ranges, so be cautious with use if set is pretty populated + pub fn last(&self) -> Result { + self.ranges() + .last() + .map(|r| r.end - 1) + .ok_or("no last bit set") + } + /// Returns an iterator over the indices of the bit field's set bits. pub fn iter(&self) -> impl Iterator + '_ { // this code results in the same values as `self.ranges().flatten()`, but there's diff --git a/utils/bitfield/src/rleplus/mod.rs b/utils/bitfield/src/rleplus/mod.rs index 1b77735197d..0e67957b88e 100644 --- a/utils/bitfield/src/rleplus/mod.rs +++ b/utils/bitfield/src/rleplus/mod.rs @@ -382,4 +382,42 @@ mod tests { assert_eq!(bf.ranges().collect::>(), ranges); } } + + // auditor says this: + // I used the ntest package to add a 60 sec timeout + // to the test, as it would otherwise not complete. + // may consider doing this eventually + //#[timeout(60000)] + #[test] + fn iter_last() { + // Create RLE with 2**64-2 set bits- tests timeout on the `let max` line with last + let rle: Vec = vec![ + 0xE4, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x2F, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0x7F, + ]; + let max = BitField::from_bytes(&rle).unwrap().last().unwrap(); + assert_eq!(max, 18446744073709551614); + } + + #[test] + fn test_unset_last() { + // Create a bitfield first 3 set bits + let ranges: Vec = vec![0, 1, 2, 3]; + let iter = ranges_from_bits(ranges.clone()); + let mut bf = BitField::from_ranges(iter); + // Unset bit at pos 3 + bf.unset(3); + + let last = bf.last().unwrap(); + assert_eq!(2, last); + } + + #[test] + fn test_zero_last() { + let mut bf = BitField::new(); + bf.set(0); + + let last = bf.last().unwrap(); + assert_eq!(0, last); + } } diff --git a/vm/actor/src/builtin/miner/mod.rs b/vm/actor/src/builtin/miner/mod.rs index d220a670d49..9f35d19a607 100644 --- a/vm/actor/src/builtin/miner/mod.rs +++ b/vm/actor/src/builtin/miner/mod.rs @@ -2508,14 +2508,16 @@ impl Actor { .validate() .map_err(|e| actor_error!(ErrIllegalArgument, "invalid mask bitfield: {}", e))?; - let last_sector_number = mask_sector_numbers - .iter() - .last() - .ok_or_else(|| actor_error!(ErrIllegalArgument, "invalid mask bitfield"))? - as SectorNumber; + let last_sector_number = mask_sector_numbers.last().map_err(|e| { + actor_error!( + ErrIllegalArgument, + "invalid mask bitfield, no sectors set: {}", + e + ) + })?; #[allow(clippy::absurd_extreme_comparisons)] - if last_sector_number > MAX_SECTOR_NUMBER { + if (last_sector_number as u64) > MAX_SECTOR_NUMBER { return Err(actor_error!( ErrIllegalArgument, "masked sector number {} exceeded max sector number",