Skip to content

fix: refuse to create entries inside a stream - #84

Open
francisdb wants to merge 2 commits into
mdsteele:masterfrom
francisdb:fix/parent-must-be-storage
Open

fix: refuse to create entries inside a stream#84
francisdb wants to merge 2 commits into
mdsteele:masterfrom
francisdb:fix/parent-must-be-storage

Conversation

@francisdb

Copy link
Copy Markdown
Contributor

Creating a stream or storage did not check that the parent is a storage. Names compare case-insensitively, so with a stream a in the root, create_stream("/A/child") hung the new entry off the stream's directory entry as a child, which strict validation (and other readers) reject.

create_stream, create_new_stream and create_storage now fail with InvalidInput when the parent is a stream.

Found by the new fuzz_write target in the second commit, which drives the write API and reopens the result under strict validation; it runs clean with the fix.

Creating a stream or storage resolved its parent by name without
checking that the parent is a storage. With a stream named `a`,
creating `/A/child` hung the new entry off the stream's directory entry
as its child, which strict validation (and other readers) reject.
The parent must now be a storage; otherwise the call fails with
InvalidInput.

Found by a fuzz target that drives the write API.
Creates, overwrites and removes streams and storages under names taken
from the fuzz input (including names that differ only in case), then
reopens the result under strict validation and checks every entry and
its content against a model of what should be there.
Comment thread tests/basic.rs
let cursor = Cursor::new(Vec::new());
let mut comp = CompoundFile::create(cursor).unwrap();
comp.create_stream("/a").unwrap().write_all(b"data").unwrap();
let err = match comp.create_stream("/A/child") {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this just be let err = comp.create_stream("/A/child").unwrap_err();, similarly to below?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants