aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRoland Shoemaker <roland@golang.org>2021-08-18 11:49:29 -0700
committerCherry Mui <cherryyz@google.com>2021-09-01 16:41:15 +0000
commit1dd24caf08985066b309af6bc461780c73e05c35 (patch)
tree2f8810231159e1611706a0408e3b169ff92b9765
parentec5170397c724a8ae440b2bc529f857c86f0e6b1 (diff)
downloadgo-1dd24caf08985066b309af6bc461780c73e05c35.tar.gz
go-1dd24caf08985066b309af6bc461780c73e05c35.zip
[release-branch.go1.17] 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 #47986 Updates #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> (cherry picked from commit bacbc33439b124ffd7392c91a5f5d96eca8c0c0b) Reviewed-on: https://go-review.googlesource.com/c/go/+/345410 Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com> Trust: Cherry Mui <cherryyz@google.com>
-rw-r--r--src/archive/zip/reader.go2
-rw-r--r--src/archive/zip/reader_test.go18
2 files changed, 19 insertions, 1 deletions
diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index 2d53f4c7231..c91a8d00e6c 100644
--- 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 37dafe6c8e7..afb03ace24d 100644
--- 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)
+ }
+}