From 51d9acbb7cd873a7898a93f713b75c33db3942ce Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Mon, 27 Jun 2022 09:55:55 +0200 Subject: [PATCH 1/2] zip: Merge upstream Add Go 1.19 improvements. --- zip/fuzz_test.go | 84 +++++++++++ zip/reader.go | 122 +++++++++++---- zip/reader_test.go | 299 +++++++++++++++++++++++++++++++++++-- zip/register.go | 3 +- zip/testdata/readme.notzip | Bin 1906 -> 1906 bytes zip/writer_test.go | 45 +++--- zip/zip_test.go | 5 +- 7 files changed, 494 insertions(+), 64 deletions(-) create mode 100644 zip/fuzz_test.go diff --git a/zip/fuzz_test.go b/zip/fuzz_test.go new file mode 100644 index 0000000000..760d5e5664 --- /dev/null +++ b/zip/fuzz_test.go @@ -0,0 +1,84 @@ +//go:build go1.18 +// +build go1.18 + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package zip + +import ( + "bytes" + "io" + "os" + "path/filepath" + "testing" +) + +func FuzzReader(f *testing.F) { + testdata, err := os.ReadDir("testdata") + if err != nil { + f.Fatalf("failed to read testdata directory: %s", err) + } + for _, de := range testdata { + if de.IsDir() { + continue + } + b, err := os.ReadFile(filepath.Join("testdata", de.Name())) + if err != nil { + f.Fatalf("failed to read testdata: %s", err) + } + f.Add(b) + } + + f.Fuzz(func(t *testing.T, b []byte) { + r, err := NewReader(bytes.NewReader(b), int64(len(b))) + if err != nil { + return + } + + type file struct { + header *FileHeader + content []byte + } + files := []file{} + + for _, f := range r.File { + fr, err := f.Open() + if err != nil { + continue + } + content, err := io.ReadAll(fr) + if err != nil { + continue + } + files = append(files, file{header: &f.FileHeader, content: content}) + if _, err := r.Open(f.Name); err != nil { + continue + } + } + + // If we were unable to read anything out of the archive don't + // bother trying to roundtrip it. + if len(files) == 0 { + return + } + + w := NewWriter(io.Discard) + for _, f := range files { + ww, err := w.CreateHeader(f.header) + if err != nil { + t.Fatalf("unable to write previously parsed header: %s", err) + } + if _, err := ww.Write(f.content); err != nil { + t.Fatalf("unable to write previously parsed content: %s", err) + } + } + + if err := w.Close(); err != nil { + t.Fatalf("Unable to write archive: %s", err) + } + + // TODO: We may want to check if the archive roundtrips. + }) +} diff --git a/zip/reader.go b/zip/reader.go index 4d0d5e09a7..65aab5bf20 100644 --- a/zip/reader.go +++ b/zip/reader.go @@ -33,6 +33,10 @@ type Reader struct { Comment string decompressors map[uint16]Decompressor + // Some JAR files are zip files with a prefix that is a bash script. + // The baseOffset field is the start of the zip file proper. + baseOffset int64 + // fileList is a list of files sorted by ename, // for use by the Open method. fileListOnce sync.Once @@ -52,9 +56,8 @@ type File struct { FileHeader zip *Reader zipr io.ReaderAt - headerOffset int64 + headerOffset int64 // includes overall ZIP archive baseOffset zip64 bool // zip64 extended information extra field presence - descErr error // error reading the data descriptor during init } // OpenReader will open the Zip file specified by name and return a ReadCloser. @@ -91,23 +94,24 @@ func NewReader(r io.ReaderAt, size int64) (*Reader, error) { } func (z *Reader) init(r io.ReaderAt, size int64) error { - end, err := readDirectoryEnd(r, size) + end, baseOffset, err := readDirectoryEnd(r, size) if err != nil { return err } z.r = r + z.baseOffset = baseOffset // 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 { + if end.directorySize < uint64(size) && (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 { + if _, err = rs.Seek(z.baseOffset+int64(end.directoryOffset), io.SeekStart); err != nil { return err } buf := bufio.NewReader(rs) @@ -119,12 +123,27 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { for { f := &File{zip: z, zipr: r} err = readDirectoryHeader(f, buf) + + // For compatibility with other zip programs, + // if we have a non-zero base offset and can't read + // the first directory header, try again with a zero + // base offset. + if err == ErrFormat && z.baseOffset != 0 && len(z.File) == 0 { + z.baseOffset = 0 + if _, err = rs.Seek(int64(end.directoryOffset), io.SeekStart); err != nil { + return err + } + buf.Reset(rs) + continue + } + if err == ErrFormat || err == io.ErrUnexpectedEOF { break } if err != nil { return err } + f.headerOffset += z.baseOffset z.File = append(z.File, f) } if uint16(len(z.File)) != uint16(end.directoryRecords) { // only compare 16 bits here @@ -229,6 +248,9 @@ func (r *checksumReader) Read(b []byte) (n int, err error) { n, err = r.rc.Read(b) r.hash.Write(b[:n]) r.nread += uint64(n) + if r.nread > r.f.UncompressedSize64 { + return 0, ErrFormat + } if err == nil { return } @@ -492,7 +514,7 @@ func readDataDescriptor(r io.Reader, f *File) error { return nil } -func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, err error) { +func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, baseOffset int64, err error) { // look for directoryEndSignature in the last 1k, then in the last 65k var buf []byte var directoryEndOffset int64 @@ -502,7 +524,7 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, err error) } buf = make([]byte, int(bLen)) if _, err := r.ReadAt(buf, size-bLen); err != nil && err != io.EOF { - return nil, err + return nil, 0, err } if p := findSignatureInBlock(buf); p >= 0 { buf = buf[p:] @@ -510,7 +532,7 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, err error) break } if i == 1 || bLen == size { - return nil, ErrFormat + return nil, 0, ErrFormat } } @@ -527,7 +549,7 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, err error) } l := int(d.commentLen) if l > len(b) { - return nil, errors.New("zip: invalid comment length") + return nil, 0, errors.New("zip: invalid comment length") } d.comment = string(b[:l]) @@ -535,17 +557,21 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, err error) if d.directoryRecords == 0xffff || d.directorySize == 0xffff || d.directoryOffset == 0xffffffff { p, err := findDirectory64End(r, directoryEndOffset) if err == nil && p >= 0 { + directoryEndOffset = p err = readDirectory64End(r, p, d) } if err != nil { - return nil, err + return nil, 0, err } } + + baseOffset = directoryEndOffset - int64(d.directorySize) - int64(d.directoryOffset) + // Make sure directoryOffset points to somewhere in our file. - if o := int64(d.directoryOffset); o < 0 || o >= size { - return nil, ErrFormat + if o := baseOffset + int64(d.directoryOffset); o < 0 || o >= size { + return nil, 0, ErrFormat } - return d, nil + return d, baseOffset, nil } // findDirectory64End tries to read the zip64 locator just before the @@ -650,6 +676,7 @@ type fileListEntry struct { name string file *File isDir bool + isDup bool } type fileInfoDirEntry interface { @@ -657,11 +684,14 @@ type fileInfoDirEntry interface { fs.DirEntry } -func (e *fileListEntry) stat() fileInfoDirEntry { +func (e *fileListEntry) stat() (fileInfoDirEntry, error) { + if e.isDup { + return nil, errors.New(e.name + ": duplicate entries in zip file") + } if !e.isDir { - return headerFileInfo{&e.file.FileHeader} + return headerFileInfo{&e.file.FileHeader}, nil } - return e + return e, nil } // Only used for directories. @@ -696,14 +726,37 @@ func toValidName(name string) string { func (r *Reader) initFileList() { r.fileListOnce.Do(func() { + // files and knownDirs map from a file/directory name + // to an index into the r.fileList entry that we are + // building. They are used to mark duplicate entries. + files := make(map[string]int) + knownDirs := make(map[string]int) + + // dirs[name] is true if name is known to be a directory, + // because it appears as a prefix in a path. dirs := make(map[string]bool) - knownDirs := make(map[string]bool) + for _, file := range r.File { isDir := len(file.Name) > 0 && file.Name[len(file.Name)-1] == '/' name := toValidName(file.Name) + if name == "" { + continue + } + + if idx, ok := files[name]; ok { + r.fileList[idx].isDup = true + continue + } + if idx, ok := knownDirs[name]; ok { + r.fileList[idx].isDup = true + continue + } + for dir := path.Dir(name); dir != "."; dir = path.Dir(dir) { dirs[dir] = true } + + idx := len(r.fileList) entry := fileListEntry{ name: name, file: file, @@ -711,17 +764,23 @@ func (r *Reader) initFileList() { } r.fileList = append(r.fileList, entry) if isDir { - knownDirs[name] = true + knownDirs[name] = idx + } else { + files[name] = idx } } for dir := range dirs { - if !knownDirs[dir] { - entry := fileListEntry{ - name: dir, - file: nil, - isDir: true, + if _, ok := knownDirs[dir]; !ok { + if idx, ok := files[dir]; ok { + r.fileList[idx].isDup = true + } else { + entry := fileListEntry{ + name: dir, + file: nil, + isDir: true, + } + r.fileList = append(r.fileList, entry) } - r.fileList = append(r.fileList, entry) } } @@ -740,12 +799,11 @@ func fileEntryLess(x, y string) bool { // paths are always slash separated, with no // leading / or ../ elements. func (r *Reader) Open(name string) (fs.File, error) { + r.initFileList() + if !fs.ValidPath(name) { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} } - - r.initFileList() - e := r.openLookup(name) if e == nil { return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} @@ -761,7 +819,7 @@ func (r *Reader) Open(name string) (fs.File, error) { } func split(name string) (dir, elem string, isDir bool) { - if name[len(name)-1] == '/' { + if len(name) > 0 && name[len(name)-1] == '/' { isDir = true name = name[:len(name)-1] } @@ -817,7 +875,7 @@ type openDir struct { } func (d *openDir) Close() error { return nil } -func (d *openDir) Stat() (fs.FileInfo, error) { return d.e.stat(), nil } +func (d *openDir) Stat() (fs.FileInfo, error) { return d.e.stat() } func (d *openDir) Read([]byte) (int, error) { return 0, &fs.PathError{Op: "read", Path: d.e.name, Err: errors.New("is a directory")} @@ -836,7 +894,11 @@ func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { } list := make([]fs.DirEntry, n) for i := range list { - list[i] = d.files[d.offset+i].stat() + s, err := d.files[d.offset+i].stat() + if err != nil { + return nil, err + } + list[i] = s } d.offset += n return list, nil diff --git a/zip/reader_test.go b/zip/reader_test.go index 8c3654ede1..a08a2e8a2d 100644 --- a/zip/reader_test.go +++ b/zip/reader_test.go @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.16 -// +build go1.16 - package zip import ( @@ -15,6 +12,7 @@ import ( "io/fs" "os" "path/filepath" + "reflect" "regexp" "strings" "testing" @@ -93,6 +91,42 @@ var tests = []ZipTest{ }, }, }, + { + Name: "test-prefix.zip", + Comment: "This is a zipfile comment.", + File: []ZipTestFile{ + { + Name: "test.txt", + Content: []byte("This is a test text file.\n"), + Modified: time.Date(2010, 9, 5, 12, 12, 1, 0, timeZone(+10*time.Hour)), + Mode: 0644, + }, + { + Name: "gophercolor16x16.png", + File: "gophercolor16x16.png", + Modified: time.Date(2010, 9, 5, 15, 52, 58, 0, timeZone(+10*time.Hour)), + Mode: 0644, + }, + }, + }, + { + Name: "test-baddirsz.zip", + Comment: "This is a zipfile comment.", + File: []ZipTestFile{ + { + Name: "test.txt", + Content: []byte("This is a test text file.\n"), + Modified: time.Date(2010, 9, 5, 12, 12, 1, 0, timeZone(+10*time.Hour)), + Mode: 0644, + }, + { + Name: "gophercolor16x16.png", + File: "gophercolor16x16.png", + Modified: time.Date(2010, 9, 5, 15, 52, 58, 0, timeZone(+10*time.Hour)), + Mode: 0644, + }, + }, + }, { Name: "r.zip", Source: returnRecursiveZip, @@ -490,6 +524,35 @@ var tests = []ZipTest{ }, }, }, + { + Name: "dupdir.zip", + File: []ZipTestFile{ + { + Name: "a/", + Content: []byte{}, + Mode: fs.ModeDir | 0666, + Modified: time.Date(2021, 12, 29, 0, 0, 0, 0, timeZone(0)), + }, + { + Name: "a/b", + Content: []byte{}, + Mode: 0666, + Modified: time.Date(2021, 12, 29, 0, 0, 0, 0, timeZone(0)), + }, + { + Name: "a/b/", + Content: []byte{}, + Mode: fs.ModeDir | 0666, + Modified: time.Date(2021, 12, 29, 0, 0, 0, 0, timeZone(0)), + }, + { + Name: "a/b/c", + Content: []byte{}, + Mode: 0666, + Modified: time.Date(2021, 12, 29, 0, 0, 0, 0, timeZone(0)), + }, + }, + }, } func TestReader(t *testing.T) { @@ -868,7 +931,6 @@ func returnRecursiveZip() (r io.ReaderAt, size int64) { // // It's here in hex for the same reason as rZipBytes above: to avoid // problems with on-disk virus scanners or other zip processors. -// func biggestZipBytes() []byte { s := ` 0000000 50 4b 03 04 14 00 08 00 08 00 00 00 00 00 00 00 @@ -1015,7 +1077,7 @@ func TestIssue10957(t *testing.T) { "\x00\x00\x00\x00\x0000000000\x00\x00\x00\x00000" + "00000000PK\x01\x0200000000" + "0000000000000000\v\x00\x00\x00" + - "\x00\x0000PK\x05\x06000000\x05\x000000" + + "\x00\x0000PK\x05\x06000000\x05\x00\xfd\x00\x00\x00" + "\v\x00\x00\x00\x00\x00") z, err := NewReader(bytes.NewReader(data), int64(len(data))) if err != nil { @@ -1060,7 +1122,7 @@ func TestIssue11146(t *testing.T) { "0000000000000000PK\x01\x02" + "0000\b0\b\x00000000000000" + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x000000PK\x05\x06\x00\x00" + - "\x00\x0000\x01\x0000008\x00\x00\x00\x00\x00") + "\x00\x0000\x01\x00\x26\x00\x00\x008\x00\x00\x00\x00\x00") z, err := NewReader(bytes.NewReader(data), int64(len(data))) if err != nil { t.Fatal(err) @@ -1127,6 +1189,7 @@ func TestFS(t *testing.T) { []string{"a/b/c"}, }, } { + test := test t.Run(test.file, func(t *testing.T) { t.Parallel() z, err := OpenReader(test.file) @@ -1141,6 +1204,60 @@ func TestFS(t *testing.T) { } } +func TestFSWalk(t *testing.T) { + for _, test := range []struct { + file string + want []string + wantErr bool + }{ + { + file: "testdata/unix.zip", + want: []string{".", "dir", "dir/bar", "dir/empty", "hello", "readonly"}, + }, + { + file: "testdata/subdir.zip", + want: []string{".", "a", "a/b", "a/b/c"}, + }, + { + file: "testdata/dupdir.zip", + wantErr: true, + }, + } { + test := test + t.Run(test.file, func(t *testing.T) { + t.Parallel() + z, err := OpenReader(test.file) + if err != nil { + t.Fatal(err) + } + var files []string + sawErr := false + err = fs.WalkDir(z, ".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + if !test.wantErr { + t.Errorf("%s: %v", path, err) + } + sawErr = true + return nil + } + files = append(files, path) + return nil + }) + if err != nil { + t.Errorf("fs.WalkDir error: %v", err) + } + if test.wantErr && !sawErr { + t.Error("succeeded but want error") + } else if !test.wantErr && sawErr { + t.Error("unexpected error") + } + if test.want != nil && !reflect.DeepEqual(files, test.want) { + t.Errorf("got %v want %v", files, test.want) + } + }) + } +} + func TestFSModTime(t *testing.T) { t.Parallel() z, err := OpenReader("testdata/subdir.zip") @@ -1206,6 +1323,15 @@ func TestCVE202127919(t *testing.T) { if err != nil { t.Errorf("Error reading file: %v", err) } + if len(r.File) != 1 { + t.Fatalf("No entries in the file list") + } + if r.File[0].Name != "../test.txt" { + t.Errorf("Unexpected entry name: %s", r.File[0].Name) + } + if _, err := r.File[0].Open(); err != nil { + t.Errorf("Error opening file: %v", err) + } } func TestCVE202133196(t *testing.T) { @@ -1267,16 +1393,165 @@ func TestCVE202133196(t *testing.T) { } } -func TestIssue48085(t *testing.T) { - z, err := OpenReader("testdata/unix.zip") +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) + } +} + +func TestCVE202141772(t *testing.T) { + // Archive contains a file whose name is exclusively made up of '/', '\' + // characters, or "../", "..\" paths, which would previously cause a panic. + // + // Length Method Size Cmpr Date Time CRC-32 Name + // -------- ------ ------- ---- ---------- ----- -------- ---- + // 0 Stored 0 0% 08-05-2021 18:32 00000000 / + // 0 Stored 0 0% 09-14-2021 12:59 00000000 // + // 0 Stored 0 0% 09-14-2021 12:59 00000000 \ + // 11 Stored 11 0% 09-14-2021 13:04 0d4a1185 /test.txt + // -------- ------- --- ------- + // 11 11 0% 4 files + data := []byte{ + 0x50, 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x08, + 0x00, 0x00, 0x06, 0x94, 0x05, 0x53, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x2f, 0x50, + 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x78, 0x67, 0x2e, 0x53, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x02, 0x00, 0x00, 0x00, 0x2f, 0x2f, 0x50, + 0x4b, 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x78, 0x67, 0x2e, 0x53, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x5c, 0x50, 0x4b, + 0x03, 0x04, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x91, 0x68, 0x2e, 0x53, 0x85, 0x11, 0x4a, 0x0d, + 0x0b, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x00, 0x00, + 0x09, 0x00, 0x00, 0x00, 0x2f, 0x74, 0x65, 0x73, + 0x74, 0x2e, 0x74, 0x78, 0x74, 0x68, 0x65, 0x6c, + 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64, + 0x50, 0x4b, 0x01, 0x02, 0x14, 0x03, 0x0a, 0x00, + 0x00, 0x08, 0x00, 0x00, 0x06, 0x94, 0x05, 0x53, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, + 0xed, 0x41, 0x00, 0x00, 0x00, 0x00, 0x2f, 0x50, + 0x4b, 0x01, 0x02, 0x3f, 0x00, 0x0a, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x78, 0x67, 0x2e, 0x53, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x02, 0x00, 0x24, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, + 0x00, 0x1f, 0x00, 0x00, 0x00, 0x2f, 0x2f, 0x0a, + 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x18, 0x00, 0x93, 0x98, 0x25, 0x57, 0x25, + 0xa9, 0xd7, 0x01, 0x93, 0x98, 0x25, 0x57, 0x25, + 0xa9, 0xd7, 0x01, 0x93, 0x98, 0x25, 0x57, 0x25, + 0xa9, 0xd7, 0x01, 0x50, 0x4b, 0x01, 0x02, 0x3f, + 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x78, + 0x67, 0x2e, 0x53, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x20, 0x00, 0x00, 0x00, 0x3f, 0x00, 0x00, + 0x00, 0x5c, 0x0a, 0x00, 0x20, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x18, 0x00, 0x93, 0x98, + 0x25, 0x57, 0x25, 0xa9, 0xd7, 0x01, 0x93, 0x98, + 0x25, 0x57, 0x25, 0xa9, 0xd7, 0x01, 0x93, 0x98, + 0x25, 0x57, 0x25, 0xa9, 0xd7, 0x01, 0x50, 0x4b, + 0x01, 0x02, 0x3f, 0x00, 0x0a, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x91, 0x68, 0x2e, 0x53, 0x85, 0x11, + 0x4a, 0x0d, 0x0b, 0x00, 0x00, 0x00, 0x0b, 0x00, + 0x00, 0x00, 0x09, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, + 0x5e, 0x00, 0x00, 0x00, 0x2f, 0x74, 0x65, 0x73, + 0x74, 0x2e, 0x74, 0x78, 0x74, 0x0a, 0x00, 0x20, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x18, + 0x00, 0xa9, 0x80, 0x51, 0x01, 0x26, 0xa9, 0xd7, + 0x01, 0x31, 0xd1, 0x57, 0x01, 0x26, 0xa9, 0xd7, + 0x01, 0xdf, 0x48, 0x85, 0xf9, 0x25, 0xa9, 0xd7, + 0x01, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, + 0x00, 0x04, 0x00, 0x04, 0x00, 0x31, 0x01, 0x00, + 0x00, 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, + } + r, err := NewReader(bytes.NewReader([]byte(data)), int64(len(data))) if err != nil { - t.Fatal(err) + t.Fatalf("Error reading the archive: %v", err) + } + entryNames := []string{`/`, `//`, `\`, `/test.txt`} + var names []string + for _, f := range r.File { + names = append(names, f.Name) + if _, err := f.Open(); err != nil { + t.Errorf("Error opening %q: %v", f.Name, err) + } + if _, err := r.Open(f.Name); err == nil { + t.Errorf("Opening %q with fs.FS API succeeded", f.Name) + } + } + if !reflect.DeepEqual(names, entryNames) { + t.Errorf("Unexpected file entries: %q", names) + } + if _, err := r.Open(""); err == nil { + t.Errorf("Opening %q with fs.FS API succeeded", "") + } + if _, err := r.Open("test.txt"); err != nil { + t.Errorf("Error opening %q with fs.FS API: %v", "test.txt", err) + } + dirEntries, err := fs.ReadDir(r, ".") + if err != nil { + t.Fatalf("Error reading the root directory: %v", err) + } + if len(dirEntries) != 1 || dirEntries[0].Name() != "test.txt" { + t.Errorf("Unexpected directory entries") + for _, dirEntry := range dirEntries { + _, err := r.Open(dirEntry.Name()) + t.Logf("%q (Open error: %v)", dirEntry.Name(), err) + } + t.FailNow() } + info, err := dirEntries[0].Info() + if err != nil { + t.Fatalf("Error reading info entry: %v", err) + } + if name := info.Name(); name != "test.txt" { + t.Errorf("Inconsistent name in info entry: %v", name) + } +} +func TestUnderSize(t *testing.T) { + z, err := OpenReader("testdata/readme.zip") + if err != nil { + t.Fatal(err) + } defer z.Close() - f, err := z.Open("") - if f != nil || err == nil || !strings.Contains(err.Error(), "invalid argument") { - t.Fatalf("expecting open failed with invalid argument, got:%s %q", err, f) + for _, f := range z.File { + f.UncompressedSize64 = 1 + } + + for _, f := range z.File { + t.Run(f.Name, func(t *testing.T) { + rd, err := f.Open() + if err != nil { + t.Fatal(err) + } + defer rd.Close() + + _, err = io.Copy(io.Discard, rd) + if err != ErrFormat { + t.Fatalf("Error mismatch\n\tGot: %v\n\tWant: %v", err, ErrFormat) + } + }) } } diff --git a/zip/register.go b/zip/register.go index 3bcb1ffd7a..ca8c13ce92 100644 --- a/zip/register.go +++ b/zip/register.go @@ -7,7 +7,6 @@ package zip import ( "errors" "io" - "io/ioutil" "sync" "github.com/klauspost/compress/flate" @@ -112,7 +111,7 @@ func init() { compressors.Store(Store, Compressor(func(w io.Writer) (io.WriteCloser, error) { return &nopCloser{w}, nil })) compressors.Store(Deflate, Compressor(func(w io.Writer) (io.WriteCloser, error) { return newFlateWriter(w), nil })) - decompressors.Store(Store, Decompressor(ioutil.NopCloser)) + decompressors.Store(Store, Decompressor(io.NopCloser)) decompressors.Store(Deflate, Decompressor(newFlateReader)) } diff --git a/zip/testdata/readme.notzip b/zip/testdata/readme.notzip index 81737275c6ebf5ea69b992753ab4050f031f31b8..79b1cb6de33c6ae86451acedbd50df4207a5710e 100644 GIT binary patch delta 15 Wcmeyw_la-AHzp?UfXzRcs#pLyiw1E3 delta 15 Wcmeyw_la-AHzuY4@6A7$s#pLykOpx8 diff --git a/zip/writer_test.go b/zip/writer_test.go index e0b549deee..eb3109bf3e 100644 --- a/zip/writer_test.go +++ b/zip/writer_test.go @@ -11,10 +11,9 @@ import ( "fmt" "hash/crc32" "io" - "io/ioutil" + "io/fs" "math/rand" "os" - "runtime" "strings" "testing" "time" @@ -26,7 +25,7 @@ type WriteTest struct { Name string Data []byte Method uint16 - Mode os.FileMode + Mode fs.FileMode } var writeTests = []WriteTest{ @@ -46,19 +45,31 @@ var writeTests = []WriteTest{ Name: "setuid", Data: []byte("setuid file"), Method: Deflate, - Mode: 0755 | os.ModeSetuid, + Mode: 0755 | fs.ModeSetuid, }, { Name: "setgid", Data: []byte("setgid file"), Method: Deflate, - Mode: 0755 | os.ModeSetgid, + Mode: 0755 | fs.ModeSetgid, }, { Name: "symlink", Data: []byte("../link/target"), Method: Deflate, - Mode: 0755 | os.ModeSymlink, + Mode: 0755 | fs.ModeSymlink, + }, + { + Name: "device", + Data: []byte("device file"), + Method: Deflate, + Mode: 0755 | fs.ModeDevice, + }, + { + Name: "chardevice", + Data: []byte("char device file"), + Method: Deflate, + Mode: 0755 | fs.ModeDevice | fs.ModeCharDevice, }, } @@ -240,7 +251,7 @@ func TestWriterTime(t *testing.T) { t.Fatalf("unexpected Close error: %v", err) } - want, err := ioutil.ReadFile("testdata/time-go.zip") + want, err := os.ReadFile("testdata/time-go.zip") if err != nil { t.Fatalf("unexpected ReadFile error: %v", err) } @@ -304,7 +315,7 @@ func TestWriterFlush(t *testing.T) { } func TestWriterDir(t *testing.T) { - w := NewWriter(ioutil.Discard) + w := NewWriter(io.Discard) dw, err := w.Create("dir/") if err != nil { t.Fatal(err) @@ -357,7 +368,7 @@ func TestWriterDirAttributes(t *testing.T) { } func TestWriterCopy(t *testing.T) { - want, err := ioutil.ReadFile("testdata/test.zip") + want, err := os.ReadFile("testdata/test.zip") if err != nil { t.Fatalf("unexpected ReadFile error: %v", err) } @@ -388,7 +399,7 @@ func TestWriterCopy(t *testing.T) { if err != nil { t.Fatalf("unexpected Open error: %v", err) } - want, err := ioutil.ReadAll(wantR) + want, err := io.ReadAll(wantR) if err != nil { t.Fatalf("unexpected Copy error: %v", err) } @@ -398,7 +409,7 @@ func TestWriterCopy(t *testing.T) { if err != nil { t.Fatalf("unexpected Open error: %v", err) } - got, err := ioutil.ReadAll(gotR) + got, err := io.ReadAll(gotR) if err != nil { t.Fatalf("unexpected Copy error: %v", err) } @@ -543,7 +554,7 @@ func TestWriterCreateRaw(t *testing.T) { if got.Method != want.method { t.Errorf("%s: got Method %#x; want %#x", want.name, got.Method, want.method) } - if got.Flags != want.flags && runtime.Version() >= "go1.16" { + if got.Flags != want.flags { t.Errorf("%s: got Flags %#x; want %#x", want.name, got.Flags, want.flags) } if got.CRC32 != want.crc32 { @@ -562,7 +573,7 @@ func TestWriterCreateRaw(t *testing.T) { continue } - buf, err := ioutil.ReadAll(r) + buf, err := io.ReadAll(r) if err != nil { t.Errorf("%s: ReadAll err = %v", got.Name, err) continue @@ -599,15 +610,15 @@ func testReadFile(t *testing.T, f *File, wt *WriteTest) { testFileMode(t, f, wt.Mode) rc, err := f.Open() if err != nil { - t.Fatal("opening:", err) + t.Fatalf("opening %s: %v", f.Name, err) } - b, err := ioutil.ReadAll(rc) + b, err := io.ReadAll(rc) if err != nil { - t.Fatal("reading:", err) + t.Fatalf("reading %s: %v", f.Name, err) } err = rc.Close() if err != nil { - t.Fatal("closing:", err) + t.Fatalf("closing %s: %v", f.Name, err) } if !bytes.Equal(b, wt.Data) { t.Errorf("File contents %q, want %q", b, wt.Data) diff --git a/zip/zip_test.go b/zip/zip_test.go index a01e1963f7..0b4faa82ed 100644 --- a/zip/zip_test.go +++ b/zip/zip_test.go @@ -12,7 +12,6 @@ import ( "fmt" "hash" "io" - "io/ioutil" "runtime" "sort" "strings" @@ -597,7 +596,7 @@ func testZip64(t testing.TB, size int64) *rleBuffer { } // read back zip file and check that we get to the end of it - r, err := NewReader(buf, buf.Size()) + r, err := NewReader(buf, int64(buf.Size())) if err != nil { t.Fatal("reader:", err) } @@ -619,7 +618,7 @@ func testZip64(t testing.TB, size int64) *rleBuffer { t.Fatal("read:", err) } } - gotEnd, err := ioutil.ReadAll(rc) + gotEnd, err := io.ReadAll(rc) if err != nil { t.Fatal("read end:", err) } From 0e610953c04020f4e9dd8fa3ae9e01ea33c6efe5 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Mon, 27 Jun 2022 09:58:47 +0200 Subject: [PATCH 2/2] Add test files. --- zip/testdata/dupdir.zip | Bin 0 -> 458 bytes zip/testdata/test-baddirsz.zip | Bin 0 -> 1170 bytes zip/testdata/test-prefix.zip | Bin 0 -> 1227 bytes 3 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 zip/testdata/dupdir.zip create mode 100644 zip/testdata/test-baddirsz.zip create mode 100644 zip/testdata/test-prefix.zip diff --git a/zip/testdata/dupdir.zip b/zip/testdata/dupdir.zip new file mode 100644 index 0000000000000000000000000000000000000000..292720b7f01fc4ffafb47d0e6afc920b31e9b8ea GIT binary patch literal 458 zcmWIWW@Zs#fPlHdPzIP{V&G&*)DI0|Wng4jdO9%xtdIjNg{qJltS|{lB|AhJ)HH}r z2nSUs3q&U|masyVCgZV&kx7IBw_l*Hfl??wgJ{I%H-tt_ATv=P*{8Uz1o;gCk|8>g eeTv&skY5p?52_W}=d5fXc@`ji1*A`bI1B*y1}_r; literal 0 HcmV?d00001 diff --git a/zip/testdata/test-baddirsz.zip b/zip/testdata/test-baddirsz.zip new file mode 100644 index 0000000000000000000000000000000000000000..45b331407624d02ac41e3b53283e4960a6e9f832 GIT binary patch literal 1170 zcmWIWW@Zs#U|`^2XiQYKJ#hW)VM!oQ3M?YSP?B0)qE}K;5*otEz+CvJ$)^m6ODnh; z7+JnDGBAL3a-Te*6UMN}rFGJkM?wmLfr}l&aaaM)^pwV1FgBTd*)~VY5GrSri z$jrb1!XgYZ4C(m=8L36d`8oMThGrFpW_ksA>0oQD44Qqcff&u2&Hz7mUM?w+fxMm` zE&U=x?Zy@V2qPe0vcxr_Bsf2F!C6nVlf&(QE?5{rm?pSGb*exy9+#I&RurKk- z+m)0yRCPG=d-HHDZeo&8l5*8w*j(kQ)h25CKV`;*MHBS|=I94cI>i2RTbstM*llm) zO8IWizic_XckQR<_w%av+wcEed*5>UK?&z&H{O(96e0boH{+WCE(bu95f2^3EZ4O=VNZ?3)UvtyJg5QDRcf$^EI~Jki zWtQzJsL_x*GnsW)scd27+nNv&O9`(nP~TLZC!C)&dpmb9Gex+~DL%5m{kC!2{41u2r}!o#L;7=ONQ zE-qq{y7hnokNZ9E^>G%L>tA=N)+ZKnN{AiY63=$`mQ27(iv*7buak|7iv?tIZF@fl zt>|OAAsR5{#f{tFon;*@eKst4U%}L6{!DE$Q;-Gc;z`Q`4>jq0{kyW^?3@=d0sB55 zxoLcI^%PC5#tAeEd>m$^DR^;2N$vDP24)a*LyE?_;6T$0ZaWgUlR!) zO)1Y`?`E^OggslIl9kNpY|u0@YV$J_L(}Dqa{cO(OH|YIR<91PW~;8;CG9SvxT%P1 zf&1?rTi;(l9{BL-LFV7-o3%9b_5H&Yx{{nGExEP)?O%T1_U}*5o%1=o!$7pc+~7{P zVEy`CyQ(f_ZB6^{)1;6Rw%g;&&eO(;vcI48f3W$#VE69d#v5C|RN3w=iCp_vEP-JK z2X8m?^Q2e6G|k}Y>gTe~DWNIAn~_P58CTww04Zev=23T+UdKa(&kYWhQ$ShU>qC|zN%!0JcoK%J6 L{M_8syb?VCGGw}( literal 0 HcmV?d00001 diff --git a/zip/testdata/test-prefix.zip b/zip/testdata/test-prefix.zip new file mode 100644 index 0000000000000000000000000000000000000000..1eabb4861ece50d8464a5020e6f626ae22289ee2 GIT binary patch literal 1227 zcmXRYN=?hGP$>m%u`6MNKGy+NleN~Rme&#Qb^0pNe%F3W)Wdv zVBlbAOjNZ!aQ*FJNgz)OEF!~Dl3HA%S5i?D8p6rIT==KSrwoWoE4UdLS-vtdFo1P( zpFE)x#<0kxb<&kbLJEO_izZB)IeFD_RtAvKTtI^u80OZg+Ws@V8Y0NdzyQJ`3^EMq z`2`uNMalU&`9+3i6^3Sd1$pUUYpe{KeXM~P&6>^tKX+a(DUgA@o*phiKw1EZ`8n8t z?&#~tz;Nxx75@k$AYZb?HKHUqKdq!Zu_%?nF(n@u44%1(>8U}fi7AzZ zCsS=07?>7&x;TbZ+$xzI?-3j*abSP-`FF=kqr+}#spjUe1%`czSKh9qw4tiQk>8t# zYjG2ke3F!_{=()ePpvjl)BhF4KKRVlB(5Fs+eDs?A+gLCFYu_Y5&KIi;X6Pm`FkilwdYAO)d zb>PDThxE_f%a6Vmb@*e&^lWqJf=2>J>ie3T4i@|l48I$8c-yfE9WS$NPeF}_)S1bw zyGmut^TQG&xf0L6P_KD%A+Baoj(XkYkbd=;^BeL{bm~X>alQU*wjph^h*EuR7UGY$!;PJQp4|g~Rysb#rkMlhmyT40zn{ zd9RPNxLp6bOSL|+kW)hJ;Ffr{ySHQlR$3%@GbXF7szKjIE9Rac*G`nk;2LE~xHu}2&7s|{zAiK-nAfAH!mgFyH3 z*moB{szy~+1?YIoXDaYUi#oW7J#XUH`MutIp~Hv6@(Wn%ulbrt@MubT{(3i?#UMrVVjiBX%MnHZWbXO!z#mt3Nnp0|2+cr{yf!(&nV%=U0;XvO zPgg&ebxsLQ0p5&Ea?H5$t^`Oa12B&=ENKL>5ILC@l9SQ07sNDN*%e|M$ebmO(LfS= z<^`Gz%DfmRvx2fH0}~Lg0MfgF1%MPoNJeI{0uUuCRAm-`3L;=Zm7ANISE5&(pPQ