Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating an archive when user UID >= 1000000 results in a corrupt archive... #29

Closed
gregoster opened this issue Mar 24, 2024 · 2 comments

Comments

@gregoster
Copy link

If the user creating an archive has a UID greater than or equal to 1000000 (i.e. 7 characters instead of 6), the archive builder writes all 7 characters instead of limiting it to 6. This creates an invalid archive header which can no longer be read. This issue is also present for GID and other header fields where the actual size of the field is not checked before writing out the header data.

GNU ar version 2.40-14.fc39 only uses the first 6 digits of the UID, and drops the rest. Basically any strategy that doesn't break the headers is probably acceptable.

This bug was discovered building https://github.com/lichess-org/fishnet and wondering why it was throwing a:
Custom { kind: InvalidData, error: "Invalid file size field in entry header (" 454552 ")" }
error. #10 gave me the first hint it might be a rust-ar thing.

Code used for testing is as follows (mostly borrwed from the documentation):


use std::fs::File;
use ar::{Builder,Archive};

fn main() {
    {
        // Create a new archive that will be written to foo.a:
        let mut builder = Builder::new(File::create("foo.a").unwrap());
        // Add foo/bar.txt to the archive, under the name "bar.txt":
        builder.append_path("bar.txt").unwrap();
    }
    let mut testarchive = Archive::new(File::open("foo.a").unwrap());
    while let Some(entry) = testarchive.next_entry() {
	      entry.unwrap();
    }
}




@mdsteele
Copy link
Owner

Whoops, good find. My gut inclination would be to use UID mod 1000000, but it's probably better to just match whatever GNU ar does (or any other major implementation).

A PR to fix this would be very welcome.

@gregoster
Copy link
Author

See #31 . I'm not a rust person at all (this is my first real venture into the land of writing in rust), so there may be a cleaner way of doing this.

Only one code path was tested, but the other changes are mechanical.

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

No branches or pull requests

2 participants