mdsteele / rust-cfb

Rust library for reading/writing Compound File Binary (structured storage) files
MIT License
44 stars 20 forks source link

Panic on out-of-bounds in minalloc.rs #16

Closed Ben-Lichtman closed 2 years ago

Ben-Lichtman commented 2 years ago

This library may panic by out-of-bounds index on line 47 of minialloc.rs

I have attached a fuzz case which should reproduce this crash. All you need to do is walk the entries of the attached file.

lib_crash.zip

Ben-Lichtman commented 2 years ago

I have found that this problem also occurs in alloc.rs.

I'm not sure if this is the correct course of action but I believe a potential fix could be replacing:

let next_id = self.fat[sector_id as usize];

with

let next_id = *self.fat.get(sector_id as usize).unwrap_or(&consts::END_OF_CHAIN);

(and a similar adjustment for minialloc.rs)

mdsteele commented 2 years ago

Thanks for the report. I can reproduce the crash by running this command on the (unzipped) file you attached above:

cargo run --example cfbtool -- cat lib_crash.cfb:VBA/__SRP_0

I think the best solution is to change MiniAllocator::next_mini_sector() (and also Allocator::next()) to return an io::Result<u32> instead of u32, and return an InvalidData error if the sector_id is out of bounds (since any bad sector IDs will be coming from somewhere else in the CFB file).

I'll put together a change to fix this and add a regression test.