diff --git a/src/internal/directory.rs b/src/internal/directory.rs index 8e22da9..a44f6ba 100644 --- a/src/internal/directory.rs +++ b/src/internal/directory.rs @@ -413,6 +413,7 @@ impl Directory { if self.dir_entries.len() % dir_entries_per_sector == 0 { let start_sector = self.dir_start_sector; self.allocator.extend_chain(start_sector, SectorInit::Dir)?; + self.update_num_dir_sectors()?; } // Add a new entry to the end of the directory and return it. let stream_id = self.dir_entries.len() as u32; @@ -420,6 +421,27 @@ impl Directory { Ok(stream_id) } + /// Increase header num_dir_sectors if version V4 + /// note: not updating this value breaks ole32 compatibility + fn update_num_dir_sectors(&mut self) -> io::Result<()> { + let start_sector = self.dir_start_sector; + if self.version() == Version::V4 { + let num_dir_sectors = self.count_directory_sectors(start_sector)?; + self.seek_within_header(40)?.write_u32::(num_dir_sectors)?; + } + Ok(()) + } + + fn count_directory_sectors(&mut self, start_sector: u32) -> io::Result { + let mut num_dir_sectors = 1; + let mut next_sector = self.allocator.next(start_sector)?; + while next_sector != consts::END_OF_CHAIN { + num_dir_sectors += 1; + next_sector = self.allocator.next(next_sector)?; + } + Ok(num_dir_sectors) + } + /// Deallocates the specified directory entry. fn free_dir_entry(&mut self, stream_id: u32) -> io::Result<()> { debug_assert_ne!(stream_id, consts::ROOT_STREAM_ID); @@ -428,6 +450,7 @@ impl Directory { *self.dir_entry_mut(stream_id) = dir_entry; // TODO: Truncate directory chain if last directory sector is now all // unallocated. + // In that case, also call update_num_dir_sectors() Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index 96c146f..73f845b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -493,7 +493,15 @@ impl CompoundFile { let mut dir_entries = Vec::::new(); let mut seen_dir_sectors = FnvHashSet::default(); let mut current_dir_sector = header.first_dir_sector; + let mut dir_sector_count = 1; while current_dir_sector != consts::END_OF_CHAIN { + if validation.is_strict() && header.version == Version::V4 && dir_sector_count > header.num_dir_sectors { + invalid_data!( + "Directory chain includes at least {} sectors which is greater than header num_dir_sectors {}", + dir_sector_count, + header.num_dir_sectors + ); + } if current_dir_sector > consts::MAX_REGULAR_SECTOR { invalid_data!( "Directory chain includes invalid sector index {}", @@ -526,6 +534,7 @@ impl CompoundFile { } } current_dir_sector = allocator.next(current_dir_sector)?; + dir_sector_count += 1; } let mut directory = Directory::new( diff --git a/tests/malformed.rs b/tests/malformed.rs index 5971954..4057593 100644 --- a/tests/malformed.rs +++ b/tests/malformed.rs @@ -456,3 +456,29 @@ fn open_difat_terminate_freesect() { fn open_strict_difat_terminate_freesect() { CompoundFile::open_strict(difat_terminate_in_freesect()).unwrap(); } + +/// Regression test for https://github.com/mdsteele/rust-cfb/issues/52. +#[test] +#[should_panic( +expected = "Directory chain includes at least 2 sectors which is greater than header num_dir_sectors 1" +)] +fn invalid_num_dir_sectors_issue_52() { + // Create a CFB file with 2 sectors for the directory. + let cursor = Cursor::new(Vec::new()); + let mut comp = CompoundFile::create(cursor).unwrap(); + for i in 0..32 { + let path = format!("stream{}", i); + let path = Path::new(&path); + comp.create_stream(path).unwrap(); + } + comp.flush().unwrap(); + + // update the header to contain have num_dir_sectors = 1 + let mut cursor = comp.into_inner(); + cursor.seek(SeekFrom::Start(40)).unwrap(); + cursor.write_all(&[0x01, 0x00, 0x00, 0x00]).unwrap(); + cursor.flush().unwrap(); + + // Read the file back in. + CompoundFile::open_strict(cursor).unwrap(); +}