Merge pull request #176 from github/aneubeck-triage-rust-gems-security-alerts

Fix remaining medium and low code-scanning security alerts

Author: aneubeck

crates/bpe-openai/src/lib.rs

@@ -209,8 +209,11 @@ impl<'a> Iterator for Splits<'a> {
                 .next_back()
                 .expect("Expected at least a look-ahead character!");
             end -= last.len_utf8();
-            assert_ne!(end, start, "a look-ahead pattern must ALWAYS consume at least one character excluding the look-ahead character!");
         }
+        assert_ne!(
+            end, start,
+            "a pattern must ALWAYS consume at least one character!"
+        );
         self.last = end;
         Some(&self.text[start..end])
     }
@@ -277,4 +280,11 @@ mod tests {
             None
         );
     }
+
+    #[test]
+    #[should_panic(expected = "a pattern must ALWAYS consume at least one character!")]
+    fn test_empty_regex_match_panics() {
+        let pre = Pretokenizer::new("a*").expect("valid regex");
+        let _ = pre.split("b").next();
+    }
 }

crates/consistent-choose-k/src/consistent_reservoir.rs

@@ -286,6 +286,7 @@ impl ConsistentReservoir {
     pub fn new(k: u32, n: u32, master_key: u64) -> Self {
         assert!(k >= 1, "k must be at least 1");
         assert!(n <= 1u32 << 30, "n must be at most 2^30");
+        assert!(k <= n, "k must be at most n");
         let j_max = smallest_j_max(n);
         let m = 1u32 << (2 * j_max + 2);
         let mut values = vec![];
@@ -508,4 +509,10 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    #[should_panic(expected = "k must be at most n")]
+    fn new_panics_when_k_exceeds_n() {
+        let _ = ConsistentReservoir::new(5, 4, 42);
+    }
 }

crates/geo_filters/src/config.rs

@@ -336,7 +336,7 @@ pub(crate) fn hash_to_bucket(phi: f64, hash: u64) -> usize {
 /// to most significant bits.
 /// The function returns the zero-based bit index of that one bit.
 /// If no such one bit exist, the function will return 64.
-#[cfg(target_arch = "x86_64")]
+#[cfg(all(target_arch = "x86_64", target_feature = "bmi2"))]
 pub(crate) fn nth_one(value: u64, n: u32) -> u32 {
     unsafe { std::arch::x86_64::_pdep_u64(1 << n, value).trailing_zeros() }
 }
@@ -345,7 +345,7 @@ pub(crate) fn nth_one(value: u64, n: u32) -> u32 {
 /// to most significant bits.
 /// The function returns the zero-based bit index of that one bit.
 /// If no such one bit exist, the function will return 64.
-#[cfg(not(target_arch = "x86_64"))]
+#[cfg(not(all(target_arch = "x86_64", target_feature = "bmi2")))]
 pub(crate) fn nth_one(mut value: u64, mut n: u32) -> u32 {
     while n > 0 && value != 0 {
         value ^= 1 << value.trailing_zeros();

crates/geo_filters/src/config/bitchunks.rs

@@ -161,6 +161,10 @@ pub(crate) fn xor_bit_chunks(
 
 /// ANDs the bit chunks with the repetitive mask without compressing the result to the one bits.
 /// The result will only contain non-zero BitChunks!
+///
+/// # Panics
+///
+/// Panics if `modulus` is not in `1..`[`BITS_PER_BLOCK`].
 pub(crate) fn mask_bit_chunks(
     iter: impl Iterator<Item = BitChunk>,
     mask: u64,
@@ -176,7 +180,16 @@ struct MaskedBitChunksIterator<I: Iterator<Item = BitChunk>> {
 }
 
 impl<I: Iterator<Item = BitChunk>> MaskedBitChunksIterator<I> {
+    /// Creates a new iterator that masks each [`BitChunk`] with the `modulus`-bit `mask`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `modulus` is not in `1..`[`BITS_PER_BLOCK`].
     fn new(iter: I, mask: u64, modulus: usize) -> Self {
+        assert!(
+            (1..BITS_PER_BLOCK).contains(&modulus),
+            "mask_size must be in 1..{BITS_PER_BLOCK} (got {modulus})"
+        );
         let mut block_mask = mask;
         let mut i = modulus;
         while i < BITS_PER_BLOCK {

crates/geo_filters/src/diff_count.rs

@@ -127,6 +127,10 @@ impl<'a, C: GeoConfig<Diff>> GeoDiffCount<'a, C> {
     /// To reduce the number of operations, the implementation first xors the bit chunks together,
     /// then applies the mask (without compressing the mask), and finally looks for the first
     /// modified bit. This bit must be set in the larger geometric filter.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `mask_size` is not in `1..64`.
     pub fn cmp_masked(&self, other: &Self, mask: u64, mask_size: usize) -> Ordering {
         let mut differing_bit = mask_bit_chunks(
             xor_bit_chunks(self.bit_chunks(), other.bit_chunks()),
@@ -159,10 +163,15 @@ impl<'a, C: GeoConfig<Diff>> GeoDiffCount<'a, C> {
     /// When two keys are equal the ordering is undetermined and the caller must fall back to
     /// [`Self::cmp_masked`], e.g.
     /// `a_key.cmp(&b_key).then_with(|| a.cmp_masked(b, mask, mask_size))`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `mask_size` is not in `1..64` or if [`GeoConfig::bucket_position_bits`] is not in
+    /// `1..=64`.
     pub fn masked_sort_key(&self, mask: u64, mask_size: usize) -> u64 {
         assert!(
-            (1..u64::BITS as usize).contains(&mask_size),
-            "mask_size must be in 1..=63 (got {mask_size})"
+            (1..BITS_PER_BLOCK).contains(&mask_size),
+            "mask_size must be in 1..{BITS_PER_BLOCK} (got {mask_size})"
         );
         let position_bits = self.config.bucket_position_bits();
         assert!(
@@ -1278,4 +1287,12 @@ mod tests {
             serialization_round_trip::<GeoDiffConfig13>(rnd);
         });
     }
+
+    #[test]
+    #[should_panic(expected = "mask_size must be in 1..64")]
+    fn test_cmp_masked_zero_mask_size_panics() {
+        let a = GeoDiffCount7::default();
+        let b = GeoDiffCount7::default();
+        let _ = a.cmp_masked(&b, 0, 0);
+    }
 }

crates/string-offsets/src/lib.rs

@@ -151,6 +151,10 @@ pub struct Pos {
 // line ranges for empty character ranges in the middle of a line...
 impl<C: ConfigType> StringOffsets<C> {
     /// Create a new converter to work with offsets into the given string.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `content.len() >= u32::MAX as usize`.
     pub fn new(content: &str) -> Self {
         new_converter(content.as_bytes())
     }
@@ -159,6 +163,10 @@ impl<C: ConfigType> StringOffsets<C> {
     ///
     /// If `content` is UTF-8, this is just like [`StringOffsets::new`]. Otherwise, the
     /// conversion methods will produce unspecified (but memory-safe) results.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `content.len() >= u32::MAX as usize`.
     pub fn from_bytes(content: &[u8]) -> Self {
         new_converter(content)
     }
@@ -190,7 +198,7 @@ impl<C: ConfigType<HasLines = True>> StringOffsets<C> {
 
     /// UTF-8 offset of the first character of a line.
     pub fn line_to_utf8_end(&self, line_number: usize) -> usize {
-        self.line_to_utf8_begin(line_number + 1)
+        self.line_to_utf8_begin(line_number.saturating_add(1))
     }
 
     /// Return the zero-based line number of the line containing the specified UTF-8 offset.
@@ -288,7 +296,9 @@ impl<C: ConfigType<HasWhitespace = True>> StringOffsets<C> {
 impl<C: ConfigType<HasChars = True>> StringOffsets<C> {
     /// Converts a UTF-8 offset to a UTF-32 offset.
     pub fn utf8_to_char(&self, byte_number: usize) -> usize {
-        self.utf8_to_char.rank(byte_number + 1) - 1
+        self.utf8_to_char
+            .rank(byte_number.saturating_add(1))
+            .saturating_sub(1)
     }
 
     /// Converts a UTF-32 offset to a UTF-8 offset.
@@ -369,8 +379,17 @@ impl<C: ConfigType<HasChars = True, HasLines = True, HasUtf16 = True>> StringOff
     }
 }
 
+/// Creates a [`StringOffsets`] converter for `content`.
+///
+/// # Panics
+///
+/// Panics if `content.len() >= u32::MAX as usize`.
 fn new_converter<C: ConfigType>(content: &[u8]) -> StringOffsets<C> {
     let n = content.len();
+    assert!(
+        n < u32::MAX as usize,
+        "content length + 1 must fit in u32 (got {n})"
+    );
     let mut utf8_builder =
         BitRankBuilder::with_capacity(if C::HasChars::VALUE { n + 1 } else { 0 });
     let mut utf16_builder = BitRankBuilder::with_capacity(if C::HasUtf16::VALUE { n } else { 0 });
@@ -649,4 +668,16 @@ line1
         let lines: StringOffsets = StringOffsets::from_bytes(&content);
         assert_eq!(lines.utf8_to_utf16_pos(16384), pos(1, 0));
     }
+
+    #[test]
+    fn test_saturating_usize_max_offsets() {
+        let content = "hello\nworld";
+        let offsets: StringOffsets = StringOffsets::new(content);
+        assert_eq!(offsets.utf8_to_char(usize::MAX), content.chars().count());
+        assert_eq!(
+            offsets.utf8_to_utf16(usize::MAX),
+            content.encode_utf16().count()
+        );
+        assert_eq!(offsets.line_to_utf8_end(usize::MAX), content.len());
+    }
 }