Fix some range checking bugs

danlehmann · Aug 25, 2024 · 4b661d9 · 4b661d9
1 parent 5c1aaa2
commit 4b661d9
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 26 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -40,7 +40,7 @@ impl Display for TryNewError {
 }
 
 #[cfg_attr(feature = "const_convert_and_const_trait_impl", const_trait)]
-pub trait Number: Sized + Copy + Clone {
+pub trait Number: Sized + Copy + Clone + PartialOrd + Ord + PartialEq + Eq {
     type UnderlyingType: Number
         + Debug
         + From<u8>
@@ -150,6 +150,15 @@ macro_rules! impl_number_native {
 
                 #[inline]
                 fn new_<T: Number>(value: T) -> Self {
+                    if T::BITS > Self::BITS as usize {
+                        assert!(value <= T::masked_new(Self::MAX));
+                    }
+                    Self::masked_new(value)
+                }
+
+                #[inline]
+                fn masked_new<T: Number>(value: T) -> Self {
+                    // Primitive types don't need masking
                     match Self::BITS {
                         8 => value.as_u8() as Self,
                         16 => value.as_u16() as Self,
@@ -160,12 +169,6 @@ macro_rules! impl_number_native {
                     }
                 }
 
-                #[inline]
-                fn masked_new<T: Number>(value: T) -> Self {
-                    // Primitive types don't need masking
-                    Self::new_(value)
-                }
-
                 #[inline]
                 fn as_u8(&self) -> u8 { *self as u8 }
 
@@ -330,7 +333,10 @@ macro_rules! uint_impl_num {
 
                 #[inline]
                 fn new_<T: Number>(value: T) -> Self {
-                    Self::new(Self::UnderlyingType::new_(value))
+                    if Self::BITS < T::BITS {
+                        assert!(value <= Self::MAX.value.as_());
+                    }
+                    Self { value: Self::UnderlyingType::masked_new(value) }
                 }
 
                 fn masked_new<T: Number>(value: T) -> Self {
@@ -387,51 +393,46 @@ macro_rules! uint_impl {
                 /// Creates an instance. Panics if the given value is outside of the valid range
                 #[inline]
                 pub const fn new_u8(value: u8) -> Self {
-                    let value = value as $type;
                     if Self::BITS < 8 {
-                        assert!(value <= Self::MAX.value);
+                        assert!(value <= Self::MAX.value as u8);
                     }
-                    Self { value }
+                    Self { value: value as $type }
                 }
 
                 /// Creates an instance. Panics if the given value is outside of the valid range
                 #[inline]
                 pub const fn new_u16(value: u16) -> Self {
-                    let value = value as $type;
                     if Self::BITS < 16 {
-                        assert!(value <= Self::MAX.value);
+                        assert!(value <= Self::MAX.value as u16);
                     }
-                    Self { value }
+                    Self { value: value as $type }
                 }
 
                 /// Creates an instance. Panics if the given value is outside of the valid range
                 #[inline]
                 pub const fn new_u32(value: u32) -> Self {
-                    let value = value as $type;
                     if Self::BITS < 32 {
-                        assert!(value <= Self::MAX.value);
+                        assert!(value <= Self::MAX.value as u32);
                     }
-                    Self { value }
+                    Self { value: value as $type }
                 }
 
                 /// Creates an instance. Panics if the given value is outside of the valid range
                 #[inline]
                 pub const fn new_u64(value: u64) -> Self {
-                    let value = value as $type;
                     if Self::BITS < 64 {
-                        assert!(value <= Self::MAX.value);
+                        assert!(value <= Self::MAX.value as u64);
                     }
-                    Self { value }
+                    Self { value: value as $type }
                 }
 
                 /// Creates an instance. Panics if the given value is outside of the valid range
                 #[inline]
                 pub const fn new_u128(value: u128) -> Self {
-                    let value = value as $type;
                     if Self::BITS < 128 {
-                        assert!(value <= Self::MAX.value);
+                        assert!(value <= Self::MAX.value as u128);
                     }
-                    Self { value }
+                    Self { value: value as $type }
                 }
 
                 /// Creates an instance or an error if the given value is outside of the valid range

diff --git a/tests/tests.rs b/tests/tests.rs
@@ -2083,16 +2083,69 @@ fn new_flexible() {
 #[test]
 #[should_panic]
 fn new_flexible_catches_out_of_bounds() {
-    let a = u10::new(1000);
+    let a = u28::new(0x8000000);
+    let _b = u9::new_(a);
+}
+
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_flexible_catches_out_of_bounds_2() {
+    let a = u28::new(0x0000200);
     let _b = u9::new_(a);
 }
 
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_flexible_catches_out_of_bounds_primitive_type() {
+    let a = u28::new(0x8000000);
+    let _b = u8::new_(a);
+}
+
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_constructors_catch_out_bounds_0() {
+    u7::new_u8(0x80u8);
+}
+
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_constructors_catch_out_bounds_1() {
+    u7::new_u32(0x80000060u32);
+}
+
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_constructors_catch_out_bounds_2() {
+    u7::new_u16(0x8060u16);
+}
+
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_constructors_catch_out_bounds_3() {
+    u7::new_u64(0x80000000_00000060u64);
+}
+
+#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
+#[test]
+#[should_panic]
+fn new_constructors_catch_out_bounds_4() {
+    u7::new_u128(0x80000000_00000000_00000000_00000060u128);
+}
+
 #[cfg(not(feature = "const_convert_and_const_trait_impl"))]
 #[test]
 fn new_masked() {
-    let a = u16::new(1000);
+    let a = u10::new(0b11_01010111);
     let b = u9::masked_new(a);
-    assert_eq!(b.as_u32(), 488);
+    assert_eq!(b.as_u32(), 0b1_01010111);
+    let c = u7::masked_new(a);
+    assert_eq!(c.as_u32(), 0b1010111);
 }
 
 #[cfg(not(feature = "const_convert_and_const_trait_impl"))]