Skip to content

Commit 6b38a32

Browse files
pjbgfpjbgf
authored
Merge pull request #1935 from pjbgf/index-v5
[v5] plumbing: format/index, Improve v4 entry name validation
2 parents adad18d + 3ec0d70 commit 6b38a32

4 files changed

Lines changed: 483 additions & 64 deletions

File tree

‎plumbing/format/index/decoder.go‎

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import (
44
"bufio"
55
"bytes"
66
"errors"
7+
"fmt"
78
"io"
8-
99
"strconv"
1010
"time"
1111

@@ -26,12 +26,14 @@ var (
2626
ErrInvalidChecksum = errors.New("invalid checksum")
2727
// ErrUnknownExtension is returned when an index extension is encountered that is considered mandatory
2828
ErrUnknownExtension = errors.New("unknown extension")
29+
// ErrMalformedIndexFile is returned when the index file contents are
30+
// structurally invalid.
31+
ErrMalformedIndexFile = errors.New("index decoder: malformed index file")
2932
)
3033

3134
const (
3235
entryHeaderLength = 62
3336
entryExtended = 0x4000
34-
entryValid = 0x8000
3537
nameMask = 0xfff
3638
intentToAddMask = 1 << 13
3739
skipWorkTreeMask = 1 << 14
@@ -140,33 +142,55 @@ func (d *Decoder) readEntry(idx *Index) (*Entry, error) {
140142
e.SkipWorktree = extended&skipWorkTreeMask != 0
141143
}
142144

143-
if err := d.readEntryName(idx, e, flags); err != nil {
145+
nameConsumed, err := d.readEntryName(idx, e, flags)
146+
if err != nil {
144147
return nil, err
145148
}
146149

147-
return e, d.padEntry(idx, e, read)
150+
return e, d.padEntry(idx, e, read, nameConsumed)
148151
}
149152

150-
func (d *Decoder) readEntryName(idx *Index, e *Entry, flags uint16) error {
151-
var name string
152-
var err error
153-
153+
// readEntryName reads the entry path and sets e.Name. It returns the
154+
// number of bytes consumed from the stream for the name portion.
155+
func (d *Decoder) readEntryName(idx *Index, e *Entry, flags uint16) (int, error) {
154156
switch idx.Version {
155157
case 2, 3:
156-
len := flags & nameMask
157-
name, err = d.doReadEntryName(len)
158+
nameLen := flags & nameMask
159+
name, consumed, err := d.doReadEntryName(nameLen)
160+
if err != nil {
161+
return 0, err
162+
}
163+
e.Name = name
164+
return consumed, nil
158165
case 4:
159-
name, err = d.doReadEntryNameV4()
166+
name, err := d.doReadEntryNameV4()
167+
if err != nil {
168+
return 0, err
169+
}
170+
e.Name = name
171+
return 0, nil // V4 has no padding; consumed count unused
160172
default:
161-
return ErrUnsupportedVersion
173+
return 0, ErrUnsupportedVersion
162174
}
175+
}
163176

164-
if err != nil {
165-
return err
177+
// doReadEntryName reads the entry path for V2/V3 indexes. It returns the
178+
// name, the number of bytes consumed from the stream, and any error.
179+
// When nameLen equals nameMask (0xFFF), the name was too long to fit in
180+
// the 12-bit field and the real length is found by scanning for the NUL
181+
// terminator — matching C Git's strlen(name) fallback in create_from_disk.
182+
func (d *Decoder) doReadEntryName(nameLen uint16) (string, int, error) {
183+
if nameLen == nameMask {
184+
name, err := binary.ReadUntil(d.r, '\x00')
185+
if err != nil {
186+
return "", 0, err
187+
}
188+
return string(name), len(name) + 1, nil // +1 for the consumed NUL delimiter
166189
}
167190

168-
e.Name = name
169-
return nil
191+
name := make([]byte, nameLen)
192+
_, err := io.ReadFull(d.r, name)
193+
return string(name), int(nameLen), err
170194
}
171195

172196
func (d *Decoder) doReadEntryNameV4() (string, error) {
@@ -177,7 +201,14 @@ func (d *Decoder) doReadEntryNameV4() (string, error) {
177201

178202
var base string
179203
if d.lastEntry != nil {
204+
if l < 0 || int(l) > len(d.lastEntry.Name) {
205+
return "", fmt.Errorf("%w: invalid V4 entry name strip length %d (previous name length: %d)",
206+
ErrMalformedIndexFile, l, len(d.lastEntry.Name))
207+
}
180208
base = d.lastEntry.Name[:len(d.lastEntry.Name)-int(l)]
209+
} else if l > 0 {
210+
return "", fmt.Errorf("%w: non-zero strip length %d on first V4 entry",
211+
ErrMalformedIndexFile, l)
181212
}
182213

183214
name, err := binary.ReadUntil(d.r, '\x00')
@@ -188,24 +219,23 @@ func (d *Decoder) doReadEntryNameV4() (string, error) {
188219
return base + string(name), nil
189220
}
190221

191-
func (d *Decoder) doReadEntryName(len uint16) (string, error) {
192-
name := make([]byte, len)
193-
_, err := io.ReadFull(d.r, name)
194-
195-
return string(name), err
196-
}
197-
198-
// Index entries are padded out to the next 8 byte alignment
199-
// for historical reasons related to how C Git read the files.
200-
func (d *Decoder) padEntry(idx *Index, e *Entry, read int) error {
222+
// padEntry discards NUL padding bytes that follow each V2/V3 entry on
223+
// disk. nameConsumed is the number of stream bytes consumed while reading
224+
// the entry name (which may exceed len(e.Name) when a NUL terminator was
225+
// consumed for long names where the 12-bit length field overflowed).
226+
func (d *Decoder) padEntry(idx *Index, e *Entry, read, nameConsumed int) error {
201227
if idx.Version == 4 {
202228
return nil
203229
}
204230

205231
entrySize := read + len(e.Name)
206232
padLen := 8 - entrySize%8
207-
_, err := io.CopyN(io.Discard, d.r, int64(padLen))
208-
return err
233+
padLen -= nameConsumed - len(e.Name)
234+
if padLen > 0 {
235+
_, err := io.CopyN(io.Discard, d.r, int64(padLen))
236+
return err
237+
}
238+
return nil
209239
}
210240

211241
func (d *Decoder) readExtensions(idx *Index) error {
@@ -312,7 +342,7 @@ func (d *Decoder) readChecksum(expected []byte) error {
312342
}
313343

314344
func validateHeader(r io.Reader) (version uint32, err error) {
315-
var s = make([]byte, 4)
345+
s := make([]byte, 4)
316346
if _, err := io.ReadFull(r, s); err != nil {
317347
return 0, err
318348
}
@@ -376,24 +406,26 @@ func (d *treeExtensionDecoder) readEntry() (*TreeEntry, error) {
376406
return nil, err
377407
}
378408

379-
// An entry can be in an invalidated state and is represented by having a
380-
// negative number in the entry_count field.
381-
if i == -1 {
382-
return nil, nil
383-
}
384-
385409
e.Entries = i
386410
trees, err := binary.ReadUntil(d.r, '\n')
387411
if err != nil {
388412
return nil, err
389413
}
390414

391-
i, err = strconv.Atoi(string(trees))
415+
subtrees, err := strconv.Atoi(string(trees))
392416
if err != nil {
393417
return nil, err
394418
}
395419

396-
e.Trees = i
420+
e.Trees = subtrees
421+
422+
// An entry can be in an invalidated state and is represented by having a
423+
// negative number in the entry_count field. In this case, there is no
424+
// object name and the next entry starts immediately after the newline.
425+
if i < 0 {
426+
return nil, nil
427+
}
428+
397429
_, err = io.ReadFull(d.r, e.Hash[:])
398430
if err != nil {
399431
return nil, err

0 commit comments

Comments
 (0)