Ensure signed reads/writes have at least 1 bit for sign

This would cause underflow panics before, but should now generate proper errors at runtime or compile-time.
tuffy · Jun 17, 2024 · d32a289 · d32a289
1 parent 8b00494
commit d32a289
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 15 deletions.
diff --git a/src/read.rs b/src/read.rs
@@ -664,13 +664,16 @@ impl<R: io::Read, E: Endianness> BitRead for BitReader<R, E> {
     where
         S: SignedNumeric,
     {
-        if bits <= S::BITS_SIZE {
-            E::read_signed(self, bits)
-        } else {
-            Err(io::Error::new(
+        match bits {
+            0 => Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "signed reads need at least 1 bit for sign",
+            )),
+            bits if bits <= S::BITS_SIZE => E::read_signed(self, bits),
+            _ => Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 "excessive bits for type read",
-            ))
+            )),
         }
     }
 
@@ -698,6 +701,7 @@ impl<R: io::Read, E: Endianness> BitRead for BitReader<R, E> {
         S: SignedNumeric,
     {
         const {
+            assert!(BITS > 0, "signed reads need at least 1 bit for sign");
             assert!(BITS <= S::BITS_SIZE, "excessive bits for type read");
         }
         E::read_signed_fixed::<_, BITS, S>(self)

diff --git a/src/write.rs b/src/write.rs
@@ -739,13 +739,16 @@ impl<W: io::Write, E: Endianness> BitWrite for BitWriter<W, E> {
     where
         S: SignedNumeric,
     {
-        if bits > S::BITS_SIZE {
-            Err(io::Error::new(
+        match bits {
+            0 => Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "signed writes need at least 1 bit for sign",
+            )),
+            bits if bits <= S::BITS_SIZE => E::write_signed(self, bits, value),
+            _ => Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 "excessive bits for type written",
-            ))
-        } else {
-            E::write_signed(self, bits, value)
+            )),
         }
     }
 
@@ -773,6 +776,7 @@ impl<W: io::Write, E: Endianness> BitWrite for BitWriter<W, E> {
         S: SignedNumeric,
     {
         const {
+            assert!(BITS > 0, "signed writes need at least 1 bit for sign");
             assert!(BITS <= S::BITS_SIZE, "excessive bits for type written");
         }
         E::write_signed_fixed::<_, BITS, S>(self, value)
@@ -938,13 +942,16 @@ where
     where
         S: SignedNumeric,
     {
-        if bits > S::BITS_SIZE {
-            Err(io::Error::new(
+        match bits {
+            0 => Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "signed writes need at least 1 bit for sign",
+            )),
+            bits if bits <= S::BITS_SIZE => E::write_signed(self, bits, value),
+            _ => Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 "excessive bits for type written",
-            ))
-        } else {
-            E::write_signed(self, bits, value)
+            )),
         }
     }
 
@@ -954,6 +961,7 @@ where
         S: SignedNumeric,
     {
         const {
+            assert!(BITS > 0, "signed writes need at least 1 bit for sign");
             assert!(BITS <= S::BITS_SIZE, "excessive bits for type written");
         }
         E::write_signed_fixed::<_, BITS, S>(self, value)

diff --git a/tests/read.rs b/tests/read.rs
@@ -202,6 +202,12 @@ fn test_edge_cases_be() {
     assert_eq!(r.read_in::<0, u64>().unwrap(), 0);
     assert_eq!(r.read_in::<8, u8>().unwrap(), 255);
 
+    let mut r = BitReader::endian(Cursor::new(vec![255]), BigEndian);
+    assert!(r.read_signed::<i8>(0).is_err());
+    assert!(r.read_signed::<i16>(0).is_err());
+    assert!(r.read_signed::<i32>(0).is_err());
+    assert!(r.read_signed::<i64>(0).is_err());
+
     /*unsigned 32 and 64-bit values*/
     let mut r = BitReader::endian(Cursor::new(&data), BigEndian);
     assert_eq!(r.read::<u32>(32).unwrap(), 0);
@@ -424,6 +430,12 @@ fn test_edge_cases_le() {
     assert_eq!(r.read_in::<0, u64>().unwrap(), 0);
     assert_eq!(r.read_in::<8, u8>().unwrap(), 255);
 
+    let mut r = BitReader::endian(Cursor::new(vec![255]), LittleEndian);
+    assert!(r.read_signed::<i8>(0).is_err());
+    assert!(r.read_signed::<i16>(0).is_err());
+    assert!(r.read_signed::<i32>(0).is_err());
+    assert!(r.read_signed::<i64>(0).is_err());
+
     /*unsigned 32 and 64-bit values*/
     let mut r = BitReader::endian(Cursor::new(&data), LittleEndian);
     assert_eq!(r.read::<u32>(32).unwrap(), 0);

diff --git a/tests/write.rs b/tests/write.rs
@@ -294,6 +294,21 @@ fn test_writer_be() {
 fn test_writer_edge_cases_be() {
     use bitstream_io::{BigEndian, BitWrite, BitWriter};
 
+    // 0 bit writes
+    let mut w = BitWriter::endian(Vec::new(), BigEndian);
+    w.write(0, 0u8).unwrap();
+    w.write(0, 0u16).unwrap();
+    w.write(0, 0u32).unwrap();
+    w.write(0, 0u64).unwrap();
+    assert!(w.into_writer().is_empty());
+
+    let mut w = BitWriter::endian(Vec::new(), BigEndian);
+    assert!(w.write_signed(0, 0i8).is_err());
+    assert!(w.write_signed(0, 0i16).is_err());
+    assert!(w.write_signed(0, 0i32).is_err());
+    assert!(w.write_signed(0, 0i64).is_err());
+    assert!(w.into_writer().is_empty());
+
     let final_data: Vec<u8> = vec![
         0, 0, 0, 0, 255, 255, 255, 255, 128, 0, 0, 0, 127, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0,
         255, 255, 255, 255, 255, 255, 255, 255, 128, 0, 0, 0, 0, 0, 0, 0, 127, 255, 255, 255, 255,
@@ -607,6 +622,21 @@ fn test_writer_le() {
 fn test_writer_edge_cases_le() {
     use bitstream_io::{BitWrite, BitWriter, LittleEndian};
 
+    // 0 bit writes
+    let mut w = BitWriter::endian(Vec::new(), LittleEndian);
+    w.write(0, 0u8).unwrap();
+    w.write(0, 0u16).unwrap();
+    w.write(0, 0u32).unwrap();
+    w.write(0, 0u64).unwrap();
+    assert!(w.into_writer().is_empty());
+
+    let mut w = BitWriter::endian(Vec::new(), LittleEndian);
+    assert!(w.write_signed(0, 0i8).is_err());
+    assert!(w.write_signed(0, 0i16).is_err());
+    assert!(w.write_signed(0, 0i32).is_err());
+    assert!(w.write_signed(0, 0i64).is_err());
+    assert!(w.into_writer().is_empty());
+
     let final_data: Vec<u8> = vec![
         0, 0, 0, 0, 255, 255, 255, 255, 0, 0, 0, 128, 255, 255, 255, 127, 0, 0, 0, 0, 0, 0, 0, 0,
         255, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 128, 255, 255, 255, 255, 255,