]> Cypherpunks.ru repositories - gostls13.git/commitdiff
archive/zip: error if using io/fs on zip with duplicate entries
authorIan Lance Taylor <iant@golang.org>
Thu, 30 Dec 2021 06:02:45 +0000 (22:02 -0800)
committerGopher Robot <gobot@golang.org>
Tue, 10 May 2022 17:51:16 +0000 (17:51 +0000)
Fixes #50390

Change-Id: I92787cdb3fa198ff88dcaadeccfcb49a3a6a88cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/374954
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/archive/zip/reader.go
src/archive/zip/reader_test.go
src/archive/zip/testdata/dupdir.zip [new file with mode: 0644]

index d875c7be252c7b4c30dd3af3eb67dddcafdc6deb..906f3d308af4bf4b760930231edd17313bf196e5 100644 (file)
@@ -662,6 +662,7 @@ type fileListEntry struct {
        name  string
        file  *File
        isDir bool
+       isDup bool
 }
 
 type fileInfoDirEntry interface {
@@ -669,11 +670,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.
@@ -708,17 +712,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,
@@ -726,17 +750,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)
                        }
                }
 
@@ -831,7 +861,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")}
@@ -850,7 +880,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
index 4c1e82b9d4c273b2533e898eeb7da8187dd5366c..41e720aae7a82ac659ccea3cb4603d46d0f3d664 100644 (file)
@@ -505,6 +505,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) {
@@ -1141,6 +1170,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)
@@ -1155,6 +1185,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")
diff --git a/src/archive/zip/testdata/dupdir.zip b/src/archive/zip/testdata/dupdir.zip
new file mode 100644 (file)
index 0000000..292720b
Binary files /dev/null and b/src/archive/zip/testdata/dupdir.zip differ