parse: Fix empty uname/gname and invalid device number handling#13
Closed
parse: Fix empty uname/gname and invalid device number handling#13
Conversation
The parse.rs and differential.rs fuzz targets were getting almost zero coverage of deeper parser logic (PAX extensions, GNU long name/link, sparse files, field parsing, etc.) because random fuzz input almost never has valid tar header checksums. The parser's verify_checksum() call at the top of parse_header() rejects ~100% of random inputs immediately, causing the fuzzer to break on every input before exercising any interesting code paths. For the parse.rs fuzzer, add a Parser::set_verify_checksums(bool) API that allows skipping checksum verification entirely. The fuzzer uses this ~90% of the time (determined by the first byte of input), letting it exercise all the parsing code paths beyond the checksum gate. The remaining 10% keeps checksum validation testing. For the differential fuzzer, since both tar-core and tar-rs must see identical data with valid checksums, use a fixup_checksums() approach that walks through 512-byte blocks and rewrites checksum fields in-place before passing to both parsers. It also parses the size field to skip over content blocks, so subsequent headers get fixed too. The roundtrip.rs fuzzer already uses EntryBuilder which produces valid checksums, so it does not need changes. Assisted-by: OpenCode (Claude claude-opus-4-6)
…rences The differential fuzzer was hitting false positives from two sources: 1. Truncated archives: tar-core's test harness pushed entries before verifying the full padded content was present. tar-rs refuses to yield entries whose content can't be fully read. Fix by checking that the entire padded content is available before pushing the entry. 2. Parsing leniency: tar-core intentionally accepts some inputs that tar-rs rejects (e.g. all-null numeric fields are parsed as 0, while tar-rs errors on them). Change the entry count assertion to only require tar-core parses at least as many entries as tar-rs, not exactly the same count. Found by running the improved checksum-aware fuzzers from the previous commit. Assisted-by: OpenCode (Claude claude-opus-4-6)
Multiple fixes found by running the improved checksum-aware fuzzers: parse_octal: Truncate at null first, then trim all ASCII whitespace (not just spaces) from both ends. This matches tar-rs's behavior of calling .trim() after null-truncation, fixing a differential where tar-core rejected fields with tab characters that tar-rs accepted. testutil parse_tar_core: Check that enough content data is present before yielding an entry, matching tar-rs's read_exact semantics. testutil parse_tar_rs: Skip metadata entry types (GNU long name/link, PAX headers) that tar-core handles internally. Require numeric fields to parse successfully instead of silently defaulting to 0. Skip entries with empty paths since tar-core rejects those. differential fuzzer: Dump raw header via Header's Debug impl on mismatch for easier diagnosis. Allow tar-core to parse more entries than tar-rs (it's intentionally more lenient for e.g. all-null fields). Assisted-by: OpenCode (Claude claude-opus-4-6)
Fix several sources of differential fuzzer crashes between tar-core and tar-rs: Parser (src/parse.rs): Require recognized magic (is_gnu() || is_ustar()) before treating typeflags L/K/x/g/S as extension entries. A V7 header whose typeflag byte happens to be 'L' should be emitted as a regular entry, not consumed as a GNU long name extension. This matches tar-rs's `is_recognized_header` guard. Octal parsing (src/lib.rs): Add `is_tar_whitespace` helper that treats VT (0x0b) as whitespace, matching real tar implementations. Rust's `u8::is_ascii_whitespace` omits VT, causing fields like "0000000\x0b" to fail. Test harness (testutil/src/lib.rs): Three alignment fixes for `parse_tar_core`: - Call `set_allow_empty_path(true)` so parsing continues past entries with empty paths (previously it would error out early). - Skip entries with empty paths after parsing, matching `parse_tar_rs`. - Skip entries with metadata-only typeflags (L/K/x/g) that tar-core emits as regular entries when magic is unrecognized. `parse_tar_rs` already had an equivalent skip via its `continue` on these types. The root cause of the specific crash artifact: a fuzzed header had typeflag 'L' (GnuLongName) but no valid magic bytes. tar-core treated it as an extension and consumed it, while tar-rs treated it as a regular entry and skipped it. After these fixes, both parsers handle V7-style metadata typeflags consistently. Assisted-by: OpenCode (Claude claude-opus-4-6)
Filter out empty username/groupname values from header fields, treating null-prefixed fields as None instead of Some([]). This matches tar-rs behavior where groupname_bytes()/username_bytes() results are filtered through .filter(|b| !b.is_empty()). Also make device_major/device_minor resilient to garbage octal data by using unwrap_or(None) instead of propagating errors. Headers with non-standard typeflags (e.g. 'j') on UStar/GNU archives may have unparseable devmajor/devminor fields that should not abort parsing. Found via differential fuzzing with minimized artifact containing a UStar header with typeflag 'j' and garbage in devmajor/devminor. Assisted-by: OpenCode (Claude claude-opus-4-6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a differential fuzzer crash found via minimized artifact
minimized-from-c4b08631a4c66e213ed14a7c27588ccde1f28d89(514 bytes: 1 UStar header with typeflag 'j', size=0, garbage in devmajor/devminor).Root cause: Two related issues in the parser:
Empty uname/gname: When a UStar/GNU header has a null-prefixed gname field (starts with
\x00),Header::groupname()returnsSome(&[]). The parser stored this asSome(Cow::Borrowed(&[])), but tar-rs filters empty values toNone. This caused agname: rs=None tc=Some([])mismatch.Invalid device numbers: The parser used
header.device_major()?which propagatesInvalidOctalerrors for garbage devmajor/devminor fields. Headers with non-standard typeflags (like'j') on UStar archives may have unparseable device fields that shouldn't abort parsing.Fixes:
.filter(|b| !b.is_empty())before wrapping inCow, matching tar-rs behaviorunwrap_or(None)for device_major/device_minor instead of?error propagation