diff --git a/src/internal/alloc.rs b/src/internal/alloc.rs index 9fdd8cb..02d762d 100644 --- a/src/internal/alloc.rs +++ b/src/internal/alloc.rs @@ -118,11 +118,16 @@ impl Allocator { 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 Allocator { 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] diff --git a/src/internal/direntry.rs b/src/internal/direntry.rs index 0831d27..89407f6 100644 --- a/src/internal/direntry.rs +++ b/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, diff --git a/src/internal/minialloc.rs b/src/internal/minialloc.rs index 52533e3..08f537e 100644 --- a/src/internal/minialloc.rs +++ b/src/internal/minialloc.rs @@ -138,24 +138,33 @@ impl MiniAllocator { } } 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) -> MiniAllocator>> { + make_minialloc_with(minifat, Validation::Strict) + } + + fn make_minialloc_with( + minifat: Vec, + validation: Validation, + ) -> MiniAllocator>> { 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, root_stream_len: u64, + validation: Validation, ) -> MiniAllocator>> { - 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)"