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 <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index 2d53f4c7231653d651b4f3b5bb1a15325b4a33ac..c91a8d00e6c1d488a4386296bb8a20ddaf14e3bb 100644 (file)
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -102,7 +102,7 @@ func (z *Reader) init(r io.ReaderAt, size int64) error {
        // indicate it contains up to 1 << 128 - 1 files. Since each file has a
        // header which will be _at least_ 30 bytes we can safely preallocate
        // if (data size / 30) >= end.directoryRecords.
-       if (uint64(size)-end.directorySize)/30 >= end.directoryRecords {
+       if end.directorySize < uint64(size) && (uint64(size)-end.directorySize)/30 >= end.directoryRecords {
                z.File = make([]*File, 0, end.directoryRecords)
        }
        z.Comment = end.comment

diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
index 37dafe6c8e7c448c8fa28359e07fc2cdbb73560f..afb03ace24d286dee2208d46ee3ce274a55e47f4 100644 (file)
--- a/src/archive/zip/reader_test.go
+++ b/src/archive/zip/reader_test.go
@@ -1384,3 +1384,21 @@ func TestCVE202133196(t *testing.T) {
                t.Errorf("Archive has unexpected number of files, got %d, want 5", len(r.File))
        }
 }
+
+func TestCVE202139293(t *testing.T) {
+       // directory size is so large, that the check in Reader.init
+       // overflows when subtracting from the archive size, causing
+       // the pre-allocation check to be bypassed.
+       data := []byte{
+               0x50, 0x4b, 0x06, 0x06, 0x05, 0x06, 0x31, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x4b,
+               0x06, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+               0x00, 0x00, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x1a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x4b,
+               0x06, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+               0x00, 0x00, 0x00, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x31, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+               0xff, 0x50, 0xfe, 0x00, 0xff, 0x00, 0x3a, 0x00, 0x00, 0x00, 0xff,
+       }
+       _, err := NewReader(bytes.NewReader(data), int64(len(data)))
+       if err != ErrFormat {
+               t.Fatalf("unexpected error, got: %v, want: %v", err, ErrFormat)
+       }
+}