131 lines
5.1 KiB
Diff
131 lines
5.1 KiB
Diff
From c92adf420a3d9a5510f9aea382d826f0c9216a10 Mon Sep 17 00:00:00 2001
|
|
From: Roland Shoemaker <roland@golang.org>
|
|
Date: Tue, 11 May 2021 11:31:31 -0700
|
|
Subject: [PATCH] [release-branch.go1.15] archive/zip: only preallocate File
|
|
slice if reasonably sized
|
|
|
|
Since the number of files in the EOCD record isn't validated, it isn't
|
|
safe to preallocate Reader.Files using that field. A malformed archive
|
|
can indicate it contains up to 1 << 128 - 1 files. We can still safely
|
|
preallocate the slice by checking if the specified number of files in
|
|
the archive is reasonable, given the size of the archive.
|
|
|
|
Thanks to the OSS-Fuzz project for discovering this issue and to
|
|
Emmanuel Odeke for reporting it.
|
|
|
|
Updates #46242
|
|
Fixes #46396
|
|
Fixes CVE-2021-33196
|
|
|
|
Conflict:NA
|
|
Reference:https://github.com/golang/go/commit/c92adf420a3d9a5510f9aea382d826f0c9216a10
|
|
|
|
Change-Id: I3c76d8eec178468b380d87fdb4a3f2cb06f0ee76
|
|
Reviewed-on: https://go-review.googlesource.com/c/go/+/318909
|
|
Trust: Roland Shoemaker <roland@golang.org>
|
|
Trust: Katie Hockman <katie@golang.org>
|
|
Trust: Joe Tsai <thebrokentoaster@gmail.com>
|
|
Run-TryBot: Roland Shoemaker <roland@golang.org>
|
|
TryBot-Result: Go Bot <gobot@golang.org>
|
|
Reviewed-by: Katie Hockman <katie@golang.org>
|
|
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
|
|
(cherry picked from commit 74242baa4136c7a9132a8ccd9881354442788c8c)
|
|
Reviewed-on: https://go-review.googlesource.com/c/go/+/322949
|
|
Reviewed-by: Filippo Valsorda <filippo@golang.org>
|
|
---
|
|
src/archive/zip/reader.go | 10 +++++-
|
|
src/archive/zip/reader_test.go | 59 ++++++++++++++++++++++++++++++++++
|
|
2 files changed, 68 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
|
|
index 13ff9ddcf4..2d5151af3d 100644
|
|
--- a/src/archive/zip/reader.go
|
|
+++ b/src/archive/zip/reader.go
|
|
@@ -84,7 +84,15 @@ func (z *Reader) init(r io.ReaderAt, size int64) error {
|
|
return err
|
|
}
|
|
z.r = r
|
|
- z.File = make([]*File, 0, end.directoryRecords)
|
|
+ // Since the number of directory records is not validated, it is not
|
|
+ // safe to preallocate z.File without first checking that the specified
|
|
+ // number of files is reasonable, since a malformed archive may
|
|
+ // 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 {
|
|
+ z.File = make([]*File, 0, end.directoryRecords)
|
|
+ }
|
|
z.Comment = end.comment
|
|
rs := io.NewSectionReader(r, 0, size)
|
|
if _, err = rs.Seek(int64(end.directoryOffset), io.SeekStart); err != nil {
|
|
diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
|
|
index adca87a8b3..6f67d2e4a9 100644
|
|
--- a/src/archive/zip/reader_test.go
|
|
+++ b/src/archive/zip/reader_test.go
|
|
@@ -1070,3 +1070,62 @@ func TestIssue12449(t *testing.T) {
|
|
t.Errorf("Error reading the archive: %v", err)
|
|
}
|
|
}
|
|
+
|
|
+func TestCVE202133196(t *testing.T) {
|
|
+ // Archive that indicates it has 1 << 128 -1 files,
|
|
+ // this would previously cause a panic due to attempting
|
|
+ // to allocate a slice with 1 << 128 -1 elements.
|
|
+ data := []byte{
|
|
+ 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x08,
|
|
+ 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x02,
|
|
+ 0x03, 0x62, 0x61, 0x65, 0x03, 0x04, 0x00, 0x00,
|
|
+ 0xff, 0xff, 0x50, 0x4b, 0x07, 0x08, 0xbe, 0x20,
|
|
+ 0x5c, 0x6c, 0x09, 0x00, 0x00, 0x00, 0x03, 0x00,
|
|
+ 0x00, 0x00, 0x50, 0x4b, 0x01, 0x02, 0x14, 0x00,
|
|
+ 0x14, 0x00, 0x08, 0x08, 0x08, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0xbe, 0x20, 0x5c, 0x6c, 0x09, 0x00,
|
|
+ 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x03, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x01, 0x02, 0x03, 0x50, 0x4b, 0x06, 0x06, 0x2c,
|
|
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2d,
|
|
+ 0x00, 0x2d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff,
|
|
+ 0xff, 0xff, 0xff, 0x31, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x3a, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x50, 0x4b, 0x06, 0x07, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x6b, 0x00, 0x00, 0x00, 0x00,
|
|
+ 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x50,
|
|
+ 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0xff,
|
|
+ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
|
|
+ 0xff, 0xff, 0xff, 0x00, 0x00,
|
|
+ }
|
|
+ _, err := NewReader(bytes.NewReader(data), int64(len(data)))
|
|
+ if err != ErrFormat {
|
|
+ t.Fatalf("unexpected error, got: %v, want: %v", err, ErrFormat)
|
|
+ }
|
|
+
|
|
+ // Also check that an archive containing a handful of empty
|
|
+ // files doesn't cause an issue
|
|
+ b := bytes.NewBuffer(nil)
|
|
+ w := NewWriter(b)
|
|
+ for i := 0; i < 5; i++ {
|
|
+ _, err := w.Create("")
|
|
+ if err != nil {
|
|
+ t.Fatalf("Writer.Create failed: %s", err)
|
|
+ }
|
|
+ }
|
|
+ if err := w.Close(); err != nil {
|
|
+ t.Fatalf("Writer.Close failed: %s", err)
|
|
+ }
|
|
+ r, err := NewReader(bytes.NewReader(b.Bytes()), int64(b.Len()))
|
|
+ if err != nil {
|
|
+ t.Fatalf("NewReader failed: %s", err)
|
|
+ }
|
|
+ if len(r.File) != 5 {
|
|
+ t.Errorf("Archive has unexpected number of files, got %d, want 5", len(r.File))
|
|
+ }
|
|
+}
|
|
--
|
|
2.27.0
|
|
|