[release-branch.go1.16] archive/zip: don't panic on (*Reader).Open

Previously, opening a zip with (*Reader).Open could result in a panic if
the zip contained a file whose name was exclusively made up of slash
characters or ".." path elements.

Open could also panic if passed the empty string directly as an argument.

Now, any files in the zip whose name could not be made valid for
fs.FS.Open will be skipped, and no longer added to the fs.FS file list,
although they are still accessible through (*Reader).File.

Note that it was already the case that a file could be accessible from
(*Reader).Open with a name different from the one in (*Reader).File, as
the former is the cleaned name, while the latter is the original one.

Finally, made the actual panic site robust as a defense-in-depth measure.

Fixes CVE-2021-41772
Fixes #48251
Updates #48085

Co-authored-by: Filippo Valsorda <filippo@golang.org>
Change-Id: I6271a3f2892e7746f52e213b8eba9a1bba974678
Reviewed-on: https://go-review.googlesource.com/c/go/+/349770
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Julie Qiu <julie@golang.org>
(cherry picked from commit b24687394b55a93449e2be4e6892ead58ea9a10f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/360858
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>

1 files changed, 8 insertions, 2 deletions

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index 801d1313b6..b68d323b34 100644
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -692,6 +692,9 @@ func (r *Reader) initFileList() {
 		for _, file := range r.File {
 			isDir := len(file.Name) > 0 && file.Name[len(file.Name)-1] == '/'
 			name := toValidName(file.Name)
+			if name == "" {
+				continue
+			}
 			for dir := path.Dir(name); dir != "."; dir = path.Dir(dir) {
 				dirs[dir] = true
 			}
@@ -733,8 +736,11 @@ func fileEntryLess(x, y string) bool {
 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}
+	}
 	e := r.openLookup(name)
-	if e == nil || !fs.ValidPath(name) {
+	if e == nil {
 		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
 	}
 	if e.isDir {
@@ -748,7 +754,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]
 	}