Merge pull request #271 from kirk-baird/bitmap-deser-overflow

Fix potential overflow in deserialization of Bitmap

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};

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;
 

src/bitmap/fmt.rs

@@ -1,7 +1,5 @@
 use core::fmt;
 
-use alloc::vec::Vec;
-
 use crate::RoaringBitmap;
 
 impl fmt::Debug for RoaringBitmap {

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;

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;

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;

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};
 

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;

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;
@@ -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());
+    }
 }

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};
 

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;
 

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;

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,

src/treemap/fmt.rs

@@ -1,7 +1,5 @@
 use core::fmt;
 
-use alloc::vec::Vec;
-
 use crate::RoaringTreemap;
 
 impl fmt::Debug for RoaringTreemap {

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;
 

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;

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};

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};
 

tests/iter.rs

@@ -1,4 +1,3 @@
-use core::iter::FromIterator;
 use proptest::arbitrary::any;
 use proptest::collection::btree_set;
 use proptest::proptest;

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]

tests/push.rs

@@ -1,5 +1,4 @@
 extern crate roaring;
-use core::iter::FromIterator;
 use roaring::{RoaringBitmap, RoaringTreemap};
 
 /// macro created to reduce code duplication

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;

tests/treemap_serialization.rs

@@ -1,6 +1,5 @@
 #![cfg(feature = "std")]
 
-use core::iter::FromIterator;
 use roaring::RoaringTreemap;
 
 fn serialize_deserialize<Dataset, I>(dataset: Dataset)