Skip to content

Commit bacbc33

Browse files
archive/zip: prevent preallocation check from overflowing
If the indicated directory size in the archive header is so large that subtracting it from the archive size overflows a uint64, the check that the indicated number of files in the archive can be effectively bypassed. Prevent this from happening by checking that the indicated directory size is less than the size of the archive. Thanks to the OSS-Fuzz project for discovering this issue and to Emmanuel Odeke for reporting it. Fixes #47801 Fixes CVE-2021-39293 Change-Id: Ifade26b98a40f3b37398ca86bd5252d12394dd24 Reviewed-on: https://go-review.googlesource.com/c/go/+/343434 Trust: Roland Shoemaker <[email protected]> Run-TryBot: Roland Shoemaker <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Russ Cox <[email protected]>
1 parent 7007431 commit bacbc33

File tree

2 files changed

+19
-1
lines changed

2 files changed

+19
-1
lines changed

src/archive/zip/reader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (z *Reader) init(r io.ReaderAt, size int64) error {
102102
// indicate it contains up to 1 << 128 - 1 files. Since each file has a
103103
// header which will be _at least_ 30 bytes we can safely preallocate
104104
// if (data size / 30) >= end.directoryRecords.
105-
if (uint64(size)-end.directorySize)/30 >= end.directoryRecords {
105+
if end.directorySize < uint64(size) && (uint64(size)-end.directorySize)/30 >= end.directoryRecords {
106106
z.File = make([]*File, 0, end.directoryRecords)
107107
}
108108
z.Comment = end.comment

src/archive/zip/reader_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,3 +1384,21 @@ func TestCVE202133196(t *testing.T) {
13841384
t.Errorf("Archive has unexpected number of files, got %d, want 5", len(r.File))
13851385
}
13861386
}
1387+
1388+
func TestCVE202139293(t *testing.T) {
1389+
// directory size is so large, that the check in Reader.init
1390+
// overflows when subtracting from the archive size, causing
1391+
// the pre-allocation check to be bypassed.
1392+
data := []byte{
1393+
0x50, 0x4b, 0x06, 0x06, 0x05, 0x06, 0x31, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x4b,
1394+
0x06, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
1395+
0x00, 0x00, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x4b,
1396+
0x06, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
1397+
0x00, 0x00, 0x00, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x31, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
1398+
0xff, 0x50, 0xfe, 0x00, 0xff, 0x00, 0x3a, 0x00, 0x00, 0x00, 0xff,
1399+
}
1400+
_, err := NewReader(bytes.NewReader(data), int64(len(data)))
1401+
if err != ErrFormat {
1402+
t.Fatalf("unexpected error, got: %v, want: %v", err, ErrFormat)
1403+
}
1404+
}

0 commit comments

Comments
 (0)