plumbing: format/packfile, harden malformed pack handling#2113
Merged
Conversation
hiddeco
previously approved these changes
May 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens packfile parsing/iteration against malformed or adversarial pack data, aiming to surface delta/iteration failures to callers and reject malformed pack entries earlier/safer.
Changes:
- Tightens object-type validation (explicitly rejecting Git’s reserved type
5). - Adds stricter object inflation bounds (declared size + 1 byte) and rejects oversized/overflowing sizes during scan/parse.
- Enforces a delta chain depth ceiling (4095) and propagates delta resolution errors during iteration.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plumbing/object.go | Makes ObjectType.Valid() explicitly whitelist valid object types (reject reserved type 5). |
| plumbing/format/packfile/scanner.go | Adds copyObjectContent size enforcement; updates inflation paths to use declared size; adds size overflow rejection. |
| plumbing/format/packfile/parser.go | Uses size-bounded inflation, adds max delta chain depth enforcement, and reuses copyObjectContent. |
| plumbing/format/packfile/packfile.go | Propagates scanner errors from headerFromOffset; uses size-bounded inflation in object retrieval. |
| plumbing/format/packfile/packfile_iter.go | Propagates objectFromHeader errors when iterating deltas (avoids masking failures). |
| plumbing/format/packfile/testpack_internal_test.go | Adds internal test helpers to construct minimal packfiles with controlled malformations. |
| plumbing/format/packfile/scanner_test.go | Adds tests for reserved object type rejection and oversized inflation rejection. |
| plumbing/format/packfile/parser_internal_test.go | Adds tests for delta chain depth ceiling and oversized inflation rejection in Parser. |
| plumbing/format/packfile/packfile_iter_test.go | Adds tests for iterator behavior on malformed/missing-base deltas and oversized inflation. |
| plumbing/format/packfile/delta_test.go | Adds a delta round-trip fuzz test to validate delta encode/decode application paths. |
The delta branch of objectIter.next dereferenced the returned object before checking the error, masking failures (and risking a nil dereference) when delta resolution failed. Check err first and surface it so callers see ErrObjectNotFound or ErrMalformedPackfile instead of silently skipping the entry. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
ObjectType.Valid used a range check that accepted type 5, which Git reserves and never assigns. Switch to an explicit enumeration so the scanner surfaces ErrMalformedPackfile on packs that carry the reserved type instead of letting them through. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
headerFromOffset treated a failed Scan as ErrObjectNotFound, hiding the underlying scanner error (e.g. ErrMalformedPackfile, I/O errors) from callers. Check scanner.Error() first so callers see the real reason instead of a generic missing-object error. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
A pack with a deeply chained delta sequence forced unbounded recursion during parse, letting a malformed input drive arbitrary stack growth. Walk the parent chain in processDelta and reject anything beyond 4095 links — the same ceiling upstream Git enforces via OE_DEPTH_BITS in pack-objects.h — surfacing ErrMalformedPackfile. Move the packfile_iter helpers into internal_test.go alongside the new TestParserRejectsDeepDeltaChain coverage and drop the inflated-size test that duplicates scanner_test.go's ErrInflatedSizeMismatch coverage. Assisted-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Paulo Gomes <paulo@entire.io>
hiddeco
approved these changes
May 18, 2026
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
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.
This PR hardens packfile parsing and iteration against malformed or adversarial pack data.
objectFromHeadererrors from packfile iteration so delta resolution failures are returned to callers instead of being masked or risking nil dereferences.5explicitly inObjectType.Valid, causing malformed pack entries to fail withErrMalformedPackfile.4095before resolving delta content.