archive/zip: reject overflowing directorySize & directoryOffset

We added a check for incorrect baseOffset in CL 408734, but in doing so
we introduced a panic when directoryOffset overflowed a int64. The zip
spec uses uint64, but since io.SectionReader requires int64 we convert,
and possibly introduce an overflow. If offset < 0 && size-offset < 0,
SectionReader will panic when we attempt to read from it.

Since it's extremely unlikely we're ever going to process a zip file
larger than 1<<63-1 byte, just limit directory size and offset to the
max int64.

Change-Id: I1aaa755cf4da927a6e12ef59f97dfc83a3426d86
Reviewed-on: https://go-review.googlesource.com/c/go/+/488195
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index ae296e6fa7b42dd1a7a5340cf7382f9a2e6bb75d..c0e8d97e4ec57d95b5956c77223c4621dcff03dd 100644 (file)
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -615,6 +615,11 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, baseOffset
                }
        }
 
+       maxInt64 := uint64(1<<63 - 1)
+       if d.directorySize > maxInt64 || d.directoryOffset > maxInt64 {
+               return nil, 0, ErrFormat
+       }
+
        baseOffset = directoryEndOffset - int64(d.directorySize) - int64(d.directoryOffset)
 
        // Make sure directoryOffset points to somewhere in our file.

diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
index f793e01e2bb240dc655ad92d1c1f2160c1ca1de2..a67c33598d1b41a8a96ff12d88a9a079c2463a3d 100644 (file)
--- a/src/archive/zip/reader_test.go
+++ b/src/archive/zip/reader_test.go
@@ -1785,3 +1785,44 @@ func TestCompressedDirectory(t *testing.T) {
                }
        }
 }
+
+func TestBaseOffsetPlusOverflow(t *testing.T) {
+       // directoryOffset > maxInt64 && size-directoryOffset < 0
+       data := []byte{
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0xff, 0xff, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x50, 0x4b, 0x06, 0x06, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+               0x20, 0xff, 0xff, 0x20, 0x00, 0x00, 0x00, 0x00,
+               0x00, 0x00, 0x00, 0x20, 0x08, 0x00, 0x00, 0x00,
+               0x00, 0x00, 0x80, 0x50, 0x4b, 0x06, 0x07, 0x00,
+               0x00, 0x00, 0x00, 0x6b, 0x00, 0x00, 0x00, 0x00,
+               0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x50,
+               0x4b, 0x05, 0x06, 0x20, 0x20, 0x20, 0x20, 0xff,
+               0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+               0xff, 0xff, 0xff, 0x20, 0x00,
+       }
+       defer func() {
+               if r := recover(); r != nil {
+                       t.Fatalf("NewReader panicked: %s", r)
+               }
+       }()
+       // Previously, this would trigger a panic as we attempt to read from
+       // a io.SectionReader which would access a slice at a negative offset
+       // as the section reader offset & size were < 0.
+       NewReader(bytes.NewReader(data), int64(len(data))+1875)
+}