Skip to content

Out-of-range FAT entry rejects the whole file even under Validation::Permissive #80

Description

@ilgu-dcty

Summary

An out-of-range FAT pointee rejects the whole file even under Validation::Permissive (i.e. plain CompoundFile::open), including when no stream chain traverses the bad entry. olefile reads the same files, because it validates lazily and only follows the chains it actually needs.

This is a different check from the count mismatch in #8 / #41 (FAT has N entries, but file has only M sectors). Those are fixed. This one is alloc.rs:

for (from_sector, &to_sector) in self.fat.iter().enumerate() {
    if to_sector <= consts::MAX_REGULAR_SECTOR {
        if to_sector as usize >= self.fat.len() {
            malformed!("FAT has {} entries, but sector {} points to {}", ...);

It is not gated on validation, unlike the neighbouring FAT sector {} is not marked as such in the FAT check a few lines above, which repairs under Permissive and has a fat_sector_not_marked_in_fat_permissive test.

Reproduction (cfb 0.14.0)

Self-contained, no fixture file needed. Builds a valid v3 CFB, frees a stream to leave a hole in the middle of the FAT, then sets one FREESECT slot to an out-of-range value. No chain traverses it; every stream is still fully reachable.

use std::io::{Cursor, Write};

fn le_u16(b: &[u8], at: usize) -> u16 { u16::from_le_bytes([b[at], b[at + 1]]) }
fn le_u32(b: &[u8], at: usize) -> u32 {
    u32::from_le_bytes([b[at], b[at + 1], b[at + 2], b[at + 3]])
}

fn main() {
    let mut buf = Cursor::new(Vec::new());
    {
        let mut comp =
            cfb::CompoundFile::create_with_version(cfb::Version::V3, &mut buf).unwrap();
        comp.create_storage("/BodyText").unwrap();
        comp.create_stream("/scratch").unwrap().write_all(&vec![1u8; 60_000]).unwrap();
        comp.create_stream("/BodyText/Section0")
            .unwrap()
            .write_all(&vec![7u8; 60_000])
            .unwrap();
        comp.remove_stream("/scratch").unwrap();   // leaves FREESECTs mid-FAT
        comp.flush().unwrap();
    }
    let mut bytes = buf.into_inner();
    assert!(cfb::CompoundFile::open(Cursor::new(bytes.clone())).is_ok(), "baseline must open");

    let sector_size = 1usize << le_u16(&bytes, 30);
    let sector_count = bytes.len() / sector_size - 1;
    let fat_base = (le_u32(&bytes, 76) as usize + 1) * sector_size;

    let (idx, at) = (0..(sector_size / 4).min(sector_count))
        .find(|i| le_u32(&bytes, fat_base + 4 * i) == u32::MAX)
        .map(|i| (i, fat_base + 4 * i))
        .expect("no FREESECT slot inside the live range");
    println!("corrupting FAT[{idx}] (was FREESECT), sector_count={sector_count}");
    bytes[at..at + 4].copy_from_slice(&0x1E55_5E69u32.to_le_bytes());

    match cfb::CompoundFile::open(Cursor::new(bytes)) {
        Ok(_) => println!("OPENED (no repro)"),
        Err(e) => println!("REJECTED: {e}"),
    }
}

Output:

corrupting FAT[2] (was FREESECT), sector_count=239
REJECTED: Malformed FAT (FAT has 239 entries, but sector 2 points to 508911209)

Where I hit it in the wild

Sweeping a corpus of 99 legacy Korean HWP 5.0 documents (.hwp is an OLE compound file), one 5.7 MB file was rejected:

Malformed FAT (FAT has 11165 entries, but sector 7296 points to 508925609)

In that file, 16 FAT sectors' worth of entries (7296..9343) hold document data rather than FAT. The FileHeader and BodyText/Section* chains never pass through that region, and olefile extracts them cleanly. I checked the extracted text was real content and not salvage noise: 468 characters, 55.6% Hangul, zero U+FFFD, zero stray control characters. Byte-for-byte identical to what olefile returns.

So this is one file in 99 (~1%) where cfb discards recoverable content that another implementation reads without complaint. I cannot share the file (customer document), which is why the reproducer above is synthetic.

Suggested fix

Under Validation::Permissive, treat an out-of-range pointee as FREE_SECTOR / END_OF_CHAIN rather than rejecting the file, so only a chain that actually traverses the bad entry is truncated. Validation::Strict keeps rejecting. That matches:

Happy to send a PR if that direction looks right.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions