Issue 3 crashes found by honggfuzz

src/header.rs

@@ -61,6 +61,13 @@ pub struct Header {
   pub hash_type: HashType,
 }
 
+const HEADER_LENGTH: usize = 32;
+const MAX_ALGORITHM_NAME_LENGTH: usize = HEADER_LENGTH - 8;
+
+/// According to https://github.com/datproject/docs/blob/master/papers/sleep.md trailing bytes
+/// should be zeros, so garbage is probably fine too.
+const VERIFY_TRAILING_ZEROS: bool = false;
+
 impl Header {
   /// Create a new `Header`.
   pub fn new(
@@ -76,54 +83,48 @@ impl Header {
     }
   }
 
-  /// Parse a 32 bit buffer slice into a valid Header.
+  /// Parse a 32 byte buffer slice into a valid Header.
   pub fn from_vec(buffer: &[u8]) -> Result<Header, Error> {
     ensure!(buffer.len() == 32, "buffer should be 32 bytes");
 
     let mut rdr = Cursor::new(buffer);
     let byte = rdr.read_u8().unwrap();
     ensure!(
       byte == 5,
-      format!(
-        "The first byte of a SLEEP header should be '5', found {}",
-        byte
-      )
+      "The first byte of a SLEEP header should be '5', found {}",
+      byte
     );
 
     let byte = rdr.read_u8().unwrap();
     ensure!(
       byte == 2,
-      format!(
-        "The second byte of a SLEEP header should be '2', found {}",
-        byte
-      )
+      "The second byte of a SLEEP header should be '2', found {}",
+      byte
     );
 
     let byte = rdr.read_u8().unwrap();
     ensure!(
       byte == 87,
-      format!(
-        "The third byte of a SLEEP header should be '87', found {}",
-        byte
-      )
+      "The third byte of a SLEEP header should be '87', found {}",
+      byte
     );
 
     let file_type = match rdr.read_u8().unwrap() {
       0 => FileType::BitField,
       1 => FileType::Signatures,
       2 => FileType::Tree,
-      num => bail!(format!(
+      num => bail!(
         "The fourth byte '{}' does not belong to any known SLEEP file type",
         num
-      )),
+      ),
     };
 
     let protocol_version = match rdr.read_u8().unwrap() {
       0 => ProtocolVersion::V0,
-      num => bail!(format!(
+      num => bail!(
         "The fifth byte '{}' does not belong to any known SLEEP protocol protocol_version",
         num
-      )),
+      ),
     };
 
     // Read entry size which will inform how many bytes to read next.
@@ -134,21 +135,43 @@ impl Header {
     let hash_name_len = rdr.read_u8().unwrap() as usize;
     let current = rdr.position() as usize;
 
+    ensure!(
+      hash_name_len <= MAX_ALGORITHM_NAME_LENGTH,
+      "Algorithm name is too long: {} (max: {})",
+      hash_name_len,
+      MAX_ALGORITHM_NAME_LENGTH
+    );
+
     let hash_name_upper = current + hash_name_len;
+    ensure!(
+      buffer.len() >= hash_name_upper,
+      "Broken parser: algorithm name is out of bounds: {} {}",
+      hash_name_upper,
+      buffer.len()
+    );
+
     let buf_slice = &buffer[current..hash_name_upper];
     rdr.set_position(hash_name_upper as u64 + 1);
-    let algo = ::std::str::from_utf8(buf_slice)
-      .expect("The algorithm string was invalid utf8 encoded");
+    let algo = ::std::str::from_utf8(buf_slice).map_err(|e| {
+      format_err!("The algorithm string was invalid utf8 encoded: {:?}", e)
+    })?;
 
     let hash_type = match algo {
       "BLAKE2b" => HashType::BLAKE2b,
       "Ed25519" => HashType::Ed25519,
-      _ => HashType::None,
+      "" => HashType::None,
+      name => bail!("Unexpected algorithm name: {}", name),
     };
 
-    for index in rdr.position()..32 {
-      let byte = rdr.read_u8().unwrap();
-      ensure!(byte == 0, format!("The remainder of the header should be zero-filled. Found byte '{}' at position '{}'.", byte, index));
+    if VERIFY_TRAILING_ZEROS {
+      for index in rdr.position()..32 {
+        let byte = rdr.read_u8().unwrap();
+        ensure!(
+          byte == 0,
+          "The remainder of the header should be zero-filled. Found byte '{}' at position '{}'.",
+          byte, index
+        );
+      }
     }
 
     Ok(Header {

tests/header.rs

@@ -2,68 +2,62 @@ extern crate sleep_parser;
 
 use sleep_parser::*;
 use std::fs::File;
-use std::io::{BufRead, BufReader};
+use std::io::Read;
 
 #[test]
 fn new_header() {
   Header::new(FileType::Tree, 40, HashType::BLAKE2b);
 }
 
+fn read_header_bytes(file_name: &str) -> Result<[u8; 32], std::io::Error> {
+  let mut file = File::open(file_name)?;
+  let mut buffer = [0u8; 32];
+  file.read_exact(&mut buffer).map(|_| buffer)
+}
+
 #[test]
 fn from_vec_content_bitfield() {
-  let file = File::open("tests/fixtures/content.bitfield").unwrap();
-  let mut reader = BufReader::with_capacity(32, file);
-  let buffer = reader.fill_buf().unwrap();
+  let buffer = read_header_bytes("tests/fixtures/content.bitfield").unwrap();
   let header = Header::from_vec(&buffer).unwrap();
   assert!(header.is_bitfield());
   assert_eq!(header.to_vec(), buffer);
 }
 
 #[test]
 fn from_vec_content_signatures() {
-  let file = File::open("tests/fixtures/content.signatures").unwrap();
-  let mut reader = BufReader::with_capacity(32, file);
-  let buffer = reader.fill_buf().unwrap();
+  let buffer = read_header_bytes("tests/fixtures/content.signatures").unwrap();
   let header = Header::from_vec(&buffer).unwrap();
   assert!(header.is_signatures());
   assert_eq!(header.to_vec(), buffer);
 }
 
 #[test]
 fn from_vec_content_tree() {
-  let file = File::open("tests/fixtures/content.tree").unwrap();
-  let mut reader = BufReader::with_capacity(32, file);
-  let buffer = reader.fill_buf().unwrap();
+  let buffer = read_header_bytes("tests/fixtures/content.tree").unwrap();
   let header = Header::from_vec(&buffer).unwrap();
   assert!(header.is_tree());
   assert_eq!(header.to_vec(), buffer);
 }
 
 #[test]
 fn from_vec_metadata_bitfield() {
-  let file = File::open("tests/fixtures/metadata.bitfield").unwrap();
-  let mut reader = BufReader::with_capacity(32, file);
-  let buffer = reader.fill_buf().unwrap();
+  let buffer = read_header_bytes("tests/fixtures/metadata.bitfield").unwrap();
   let header = Header::from_vec(&buffer).unwrap();
   assert!(header.is_bitfield());
   assert_eq!(header.to_vec(), buffer);
 }
 
 #[test]
 fn from_vec_metadata_signatures() {
-  let file = File::open("tests/fixtures/metadata.signatures").unwrap();
-  let mut reader = BufReader::with_capacity(32, file);
-  let buffer = reader.fill_buf().unwrap();
+  let buffer = read_header_bytes("tests/fixtures/metadata.signatures").unwrap();
   let header = Header::from_vec(&buffer).unwrap();
   assert!(header.is_signatures());
   assert_eq!(header.to_vec(), buffer);
 }
 
 #[test]
 fn from_vec_metadata_tree() {
-  let file = File::open("tests/fixtures/metadata.tree").unwrap();
-  let mut reader = BufReader::with_capacity(32, file);
-  let buffer = reader.fill_buf().unwrap();
+  let buffer = read_header_bytes("tests/fixtures/metadata.tree").unwrap();
   let header = Header::from_vec(&buffer).unwrap();
   assert!(header.is_tree());
   assert_eq!(header.to_vec(), buffer);
@@ -80,3 +74,34 @@ fn to_vec() {
     ]
   );
 }
+
+#[test]
+fn issue_3() {
+  // https://github.com/datrs/sleep-parser/issues/3
+
+  let data = b"\x05\x02W\x01\x00\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xfb\x03p\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xb0\xbb9\xb0\xf5\xf5";
+  assert!(Header::from_vec(data).is_err());
+
+  let data = b"\x05\x02W\x01\x00\x00\x00\x12\x12\x12\x00\x00S\xc3\xcf\x8a2\xcc\xd1\xce9\xc4K\x9343\x00602\xb5\x07";
+  assert!(Header::from_vec(data).is_err());
+}
+
+#[test]
+fn invalid_algorithm() {
+  fn mk_header(prefix: &[u8]) -> [u8; 32] {
+    let mut h = [0u8; 32];
+    h[0..prefix.len()].clone_from_slice(prefix);
+    h
+  }
+
+  assert!(
+    Header::from_vec(&mk_header(b"\x05\x02W\x01\x00\x00\x28\x01B")).is_err()
+  );
+  assert!(
+    Header::from_vec(&mk_header(b"\x05\x02W\x01\x00\x00\x28\x01B")).is_err()
+  );
+  assert!(
+    Header::from_vec(b"\x05\x02W\x01\x00\x00\x28\x19BLAKE2bXXXXXXXXXXXXXXXXXX")
+      .is_err()
+  );
+}