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.

Sign up for GitHub

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

Jump to bottom

archive/zip: overflow in preallocation check can cause OOM panic #47801

Closed
rolandshoemaker opened this issue Aug 19, 2021 · 7 comments
Closed

archive/zip: overflow in preallocation check can cause OOM panic #47801

rolandshoemaker opened this issue Aug 19, 2021 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Security

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Aug 19, 2021 •
edited

An oversight in the fix for #46242 still allows for an OOM panic when the indicated directory size in the archive header is so large that subtracting it from the archive size overflows a uint64, effectively bypassing the check that the number of files in the archive is reasonable.

This is CVE-2021-39293.
@rolandshoemaker rolandshoemaker added Security NeedsFix The path to resolution is known, but the work has not been done. labels Aug 19, 2021
@gopherbot
Copy link

Change https://golang.org/cl/343434 mentions this issue: archive/zip: prevent preallocation check from overflowing

@odeke-em
Copy link
Member

Thank you for filing this issue, and for fixing it @rolandshoemaker!

Could we perhaps backport the fix? There isn't a way for Go users using 1.16 and 1.17 to mitigate this issue, and I can see a huge surface number of affected users who can't upgrade to tip aka Go1.18 currently.

@rolandshoemaker
Copy link
Member Author

Oops, yup, thought I'd already done this.

@rolandshoemaker
Copy link
Member Author

@gopherbot please backport to 1.16 and 1.17.

This is a security issue.

This was referenced Aug 26, 2021
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #47985 (for 1.16), #47986 (for 1.17).

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

Change https://golang.org/cl/345409 mentions this issue: [release-branch.go1.16] archive/zip: prevent preallocation check from overflowing

@gopherbot
Copy link

Change https://golang.org/cl/345410 mentions this issue: [release-branch.go1.17] archive/zip: prevent preallocation check from overflowing

gopherbot pushed a commit that referenced this issue Sep 1, 2021
@rolandshoemaker @cherrymui
[release-branch.go1.17] archive/zip: prevent preallocation check from…
1dd24ca 
… overflowing

If the indicated directory size in the archive header is so large that
subtracting it from the archive size overflows a uint64, the check that
the indicated number of files in the archive can be effectively
bypassed. Prevent this from happening by checking that the indicated
directory size is less than the size of the archive.

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

Fixes #47986
Updates #47801
Fixes CVE-2021-39293

Change-Id: Ifade26b98a40f3b37398ca86bd5252d12394dd24
Reviewed-on: https://go-review.googlesource.com/c/go/+/343434
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit bacbc33)
Reviewed-on: https://go-review.googlesource.com/c/go/+/345410
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Cherry Mui <cherryyz@google.com>
gopherbot pushed a commit that referenced this issue Sep 1, 2021
@rolandshoemaker @cherrymui
[release-branch.go1.16] archive/zip: prevent preallocation check from…
6c48001 
… overflowing

If the indicated directory size in the archive header is so large that
subtracting it from the archive size overflows a uint64, the check that
the indicated number of files in the archive can be effectively
bypassed. Prevent this from happening by checking that the indicated
directory size is less than the size of the archive.

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

Fixes #47985
Updates #47801
Fixes CVE-2021-39293

Change-Id: Ifade26b98a40f3b37398ca86bd5252d12394dd24
Reviewed-on: https://go-review.googlesource.com/c/go/+/343434
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit bacbc33)
Reviewed-on: https://go-review.googlesource.com/c/go/+/345409
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Cherry Mui <cherryyz@google.com>
@alvin-huang alvin-huang mentioned this issue Nov 3, 2021
Merged
@msakall1 msakall1 mentioned this issue Nov 4, 2021
Closed
@ramps41 ramps41 mentioned this issue Nov 16, 2021
Closed
@vanugrah vanugrah mentioned this issue Nov 16, 2021
Closed
@pjachowi pjachowi mentioned this issue Nov 17, 2021
Closed
@daveneeley daveneeley mentioned this issue Dec 18, 2021
Closed
@vmikula vmikula mentioned this issue Jan 3, 2022
Closed
@golang golang locked and limited conversation to collaborators Aug 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Security
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@odeke-em @rolandshoemaker @gopherbot