archive/zip: handle trailing data after the end of directory header

The spec doesn't explicitly say that trailing data is okay, but a lot
of people do this and most unzippers will handle it just fine. In any
case, this makes the package more useful, and led me to make the
directory parsing code marginally more robust.

Fixes #5228.

R=golang-dev, dsymonds
CC=golang-dev
https://golang.org/cl/8504044

diff --git a/src/pkg/archive/zip/reader.go b/src/pkg/archive/zip/reader.go
index c10f29a8369ae0c1606d30e87c22459153050e45..f19cf2d1f1e3c2b0b8e3a9c50acf0634c9dd6402 100644 (file)
--- a/src/pkg/archive/zip/reader.go
+++ b/src/pkg/archive/zip/reader.go
@@ -353,6 +353,11 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, err error)
        if err != nil {
                return nil, err
        }
+
+       // Make sure directoryOffset points to somewhere in our file.
+       if o := int64(d.directoryOffset); o < 0 || o >= size {
+               return nil, ErrFormat
+       }
        return d, nil
 }
 
@@ -407,7 +412,7 @@ func findSignatureInBlock(b []byte) int {
                if b[i] == 'P' && b[i+1] == 'K' && b[i+2] == 0x05 && b[i+3] == 0x06 {
                        // n is length of comment
                        n := int(b[i+directoryEndLen-2]) | int(b[i+directoryEndLen-1])<<8
-                       if n+directoryEndLen+i == len(b) {
+                       if n+directoryEndLen+i <= len(b) {
                                return i
                        }
                }

diff --git a/src/pkg/archive/zip/reader_test.go b/src/pkg/archive/zip/reader_test.go
index cf9c59c4b9231b4e7efc82b86dcb768efa35fd52..833ba28ad5252a62d98eaf46441c4c4319f58d64 100644 (file)
--- a/src/pkg/archive/zip/reader_test.go
+++ b/src/pkg/archive/zip/reader_test.go
@@ -63,6 +63,24 @@ var tests = []ZipTest{
                        },
                },
        },
+       {
+               Name:    "test-trailing-junk.zip",
+               Comment: "This is a zipfile comment.",
+               File: []ZipTestFile{
+                       {
+                               Name:    "test.txt",
+                               Content: []byte("This is a test text file.\n"),
+                               Mtime:   "09-05-10 12:12:02",
+                               Mode:    0644,
+                       },
+                       {
+                               Name:  "gophercolor16x16.png",
+                               File:  "gophercolor16x16.png",
+                               Mtime: "09-05-10 15:52:58",
+                               Mode:  0644,
+                       },
+               },
+       },
        {
                Name:   "r.zip",
                Source: returnRecursiveZip,
@@ -262,7 +280,7 @@ func readTestZip(t *testing.T, zt ZipTest) {
                }
        }
        if err != zt.Error {
-               t.Errorf("error=%v, want %v", err, zt.Error)
+               t.Errorf("%s: error=%v, want %v", zt.Name, err, zt.Error)
                return
        }
 

diff --git a/src/pkg/archive/zip/testdata/test-trailing-junk.zip b/src/pkg/archive/zip/testdata/test-trailing-junk.zip
new file mode 100644 (file)
index 0000000..42281b4
Binary files /dev/null and b/src/pkg/archive/zip/testdata/test-trailing-junk.zip differ