Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

archive/zip: Reader.Open panics on empty string [1.16 backport] #48251

Closed
gopherbot opened this issue Sep 8, 2021 · 2 comments
Closed

archive/zip: Reader.Open panics on empty string [1.16 backport] #48251

gopherbot opened this issue Sep 8, 2021 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker Security
Milestone

Comments

@gopherbot
Copy link

@FiloSottile requested issue #48085 to be considered for backport to the next 1.16 minor release.

Thank you for reporting this to security@golang.org. Invalid input should not cause programs to panic, if the input could be attacker controlled. If this required a call to Open("") to trigger, it could have been borderline, since it's hard for an attacker to control the argument to Open. However, the reproducer in #48085 (comment) triggers a panic with a real file name.

package main

import "archive/zip"

func main() {
	reader, err := zip.OpenReader("liquibase-core-4.4.3-sources.zip")
	if err != nil {
		panic(err)
	}

	reader.Open("META-INF/MANIFEST.MF")
}

We'll backport this as a security fix in the PUBLIC track. @gopherbot, please open backport issues.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 8, 2021
@gopherbot gopherbot added this to the Go1.16.8 milestone Sep 8, 2021
@thanm thanm modified the milestones: Go1.16.8, Go1.16.9 Sep 9, 2021
@mknyszek mknyszek modified the milestones: Go1.16.9, Go1.16.10 Oct 7, 2021
@gopherbot
Copy link
Author

Change https://golang.org/cl/360858 mentions this issue: [release-branch.go1.16] archive/zip: don't panic on (*Reader).Open

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Nov 2, 2021
@gopherbot
Copy link
Author

Closed by merging 88407a8 to release-branch.go1.16.

gopherbot pushed a commit that referenced this issue Nov 3, 2021
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 b246873)
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>
@golang golang locked and limited conversation to collaborators Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker Security
Projects
None yet
Development

No branches or pull requests

5 participants