Repair cross-linked chains and bad directory links under Permissive validation

src/internal/alloc.rs

@@ -118,11 +118,16 @@ impl<F> Allocator<F> {
 
     fn validate(&mut self, validation: Validation) -> io::Result<()> {
         if self.fat.len() > self.sectors.num_sectors() as usize {
-            malformed!(
-                "FAT has {} entries, but file has only {} sectors",
-                self.fat.len(),
-                self.sectors.num_sectors()
-            );
+            if validation.is_strict() {
+                malformed!(
+                    "FAT has {} entries, but file has only {} sectors",
+                    self.fat.len(),
+                    self.sectors.num_sectors()
+                );
+            }
+            // Entries past the end of the file describe sectors that aren't
+            // there; any chain running into them gets ended below.
+            self.fat.truncate(self.sectors.num_sectors() as usize);
         }
         for &difat_sector in self.difat_sector_ids.iter() {
             let difat_sector_index = difat_sector as usize;
@@ -178,7 +183,14 @@ impl<F> Allocator<F> {
                     continue;
                 }
                 if pointees.contains(&to_sector) {
-                    malformed!("sector {} pointed to twice", to_sector);
+                    if validation.is_strict() {
+                        malformed!("sector {} pointed to twice", to_sector);
+                    }
+                    // Two chains run into the same sector. End this one
+                    // here, as above, so that the chain that got there first
+                    // keeps its sectors and only this one is truncated.
+                    self.fat[from_sector] = consts::END_OF_CHAIN;
+                    continue;
                 }
                 pointees.insert(to_sector);
             } else if to_sector == consts::INVALID_SECTOR {
@@ -581,6 +593,28 @@ mod tests {
             .unwrap();
     }
 
+    #[test]
+    fn fat_longer_than_file_permissive() {
+        let difat = vec![0];
+        let fat = vec![consts::FAT_SECTOR, 2, consts::END_OF_CHAIN];
+        let sectors = make_sectors(Version::V3, 2);
+        // The entry past the end is dropped, and the chain that ran into it
+        // ends where the file does.
+        let mut allocator = Allocator::new(
+            sectors,
+            vec![],
+            difat,
+            fat,
+            Validation::Permissive,
+        )
+        .unwrap();
+        assert_eq!(
+            allocator.fat,
+            vec![consts::FAT_SECTOR, consts::END_OF_CHAIN]
+        );
+        allocator.validate(Validation::Strict).unwrap();
+    }
+
     #[test]
     #[should_panic(
         expected = "Malformed FAT (FAT has 2 entries, but DIFAT lists 3 as \
@@ -744,10 +778,22 @@ mod tests {
 
     #[test]
     #[should_panic(expected = "Malformed FAT (sector 3 pointed to twice)")]
-    fn double_pointee() {
+    fn double_pointee_strict() {
         let difat = vec![0];
         let fat = vec![consts::FAT_SECTOR, 3, 3, consts::END_OF_CHAIN];
-        make_allocator(difat, fat, Validation::Permissive);
+        make_allocator(difat, fat, Validation::Strict);
+    }
+
+    #[test]
+    fn double_pointee_permissive() {
+        let difat = vec![0];
+        let fat = vec![consts::FAT_SECTOR, 3, 3, consts::END_OF_CHAIN];
+        // The second chain into sector 3 is ended where it would have joined
+        // the first, and the result passes Strict validation.
+        let mut allocator = make_allocator(difat, fat, Validation::Permissive);
+        assert_eq!(allocator.fat[1], 3);
+        assert_eq!(allocator.fat[2], consts::END_OF_CHAIN);
+        allocator.validate(Validation::Strict).unwrap();
     }
 
     #[test]

src/internal/direntry.rs

@@ -185,21 +185,36 @@ impl DirEntry {
                 None => malformed!("invalid color: {}", color_byte),
             }
         };
-        let left_sibling = reader.read_le_u32()?;
+        // Some writers leave a reserved value such as END_OF_CHAIN, or a
+        // child on a stream entry, where there should be no link; under
+        // Permissive validation such a link is read as no link at all.
+        let mut left_sibling = reader.read_le_u32()?;
         if left_sibling != NO_STREAM && left_sibling > MAX_REGULAR_STREAM_ID {
-            malformed!("invalid left sibling: {}", left_sibling);
+            if validation.is_strict() {
+                malformed!("invalid left sibling: {}", left_sibling);
+            }
+            left_sibling = NO_STREAM;
         }
-        let right_sibling = reader.read_le_u32()?;
+        let mut right_sibling = reader.read_le_u32()?;
         if right_sibling != NO_STREAM && right_sibling > MAX_REGULAR_STREAM_ID
         {
-            malformed!("invalid right sibling: {}", right_sibling);
+            if validation.is_strict() {
+                malformed!("invalid right sibling: {}", right_sibling);
+            }
+            right_sibling = NO_STREAM;
         }
-        let child = reader.read_le_u32()?;
+        let mut child = reader.read_le_u32()?;
         if child != NO_STREAM {
             if obj_type == ObjType::Stream {
-                malformed!("non-empty stream child: {}", child);
+                if validation.is_strict() {
+                    malformed!("non-empty stream child: {}", child);
+                }
+                child = NO_STREAM;
             } else if child > MAX_REGULAR_STREAM_ID {
-                malformed!("invalid child: {}", child);
+                if validation.is_strict() {
+                    malformed!("invalid child: {}", child);
+                }
+                child = NO_STREAM;
             }
         }
 
@@ -588,6 +603,50 @@ mod tests {
         assert_eq!(dir_entry.creation_time, Timestamp::zero());
     }
 
+    const INVALID_LINKS_ON_STREAM: [u8; consts::DIR_ENTRY_LEN] = [
+        70, 0, 111, 0, 111, 0, 98, 0, 97, 0, 114, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        0, // name
+        14, 0, // name length
+        2, // obj type
+        1, // color,
+        0xfe, 0xff, 0xff, 0xff, // left sibling
+        34, 0, 0, 0, // right sibling
+        5, 0, 0, 0, // child
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // CLSID
+        0, 0, 0, 0, // state bits
+        0, 0, 0, 0, 0, 0, 0, 0, // created
+        0, 0, 0, 0, 0, 0, 0, 0, // modified
+        0, 0, 0, 0, // start sector
+        0, 0, 0, 0, 0, 0, 0, 0, // stream length
+    ];
+
+    #[test]
+    #[should_panic(
+        expected = "Malformed directory entry (invalid left sibling: \
+                    4294967294)"
+    )]
+    fn invalid_links_on_stream_strict() {
+        let mut input: &[u8] = &INVALID_LINKS_ON_STREAM;
+        DirEntry::read_from(&mut input, Version::V4, Validation::Strict)
+            .unwrap();
+    }
+
+    #[test]
+    fn invalid_links_on_stream_permissive() {
+        let mut input: &[u8] = &INVALID_LINKS_ON_STREAM;
+        let dir_entry = DirEntry::read_from(
+            &mut input,
+            Version::V4,
+            Validation::Permissive,
+        )
+        .unwrap();
+        assert_eq!(dir_entry.left_sibling, consts::NO_STREAM);
+        assert_eq!(dir_entry.right_sibling, 34);
+        assert_eq!(dir_entry.child, consts::NO_STREAM);
+    }
+
     const NON_ZERO_MODIFIED_TIME_ON_STREAM: [u8; consts::DIR_ENTRY_LEN] = [
         70, 0, 111, 0, 111, 0, 98, 0, 97, 0, 114, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,

src/internal/minialloc.rs

@@ -138,24 +138,33 @@ impl<F> MiniAllocator<F> {
             }
         }
         let mut pointees = FnvHashSet::default();
-        for (from_mini_sector, &to_mini_sector) in
-            self.minifat.iter().enumerate()
-        {
+        for from_mini_sector in 0..self.minifat.len() {
+            let to_mini_sector = self.minifat[from_mini_sector];
             if to_mini_sector <= consts::MAX_REGULAR_SECTOR {
                 if to_mini_sector as usize >= self.minifat.len() {
-                    malformed!(
-                        "MiniFAT has {} entries, but mini sector {} points to \
-                         {}",
-                        self.minifat.len(),
-                        from_mini_sector,
-                        to_mini_sector
-                    );
+                    if validation.is_strict() {
+                        malformed!(
+                            "MiniFAT has {} entries, but mini sector {} \
+                             points to {}",
+                            self.minifat.len(),
+                            from_mini_sector,
+                            to_mini_sector
+                        );
+                    }
+                    // As in the FAT, end the chain here so that only a chain
+                    // running through this mini sector is truncated.
+                    self.minifat[from_mini_sector] = consts::END_OF_CHAIN;
+                    continue;
                 }
                 if pointees.contains(&to_mini_sector) {
-                    malformed!(
-                        "mini sector {} pointed to twice",
-                        to_mini_sector
-                    );
+                    if validation.is_strict() {
+                        malformed!(
+                            "mini sector {} pointed to twice",
+                            to_mini_sector
+                        );
+                    }
+                    self.minifat[from_mini_sector] = consts::END_OF_CHAIN;
+                    continue;
                 }
                 pointees.insert(to_mini_sector);
             }
@@ -558,15 +567,26 @@ mod tests {
     use super::MiniAllocator;
 
     fn make_minialloc(minifat: Vec<u32>) -> MiniAllocator<Cursor<Vec<u8>>> {
+        make_minialloc_with(minifat, Validation::Strict)
+    }
+
+    fn make_minialloc_with(
+        minifat: Vec<u32>,
+        validation: Validation,
+    ) -> MiniAllocator<Cursor<Vec<u8>>> {
         let root_stream_len = (consts::MINI_SECTOR_LEN * minifat.len()) as u64;
-        make_minialloc_with_root_stream_len(minifat, root_stream_len)
+        make_minialloc_with_root_stream_len(
+            minifat,
+            root_stream_len,
+            validation,
+        )
     }
 
     fn make_minialloc_with_root_stream_len(
         minifat: Vec<u32>,
         root_stream_len: u64,
+        validation: Validation,
     ) -> MiniAllocator<Cursor<Vec<u8>>> {
-        let validation = Validation::Strict;
         let version = Version::V3;
         let num_sectors = 4; // FAT, Directory, MiniFAT, and mini chain
         let data_len = (1 + num_sectors) * version.sector_len();
@@ -676,7 +696,11 @@ mod tests {
     fn root_stream_too_short() {
         let minifat = vec![1, 2, consts::END_OF_CHAIN];
         let root_stream_len = (2 * consts::MINI_SECTOR_LEN) as u64;
-        make_minialloc_with_root_stream_len(minifat, root_stream_len);
+        make_minialloc_with_root_stream_len(
+            minifat,
+            root_stream_len,
+            Validation::Strict,
+        );
     }
 
     #[test]
@@ -689,6 +713,26 @@ mod tests {
         make_minialloc(minifat);
     }
 
+    #[test]
+    fn pointee_out_of_range_permissive() {
+        let minifat = vec![1, 3];
+        let mut minialloc =
+            make_minialloc_with(minifat, Validation::Permissive);
+        assert_eq!(minialloc.minifat, vec![1, consts::END_OF_CHAIN]);
+        minialloc.validate(Validation::Strict).unwrap();
+    }
+
+    #[test]
+    fn double_pointee_permissive() {
+        let minifat = vec![1, 2, 1];
+        // The chain from mini sector 2 back into 1 is ended there, leaving
+        // 0 -> 1 -> 2 intact.
+        let mut minialloc =
+            make_minialloc_with(minifat, Validation::Permissive);
+        assert_eq!(minialloc.minifat, vec![1, 2, consts::END_OF_CHAIN]);
+        minialloc.validate(Validation::Strict).unwrap();
+    }
+
     #[test]
     #[should_panic(
         expected = "Malformed MiniFAT (mini sector 1 pointed to twice)"