Visitar URL original
plumbing: format/packfile, bound inflate size · go-git/go-git@1f38e17 · GitHub
Skip to content

Commit 1f38e17

Browse files
hiddecohiddeco
committed
plumbing: format/packfile, bound inflate size
A packfile object header carries the object's uncompressed size, and a well-formed packfile never produces more inflated bytes than that value. Enforce this structural invariant at every site that consumes the inflated stream and surface mismatches as `ErrInflatedSizeMismatch`. Both `Scanner.NextObject` and `Scanner.ReadObject` capture the declared length from the most recent `pendingObject` before clearing it, then thread that bound into `copyObject`. The shared `copyObject(w, declaredSize)` wraps its writer in a new `boundedWriter` when a positive size is supplied, so an overrun returns the legal prefix alongside `ErrInflatedSizeMismatch` rather than being silently appended. Callers in `parser.go` and `packfile.go` go through `NextObject`, so they pick up the bound without further changes; for delta entries the declared length is the size of the delta instruction stream, not the resolved result, which matches what these call sites already pass through. The lazy `FSObject.Reader` big-object path returns a new `boundedReadCloser` that wraps the underlying zlib-backed reader. It builds on `io.LimitedReader` with the standard overrun-detection trick — request `limit + 1` bytes and treat the sentinel byte as an explicit signal — so an `io.ReadAll` on an opened object cannot stream past the declared size. The bound is `o.size`, the resolved object size, which matches what the caller is reading regardless of whether the underlying entry is a non-delta or delta-resolved object. The type and its constructor are unexported for now; they can be promoted once a stable use case outside the package emerges. These changes do not reject any conforming pack. Backport of #2119. Assisted-by: Claude Opus 4.7 Signed-off-by: Hidde Beydals <hidde@hhh.computer>
1 parent f7545a0 commit 1f38e17

3 files changed

Lines changed: 365 additions & 10 deletions

File tree

‎plumbing/format/packfile/fsobject.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,13 @@ func (o *FSObject) Reader() (io.ReadCloser, error) {
7878
_ = f.Close()
7979
return nil, err
8080
}
81-
return ioutil.NewReadCloserWithCloser(r, f.Close), nil
81+
// Cap the lazy stream at the resolved object size: well-formed
82+
// content reaches EOF inside the bound, an inflated stream that
83+
// runs past surfaces ErrInflatedSizeMismatch on the byte just
84+
// past the limit. For delta-resolved objects o.size is the
85+
// expanded size, which is what the caller is reading here.
86+
bounded := newBoundedReadCloser(r, o.size)
87+
return ioutil.NewReadCloserWithCloser(bounded, f.Close), nil
8288
}
8389
r, err := p.getObjectContent(o.offset)
8490
if err != nil {

‎plumbing/format/packfile/scanner.go‎

Lines changed: 127 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,96 @@ var (
3333
// fit into its accumulator because the input declares more continuation
3434
// bytes than the type can hold.
3535
ErrLengthOverflow = errors.New("variable-length integer overflow")
36+
// ErrInflatedSizeMismatch is returned when a packfile object inflates to
37+
// more bytes than the size declared in its object header. A well-formed
38+
// packfile never produces more data than the declared size; exceeding it
39+
// indicates a structurally invalid entry.
40+
ErrInflatedSizeMismatch = errors.New("packfile: inflated object exceeds declared size")
3641
)
3742

43+
// boundedWriter passes writes through to w up to limit bytes total, then
44+
// returns ErrInflatedSizeMismatch. It is used to enforce that a packfile
45+
// object's inflated length does not exceed the size declared in its header.
46+
type boundedWriter struct {
47+
w io.Writer
48+
limit int64
49+
n int64
50+
}
51+
52+
// Write forwards p to the underlying writer while keeping the running total
53+
// at or below limit. On overrun it forwards the legal prefix and reports
54+
// the number of bytes actually consumed alongside ErrInflatedSizeMismatch,
55+
// matching the contract in io.Writer. A write error from the underlying
56+
// writer during overrun-handling is joined with ErrInflatedSizeMismatch so
57+
// it is not silently dropped.
58+
func (b *boundedWriter) Write(p []byte) (int, error) {
59+
if b.n+int64(len(p)) > b.limit {
60+
remain := int(b.limit - b.n)
61+
err := error(ErrInflatedSizeMismatch)
62+
if remain > 0 {
63+
n, werr := b.w.Write(p[:remain])
64+
b.n += int64(n)
65+
if werr != nil {
66+
err = errors.Join(ErrInflatedSizeMismatch, werr)
67+
}
68+
return n, err
69+
}
70+
return 0, err
71+
}
72+
n, err := b.w.Write(p)
73+
b.n += int64(n)
74+
return n, err
75+
}
76+
77+
// boundedReadCloser wraps a ReadCloser and reports ErrInflatedSizeMismatch
78+
// once more than limit bytes have been read. It is used by the on-demand
79+
// object reader returned from FSObject.Reader so that a lazy Read of a
80+
// packfile object cannot stream past its declared inflated size.
81+
//
82+
// The implementation builds on io.LimitedReader with the standard
83+
// overrun-detection trick: request limit+1 bytes from the underlying so
84+
// that the moment the sentinel byte materializes (LimitedReader.N drops
85+
// to zero) we know the source produced more than limit bytes.
86+
type boundedReadCloser struct {
87+
lr io.LimitedReader
88+
closer io.Closer
89+
overrun bool
90+
}
91+
92+
// newBoundedReadCloser wraps rc so that the cumulative bytes returned from
93+
// Read never exceed limit. The first call that would have returned a byte
94+
// past limit instead returns ErrInflatedSizeMismatch; subsequent calls
95+
// keep returning the same error. A negative limit is treated as zero, so
96+
// the first byte produced by rc surfaces ErrInflatedSizeMismatch.
97+
func newBoundedReadCloser(rc io.ReadCloser, limit int64) *boundedReadCloser {
98+
if limit < 0 {
99+
limit = 0
100+
}
101+
return &boundedReadCloser{
102+
lr: io.LimitedReader{R: rc, N: limit + 1},
103+
closer: rc,
104+
}
105+
}
106+
107+
// Read forwards Read up to the configured byte limit. When the underlying
108+
// stream produces the limit+1 sentinel byte, the legal prefix is returned
109+
// alongside ErrInflatedSizeMismatch; on subsequent calls only the error
110+
// is returned.
111+
func (b *boundedReadCloser) Read(p []byte) (int, error) {
112+
if b.overrun {
113+
return 0, ErrInflatedSizeMismatch
114+
}
115+
n, err := b.lr.Read(p)
116+
if b.lr.N == 0 {
117+
b.overrun = true
118+
return n - 1, ErrInflatedSizeMismatch
119+
}
120+
return n, err
121+
}
122+
123+
// Close closes the underlying ReadCloser.
124+
func (b *boundedReadCloser) Close() error { return b.closer.Close() }
125+
38126
// ObjectHeader contains the information related to the object, this information
39127
// is collected from the previous bytes to the content of the object.
40128
type ObjectHeader struct {
@@ -333,10 +421,18 @@ func (s *Scanner) readLength(first byte) (int64, error) {
333421
}
334422

335423
// NextObject writes the content of the next object into the reader, returns
336-
// the number of bytes written, the CRC32 of the content and an error, if any
424+
// the number of bytes written, the CRC32 of the content and an error, if any.
425+
//
426+
// When a prior NextObjectHeader has stashed the object header in
427+
// pendingObject, the inflated stream is bounded by the header's declared
428+
// length and surfaces ErrInflatedSizeMismatch on overrun.
337429
func (s *Scanner) NextObject(w io.Writer) (written int64, crc32 uint32, err error) {
430+
declaredSize := int64(-1)
431+
if s.pendingObject != nil {
432+
declaredSize = s.pendingObject.Length
433+
}
338434
s.pendingObject = nil
339-
written, err = s.copyObject(w)
435+
written, err = s.copyObject(w, declaredSize)
340436

341437
s.r.Flush()
342438
crc32 = s.crc.Sum32()
@@ -345,23 +441,39 @@ func (s *Scanner) NextObject(w io.Writer) (written int64, crc32 uint32, err erro
345441
return
346442
}
347443

348-
// ReadObject returns a reader for the object content and an error
444+
// ReadObject returns a reader for the object content and an error.
445+
//
446+
// When a prior NextObjectHeader has stashed the object header in
447+
// pendingObject, the returned reader is bounded by the header's declared
448+
// length so callers cannot stream past the declared inflated size; an
449+
// overrun surfaces ErrInflatedSizeMismatch on the byte just past the
450+
// limit.
349451
func (s *Scanner) ReadObject() (io.ReadCloser, error) {
452+
declaredSize := int64(-1)
453+
if s.pendingObject != nil {
454+
declaredSize = s.pendingObject.Length
455+
}
350456
s.pendingObject = nil
351457
zr, err := sync.GetZlibReader(s.r)
352458
if err != nil {
353459
return nil, fmt.Errorf("zlib reset error: %s", err)
354460
}
355461

356-
return ioutil.NewReadCloserWithCloser(zr.Reader, func() error {
462+
rc := ioutil.NewReadCloserWithCloser(zr.Reader, func() error {
357463
sync.PutZlibReader(zr)
358464
return nil
359-
}), nil
465+
})
466+
if declaredSize >= 0 {
467+
return newBoundedReadCloser(rc, declaredSize), nil
468+
}
469+
return rc, nil
360470
}
361471

362-
// ReadRegularObject reads and write a non-deltified object
363-
// from it zlib stream in an object entry in the packfile.
364-
func (s *Scanner) copyObject(w io.Writer) (n int64, err error) {
472+
// copyObject inflates a non-deltified object's zlib stream into w. When
473+
// declaredSize is non-negative, the write sink is wrapped in a
474+
// boundedWriter so an overrun surfaces ErrInflatedSizeMismatch instead
475+
// of being silently appended.
476+
func (s *Scanner) copyObject(w io.Writer, declaredSize int64) (n int64, err error) {
365477
zr, err := sync.GetZlibReader(s.r)
366478
defer sync.PutZlibReader(zr)
367479

@@ -370,8 +482,14 @@ func (s *Scanner) copyObject(w io.Writer) (n int64, err error) {
370482
}
371483

372484
defer ioutil.CheckClose(zr.Reader, &err)
485+
486+
sink := w
487+
if declaredSize >= 0 {
488+
sink = &boundedWriter{w: w, limit: declaredSize}
489+
}
490+
373491
buf := sync.GetByteSlice()
374-
n, err = io.CopyBuffer(w, zr.Reader, *buf)
492+
n, err = io.CopyBuffer(sink, zr.Reader, *buf)
375493
sync.PutByteSlice(buf)
376494
return
377495
}

0 commit comments

Comments
 (0)