From 47eba81232cb9bf9289ce632a36d013cd41bc3f6 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Mon, 8 Apr 2024 11:29:01 +1000 Subject: [PATCH 1/5] Fix potential overflow in deserialization of Bitmap --- src/bitmap/serialization.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/bitmap/serialization.rs b/src/bitmap/serialization.rs index f60bbad1..2ea63560 100644 --- a/src/bitmap/serialization.rs +++ b/src/bitmap/serialization.rs @@ -226,9 +226,11 @@ impl RoaringBitmap { let cardinality = intervals.iter().map(|[_, len]| *len as usize).sum(); let mut store = Store::with_capacity(cardinality); - intervals.into_iter().for_each(|[s, len]| { - store.insert_range(RangeInclusive::new(s, s + len)); - }); + intervals.into_iter().try_for_each(|[s, len]| -> Result<(), io::ErrorKind> { + let end = s.checked_add(len).ok_or(io::ErrorKind::InvalidData)?; + store.insert_range(RangeInclusive::new(s, end)); + Ok(()) + })?; store } else if cardinality <= ARRAY_LIMIT { let mut values = vec![0; cardinality as usize]; @@ -267,4 +269,11 @@ mod test { prop_assert_eq!(bitmap, RoaringBitmap::deserialize_from(buffer.as_slice()).unwrap()); } } + + #[test] + fn test_deserialize_overflow_s_plus_len() { + let data = vec![59, 48, 0, 0, 255, 130, 254, 59, 48, 2, 0, 41, 255, 255, 166, 197, 4, 0, 2]; + let res = RoaringBitmap::deserialize_from(data.as_slice()); + assert!(res.is_err()); + } } From e9e066ae3cda09f11dbd5c021746921b3525d731 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Fri, 12 Apr 2024 12:32:28 +1000 Subject: [PATCH 2/5] Fix compiler warnings on beta --- src/bitmap/container.rs | 2 -- src/bitmap/fmt.rs | 2 -- src/bitmap/inherent.rs | 2 -- src/bitmap/iter.rs | 4 ++-- src/bitmap/mod.rs | 2 -- src/bitmap/multiops.rs | 2 +- src/bitmap/ops.rs | 2 -- src/bitmap/serialization.rs | 2 +- src/bitmap/store/array_store/mod.rs | 4 ---- src/bitmap/store/bitmap_store.rs | 3 --- src/bitmap/store/mod.rs | 2 +- src/treemap/fmt.rs | 2 -- src/treemap/inherent.rs | 1 - src/treemap/iter.rs | 2 +- src/treemap/multiops.rs | 5 +---- src/treemap/ops.rs | 1 - 16 files changed, 7 insertions(+), 31 deletions(-) diff --git a/src/bitmap/container.rs b/src/bitmap/container.rs index 9116863a..552fff56 100644 --- a/src/bitmap/container.rs +++ b/src/bitmap/container.rs @@ -3,8 +3,6 @@ use core::ops::{ BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, RangeInclusive, Sub, SubAssign, }; -use alloc::vec::Vec; - use super::store::{self, Store}; use super::util; diff --git a/src/bitmap/fmt.rs b/src/bitmap/fmt.rs index a3a6a95a..096506d0 100644 --- a/src/bitmap/fmt.rs +++ b/src/bitmap/fmt.rs @@ -1,7 +1,5 @@ use core::fmt; -use alloc::vec::Vec; - use crate::RoaringBitmap; impl fmt::Debug for RoaringBitmap { diff --git a/src/bitmap/inherent.rs b/src/bitmap/inherent.rs index abb5acbb..caa26506 100644 --- a/src/bitmap/inherent.rs +++ b/src/bitmap/inherent.rs @@ -1,8 +1,6 @@ use core::cmp::Ordering; use core::ops::RangeBounds; -use alloc::vec::Vec; - use crate::RoaringBitmap; use super::container::Container; diff --git a/src/bitmap/iter.rs b/src/bitmap/iter.rs index 541c49ed..ac85fa33 100644 --- a/src/bitmap/iter.rs +++ b/src/bitmap/iter.rs @@ -1,5 +1,5 @@ -use alloc::vec::{self, Vec}; -use core::iter::{self, FromIterator}; +use alloc::vec; +use core::iter; use core::slice; use super::container::Container; diff --git a/src/bitmap/mod.rs b/src/bitmap/mod.rs index b3bbd080..0fce56e2 100644 --- a/src/bitmap/mod.rs +++ b/src/bitmap/mod.rs @@ -17,8 +17,6 @@ mod serde; #[cfg(feature = "std")] mod serialization; -use alloc::vec::Vec; - use self::cmp::Pairs; pub use self::iter::IntoIter; pub use self::iter::Iter; diff --git a/src/bitmap/multiops.rs b/src/bitmap/multiops.rs index fd8f5282..e3d37b51 100644 --- a/src/bitmap/multiops.rs +++ b/src/bitmap/multiops.rs @@ -5,7 +5,7 @@ use core::{ ops::{BitOrAssign, BitXorAssign}, }; -use alloc::{borrow::Cow, vec::Vec}; +use alloc::borrow::Cow; use crate::{MultiOps, RoaringBitmap}; diff --git a/src/bitmap/ops.rs b/src/bitmap/ops.rs index bfdd0ce3..75a2fa6a 100644 --- a/src/bitmap/ops.rs +++ b/src/bitmap/ops.rs @@ -1,8 +1,6 @@ use core::mem; use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Sub, SubAssign}; -use alloc::vec::Vec; - use crate::bitmap::container::Container; use crate::bitmap::Pairs; use crate::RoaringBitmap; diff --git a/src/bitmap/serialization.rs b/src/bitmap/serialization.rs index 2ea63560..790e2d28 100644 --- a/src/bitmap/serialization.rs +++ b/src/bitmap/serialization.rs @@ -1,6 +1,6 @@ use bytemuck::cast_slice_mut; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; -use core::convert::{Infallible, TryFrom}; +use core::convert::Infallible; use core::ops::RangeInclusive; use std::error::Error; use std::io; diff --git a/src/bitmap/store/array_store/mod.rs b/src/bitmap/store/array_store/mod.rs index b08bc9b6..54ae7d28 100644 --- a/src/bitmap/store/array_store/mod.rs +++ b/src/bitmap/store/array_store/mod.rs @@ -2,13 +2,9 @@ mod scalar; mod vector; mod visitor; -use alloc::boxed::Box; -use alloc::vec::Vec; - use crate::bitmap::store::array_store::visitor::{CardinalityCounter, VecWriter}; use core::cmp::Ordering; use core::cmp::Ordering::*; -use core::convert::TryFrom; use core::fmt::{Display, Formatter}; use core::ops::{BitAnd, BitAndAssign, BitOr, BitXor, RangeInclusive, Sub, SubAssign}; diff --git a/src/bitmap/store/bitmap_store.rs b/src/bitmap/store/bitmap_store.rs index 6a8c2b37..ca21e1f9 100644 --- a/src/bitmap/store/bitmap_store.rs +++ b/src/bitmap/store/bitmap_store.rs @@ -3,9 +3,6 @@ use core::cmp::Ordering; use core::fmt::{Display, Formatter}; use core::ops::{BitAndAssign, BitOrAssign, BitXorAssign, RangeInclusive, SubAssign}; -use alloc::boxed::Box; -use alloc::vec::Vec; - use super::ArrayStore; pub const BITMAP_LENGTH: usize = 1024; diff --git a/src/bitmap/store/mod.rs b/src/bitmap/store/mod.rs index c80b2078..4cba870b 100644 --- a/src/bitmap/store/mod.rs +++ b/src/bitmap/store/mod.rs @@ -1,7 +1,7 @@ mod array_store; mod bitmap_store; -use alloc::{boxed::Box, vec}; +use alloc::vec; use core::mem; use core::ops::{ BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, RangeInclusive, Sub, SubAssign, diff --git a/src/treemap/fmt.rs b/src/treemap/fmt.rs index 86840116..3c36b72b 100644 --- a/src/treemap/fmt.rs +++ b/src/treemap/fmt.rs @@ -1,7 +1,5 @@ use core::fmt; -use alloc::vec::Vec; - use crate::RoaringTreemap; impl fmt::Debug for RoaringTreemap { diff --git a/src/treemap/inherent.rs b/src/treemap/inherent.rs index 98ef7831..e1190931 100644 --- a/src/treemap/inherent.rs +++ b/src/treemap/inherent.rs @@ -1,5 +1,4 @@ use alloc::collections::btree_map::{BTreeMap, Entry}; -use alloc::vec::Vec; use core::iter; use core::ops::RangeBounds; diff --git a/src/treemap/iter.rs b/src/treemap/iter.rs index a5239bc5..afe1fb42 100644 --- a/src/treemap/iter.rs +++ b/src/treemap/iter.rs @@ -1,5 +1,5 @@ use alloc::collections::{btree_map, BTreeMap}; -use core::iter::{self, FromIterator}; +use core::iter; use super::util; use crate::bitmap::IntoIter as IntoIter32; diff --git a/src/treemap/multiops.rs b/src/treemap/multiops.rs index f43bdf71..c50b8530 100644 --- a/src/treemap/multiops.rs +++ b/src/treemap/multiops.rs @@ -1,7 +1,4 @@ -use alloc::{ - collections::{binary_heap::PeekMut, BTreeMap, BinaryHeap}, - vec::Vec, -}; +use alloc::collections::{binary_heap::PeekMut, BTreeMap, BinaryHeap}; use core::{borrow::Borrow, cmp::Ordering, mem}; use crate::{MultiOps, RoaringBitmap, RoaringTreemap}; diff --git a/src/treemap/ops.rs b/src/treemap/ops.rs index 17e1b4a8..f1a25a2f 100644 --- a/src/treemap/ops.rs +++ b/src/treemap/ops.rs @@ -1,5 +1,4 @@ use alloc::collections::btree_map::Entry; -use alloc::vec::Vec; use core::mem; use core::ops::{BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Sub, SubAssign}; From 0454dbb5f58578eb991a40e7d52d702d4a9b8c60 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Fri, 12 Apr 2024 12:34:22 +1000 Subject: [PATCH 3/5] Remove one more unused import --- src/bitmap/store/array_store/visitor.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bitmap/store/array_store/visitor.rs b/src/bitmap/store/array_store/visitor.rs index 56a1a46b..b092e6cf 100644 --- a/src/bitmap/store/array_store/visitor.rs +++ b/src/bitmap/store/array_store/visitor.rs @@ -1,5 +1,3 @@ -use alloc::vec::Vec; - #[cfg(feature = "simd")] use crate::bitmap::store::array_store::vector::swizzle_to_front; From 49f0e9d144cd68610ae80c44a24dd55edc2bd553 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Tue, 16 Apr 2024 08:54:29 +1000 Subject: [PATCH 4/5] Resolve clippy lints --- src/bitmap/arbitrary.rs | 2 -- tests/iter.rs | 1 - tests/push.rs | 1 - tests/treemap_iter.rs | 1 - tests/treemap_serialization.rs | 1 - 5 files changed, 6 deletions(-) diff --git a/src/bitmap/arbitrary.rs b/src/bitmap/arbitrary.rs index de4c4d27..45d47540 100644 --- a/src/bitmap/arbitrary.rs +++ b/src/bitmap/arbitrary.rs @@ -3,8 +3,6 @@ mod test { use crate::bitmap::container::Container; use crate::bitmap::store::{ArrayStore, BitmapStore, Store}; use crate::RoaringBitmap; - use alloc::boxed::Box; - use alloc::vec::Vec; use core::fmt::{Debug, Formatter}; use proptest::bits::{BitSetLike, BitSetStrategy, SampledBitSetStrategy}; use proptest::collection::{vec, SizeRange}; diff --git a/tests/iter.rs b/tests/iter.rs index bbffedc9..7d4e5809 100644 --- a/tests/iter.rs +++ b/tests/iter.rs @@ -1,4 +1,3 @@ -use core::iter::FromIterator; use proptest::arbitrary::any; use proptest::collection::btree_set; use proptest::proptest; diff --git a/tests/push.rs b/tests/push.rs index 0919d39a..aac60da8 100644 --- a/tests/push.rs +++ b/tests/push.rs @@ -1,5 +1,4 @@ extern crate roaring; -use core::iter::FromIterator; use roaring::{RoaringBitmap, RoaringTreemap}; /// macro created to reduce code duplication diff --git a/tests/treemap_iter.rs b/tests/treemap_iter.rs index bdb53636..ef328a58 100644 --- a/tests/treemap_iter.rs +++ b/tests/treemap_iter.rs @@ -2,7 +2,6 @@ extern crate roaring; mod iter; use roaring::RoaringTreemap; -use core::iter::FromIterator; use iter::outside_in; use proptest::arbitrary::any; use proptest::collection::btree_set; diff --git a/tests/treemap_serialization.rs b/tests/treemap_serialization.rs index 4c8f6be9..2a15ed63 100644 --- a/tests/treemap_serialization.rs +++ b/tests/treemap_serialization.rs @@ -1,6 +1,5 @@ #![cfg(feature = "std")] -use core::iter::FromIterator; use roaring::RoaringTreemap; fn serialize_deserialize(dataset: Dataset) From bf0cb5a4de146faf9ea6914fcbddd83bc5234b6a Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Tue, 16 Apr 2024 08:55:48 +1000 Subject: [PATCH 5/5] Resolve nightly clippy lints --- tests/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/lib.rs b/tests/lib.rs index 5fcf4a2f..5edbcc19 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -13,11 +13,11 @@ fn smoke() { assert!(bitmap.contains(1)); assert_eq!(bitmap.len(), 1); assert!(!bitmap.is_empty()); - bitmap.insert(u32::max_value() - 2); - assert!(bitmap.contains(u32::max_value() - 2)); + bitmap.insert(u32::MAX - 2); + assert!(bitmap.contains(u32::MAX - 2)); assert_eq!(bitmap.len(), 2); - bitmap.insert(u32::max_value()); - assert!(bitmap.contains(u32::max_value())); + bitmap.insert(u32::MAX); + assert!(bitmap.contains(u32::MAX)); assert_eq!(bitmap.len(), 3); bitmap.insert(2); assert!(bitmap.contains(2)); @@ -28,9 +28,9 @@ fn smoke() { assert!(!bitmap.contains(0)); assert!(bitmap.contains(1)); assert!(!bitmap.contains(100)); - assert!(bitmap.contains(u32::max_value() - 2)); - assert!(!bitmap.contains(u32::max_value() - 1)); - assert!(bitmap.contains(u32::max_value())); + assert!(bitmap.contains(u32::MAX - 2)); + assert!(!bitmap.contains(u32::MAX - 1)); + assert!(bitmap.contains(u32::MAX)); } #[test]