Merge pull request #6 from omerbenamram/bugfix/disallow-reading-random-bytes

hard error on invalid input

4 files changed, 61 insertions, 7 deletions

diff --git a/src/bin/mft_dump.rs b/src/bin/mft_dump.rs
index d24db4f..915abbe 100644
--- a/src/bin/mft_dump.rs
+++ b/src/bin/mft_dump.rs
@@ -429,7 +429,7 @@ fn main() {
     match app.run() {
         Ok(()) => {}
         Err(e) => {
-            eprintln!("A runtime error has occurred {}", &e);
+            eprintln!("A runtime error has occurred: {}", &e);
             exit(1);
         }
     };
diff --git a/src/entry.rs b/src/entry.rs
index 4ee46fc..dade633 100644
--- a/src/entry.rs
+++ b/src/entry.rs
@@ -22,6 +22,10 @@ use std::io::{Cursor, Seek};
 
 const SEQUENCE_NUMBER_STRIDE: usize = 512;
 
+const ZERO_HEADER: &'static [u8; 4] = b"\x00\x00\x00\x00";
+const BAAD_HEADER: &'static [u8; 4] = b"BAAD";
+const FILE_HEADER: &'static [u8; 4] = b"FILE";
+
 #[derive(Debug, Clone)]
 pub struct MftEntry {
     pub header: EntryHeader,
@@ -47,7 +51,7 @@ impl ser::Serialize for MftEntry {
 pub struct EntryHeader {
     /// MULTI_SECTOR_HEADER
     /// The signature. This value is a convenience to the user.
-    /// This is either "BAAD" or "FILE"
+    /// This is either "BAAD", "FILE", or "\x00\x00\x00\x00"
     pub signature: [u8; 4],
     /// The offset to the update sequence array, from the start of this structure.
     /// The update sequence array must end before the last USHORT value in the first sector.
@@ -92,14 +96,19 @@ impl EntryHeader {
         let mut signature = [0; 4];
         reader.read_exact(&mut signature)?;
 
-        // Empty entry
+        let header_is_valid = [FILE_HEADER, BAAD_HEADER, ZERO_HEADER].contains(&&signature);
+
         ensure!(
-            &signature != b"\x00\x00\x00\x00",
+            header_is_valid,
             err::InvalidEntrySignature {
                 bad_sig: signature.to_vec()
             }
         );
 
+        if signature == *ZERO_HEADER {
+            return Ok(Self::zero());
+        }
+
         let usa_offset = reader.read_u16::<LittleEndian>()?;
         let usa_size = reader.read_u16::<LittleEndian>()?;
         let logfile_sequence_number = reader.read_u64::<LittleEndian>()?;
@@ -131,6 +140,31 @@ impl EntryHeader {
             record_number: entry_id,
         })
     }
+
+    pub fn is_valid(&self) -> bool {
+        self.signature == *FILE_HEADER
+    }
+
+    pub fn zero() -> Self {
+        EntryHeader {
+            signature: *ZERO_HEADER,
+            usa_offset: 0,
+            usa_size: 0,
+            metadata_transaction_journal: 0,
+            sequence: 0,
+            hard_link_count: 0,
+            first_attribute_record_offset: 0,
+            flags: EntryFlags::from_bits_truncate(0),
+            used_entry_size: 0,
+            total_entry_size: 0,
+            base_reference: MftReference {
+                entry: 0,
+                sequence: 0,
+            },
+            first_attribute_id: 0,
+            record_number: 0,
+        }
+    }
 }
 
 impl MftEntry {
@@ -143,7 +177,9 @@ impl MftEntry {
         let entry_header = EntryHeader::from_reader(&mut cursor, entry_number)?;
         trace!("Number of sectors: {:#?}", entry_header);
 
-        Self::apply_fixups(&entry_header, &mut buffer)?;
+        if entry_header.is_valid() {
+            Self::apply_fixups(&entry_header, &mut buffer)?;
+        }
 
         Ok(MftEntry {
             header: entry_header,
diff --git a/src/err.rs b/src/err.rs
index bfcad21..78076fb 100644
--- a/src/err.rs
+++ b/src/err.rs
@@ -19,7 +19,10 @@ pub enum Error {
     },
     #[snafu(display("Error while decoding name in filename attribute"))]
     InvalidFilename,
-    #[snafu(display("Bad signature: {:x?}", bad_sig))]
+    #[snafu(display(
+        "Bad signature: {:x?}, expected one of [b\"FILE\", b\"BAAD\", b\"0000\"]",
+        bad_sig
+    ))]
     InvalidEntrySignature { bad_sig: Vec<u8> },
     #[snafu(display("Unknown `AttributeType`: {:04X}", attribute_type))]
     UnknownAttributeType { attribute_type: u32 },
diff --git a/tests/test_cli.rs b/tests/test_cli.rs
index b247ad5..3cac0f5 100644
--- a/tests/test_cli.rs
+++ b/tests/test_cli.rs
@@ -5,7 +5,7 @@ use fixtures::*;
 use assert_cmd::prelude::*;
 use std::fs;
 use std::fs::File;
-use std::io::Read;
+use std::io::{Read, Write};
 use std::process::Command;
 use tempfile::tempdir;
 
@@ -45,6 +45,21 @@ fn test_it_refuses_to_overwrite_directory() {
 }
 
 #[test]
+fn test_non_mft_file_is_error() {
+    let d = tempdir().unwrap();
+
+    let f = d.as_ref().join("test.out");
+
+    let mut file = File::create(&f).unwrap();
+    file.write_all(b"I'm a file!").unwrap();
+
+    let mut cmd = Command::cargo_bin("mft_dump").expect("failed to find binary");
+    cmd.args(&[f.to_str().unwrap()]);
+
+    cmd.assert().failure().code(1);
+}
+
+#[test]
 fn test_it_exports_resident_streams() {
     let d = tempdir().unwrap();