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: malformed archive may cause panic or memory exhaustion #46242

Closed
rolandshoemaker opened this issue May 18, 2021 · 6 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker Security
Milestone

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented May 18, 2021

Due to a pre-allocation optimization in zip.NewReader, a malformed archive which indicates it has a significant number of files can cause either a panic or memory exhaustion.

This was originally discoverd by OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=33912.

This is CVE-2021-33196.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/318909 mentions this issue: archive/zip: only preallocate File slice if reasonably sized

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label May 21, 2021
@dmitshur dmitshur added this to the Go1.17 milestone May 21, 2021
@dmitshur
Copy link
Contributor

dmitshur commented May 24, 2021

Applying release-blocker and okay-after-beta1 labels based on my best understanding from conversation in #46241, please update if needed.

@dmitshur dmitshur added okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker labels May 24, 2021
@rolandshoemaker
Copy link
Member Author

@gopherbot please consider this for backport to 1.15 and 1.16 as this is a security issue.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #46396 (for 1.15), #46397 (for 1.16).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/322909 mentions this issue: [release-branch.go1.16] archive/zip: only preallocate File slice if reasonably sized

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/322949 mentions this issue: [release-branch.go1.15] archive/zip: only preallocate File slice if reasonably sized

gopherbot pushed a commit that referenced this issue May 28, 2021
…easonably sized

Since the number of files in the EOCD record isn't validated, it isn't
safe to preallocate Reader.Files using that field. A malformed archive
can indicate it contains up to 1 << 128 - 1 files. We can still safely
preallocate the slice by checking if the specified number of files in
the archive is reasonable, given the size of the archive.

Thanks to the OSS-Fuzz project for discovering this issue and to
Emmanuel Odeke for reporting it.

Updates #46242
Fixes #46396
Fixes CVE-2021-33196

Change-Id: I3c76d8eec178468b380d87fdb4a3f2cb06f0ee76
Reviewed-on: https://go-review.googlesource.com/c/go/+/318909
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
(cherry picked from commit 74242ba)
Reviewed-on: https://go-review.googlesource.com/c/go/+/322949
Reviewed-by: Filippo Valsorda <filippo@golang.org>
gopherbot pushed a commit that referenced this issue May 28, 2021
…easonably sized

Since the number of files in the EOCD record isn't validated, it isn't
safe to preallocate Reader.Files using that field. A malformed archive
can indicate it contains up to 1 << 128 - 1 files. We can still safely
preallocate the slice by checking if the specified number of files in
the archive is reasonable, given the size of the archive.

Thanks to the OSS-Fuzz project for discovering this issue and to
Emmanuel Odeke for reporting it.

Updates #46242
Fixes #46397
Fixes CVE-2021-33196

Change-Id: I3c76d8eec178468b380d87fdb4a3f2cb06f0ee76
Reviewed-on: https://go-review.googlesource.com/c/go/+/318909
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
(cherry picked from commit 74242ba)
Reviewed-on: https://go-review.googlesource.com/c/go/+/322909
Reviewed-by: Filippo Valsorda <filippo@golang.org>
klauspost added a commit to klauspost/compress that referenced this issue Jun 4, 2021
Since the number of files in the EOCD record isn't validated, it isn't
safe to preallocate Reader.Files using that field. A malformed archive
can indicate it contains up to 1 << 128 - 1 files. We can still safely
preallocate the slice by checking if the specified number of files in
the archive is reasonable, given the size of the archive.

Thanks to the OSS-Fuzz project for discovering this issue and to
Emmanuel Odeke for reporting it.

See golang/go#46242
netbsd-srcmastr referenced this issue in NetBSD/pkgsrc Jun 4, 2021
go1.15.13 (released 2021-06-03) includes security fixes to the archive/zip,
math/big, net, and net/http/httputil packages, as well as bug fixes to the
linker, the go command, and the math/big and net/http packages. See the Go
1.15.13 milestone on our issue tracker for details.

The SetString and UnmarshalText methods of math/big.Rat
<https://pkg.go.dev/math/big#Rat> may cause a panic or an unrecoverable
fatal error if passed inputs with very large exponents.
This is issue <golang/go#44910> and
CVE-2021-33198.

Thanks to the OSS-Fuzz project for discovering this issue and to Emmanuel
Odeke for reporting it.

ReverseProxy in net/http/httputil <https://pkg.go.dev/net/http/httputil> could
be made to forward certain hop-by-hop headers, including Connection. In
case the target of the ReverseProxy was itself a reverse proxy, this would
let an attacker drop arbitrary headers, including those set by the
ReverseProxy.Director.
This is issue <golang/go#46313> and
CVE-2021-33197.

Thanks to Mattias Grenfeldt (https://grenfeldt.dev) and Asta Olofsson for
reporting this issue.

The LookupCNAME, LookupSRV, LookupMX, LookupNS, and LookupAddr functions in
net <https://pkg.go.dev/net>, and their respective methods on the Resolver
<https://pkg.go.dev/net#Resolver> type may return arbitrary values
retrieved from DNS which do not follow the established RFC 1035
<https://datatracker.ietf.org/doc/html/rfc1035>rules for domain names. If
these names are used without further sanitization, for instance unsafely
included in HTML, they may allow for injection of unexpected content. Note
that LookupTXT may still return arbitrary values that could require
sanitization before further use.
This is issue <golang/go#46241> and
CVE-2021-33195.

Thanks to Philipp Jeitner and Haya Shulman from Fraunhofer SIT for
reporting this issue.

The NewReader and OpenReader functions in archive/zip
<https://pkg.go.dev/archive/zip> can cause a panic or an unrecoverable
fatal error when reading an archive that claims to contain a large number
of files, regardless of its actual size.
This is issue <https://github.com/golang/go/issues/46242>and
CVE-2021-33196.

Thanks to the OSS-Fuzz project for discovering this issue and to Emmanuel
Odeke for reporting it.
netbsd-srcmastr referenced this issue in NetBSD/pkgsrc Jun 5, 2021
go1.16.5 (released 2021-06-03) includes security fixes to the archive/zip, math
/big, net, and net/http/httputil packages, as well as bug fixes to the linker,
the go command, and the net/http package. See the Go 1.16.5 milestone on our
issue tracker for details.

The SetString and UnmarshalText methods of math/big.Rat
<https://pkg.go.dev/math/big#Rat> may cause a panic or an unrecoverable
fatal error if passed inputs with very large exponents.
This is issue <golang/go#44910> and
CVE-2021-33198.

Thanks to the OSS-Fuzz project for discovering this issue and to Emmanuel
Odeke for reporting it.

ReverseProxy in net/http/httputil <https://pkg.go.dev/net/http/httputil> could
be made to forward certain hop-by-hop headers, including Connection. In
case the target of the ReverseProxy was itself a reverse proxy, this would
let an attacker drop arbitrary headers, including those set by the
ReverseProxy.Director.
This is issue <golang/go#46313> and
CVE-2021-33197.

Thanks to Mattias Grenfeldt (https://grenfeldt.dev) and Asta Olofsson for
reporting this issue.

The LookupCNAME, LookupSRV, LookupMX, LookupNS, and LookupAddr functions in
net <https://pkg.go.dev/net>, and their respective methods on the Resolver
<https://pkg.go.dev/net#Resolver> type may return arbitrary values
retrieved from DNS which do not follow the established RFC 1035
<https://datatracker.ietf.org/doc/html/rfc1035>rules for domain names. If
these names are used without further sanitization, for instance unsafely
included in HTML, they may allow for injection of unexpected content. Note
that LookupTXT may still return arbitrary values that could require
sanitization before further use.
This is issue <golang/go#46241> and
CVE-2021-33195.

Thanks to Philipp Jeitner and Haya Shulman from Fraunhofer SIT for
reporting this issue.

The NewReader and OpenReader functions in archive/zip
<https://pkg.go.dev/archive/zip> can cause a panic or an unrecoverable
fatal error when reading an archive that claims to contain a large number
of files, regardless of its actual size.
This is issue <https://github.com/golang/go/issues/46242>and
CVE-2021-33196.

Thanks to the OSS-Fuzz project for discovering this issue and to Emmanuel
Odeke for reporting it.
@golang golang locked and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 release-blocker Security
Projects
None yet
Development

No branches or pull requests

3 participants