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

net/http: ReadRequest can stack overflow due to recursion with very large headers #45710

Closed
katiehockman opened this issue Apr 22, 2021 · 10 comments · Fixed by containerd/containerd#5461
Closed

net/http: ReadRequest can stack overflow due to recursion with very large headers #45710

katiehockman opened this issue Apr 22, 2021 · 10 comments · Fixed by containerd/containerd#5461
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Milestone
Go1.17

Comments

@katiehockman
Copy link
Contributor

katiehockman commented Apr 22, 2021 •
edited
Loading

ReadRequest and ReadResponse in net/http can hit an unrecoverable panic when reading a very large header (over 7MB on 64-bit architectures, or over 4MB on 32-bit ones). Transport and Client are vulnerable and the program can be made to crash by a malicious server. Server is not vulnerable by default, but can be if the default max header of 1MB is overridden by setting Server.MaxHeaderBytes to a higher value, in which case the program can be made to crash by a malicious client.

This also affects golang.org/x/net/http2/h2c and HeaderValuesContainsToken in golang.org/x/net/http/httpguts, and is fixed in golang.org/x/[email protected].

This is CVE-2021-31525.

According to the new security policy (#44918), this will be fixed as a PUBLIC track issue.

Credit to Guido Vranken who reported the crash as part of the Ethereum 2.0 bounty program.

/cc @golang/security
@katiehockman katiehockman added this to the Go1.17 milestone Apr 22, 2021
@katiehockman katiehockman self-assigned this Apr 22, 2021
@katiehockman
Copy link
Contributor Author

katiehockman commented Apr 22, 2021 •
edited
Loading

@gopherbot please consider this for backport to 1.16.4 and 1.15.12, it's a security issue.

/cc @golang/release FYI

@gopherbot gopherbot mentioned this issue Apr 22, 2021
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #45711 (for 1.15), #45712 (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 gopherbot mentioned this issue Apr 22, 2021
Closed
@odeke-em odeke-em changed the title http: ReadRequest can stack overflow net/http: ReadRequest can stack overflow due to recursion with very large headers Apr 23, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/313069 mentions this issue: httpguts: remove recursion for HeaderValuesContainsToken

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314596 mentions this issue: std: update golang.org/x/net to 20210428140749-89ef3d95e781

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314649 mentions this issue: [internal-branch.go1.16-vendor

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 28, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/314650 mentions this issue: [release-branch.go1.15] http/httpguts: remove recursion in HeaderValuesContainsToken

@dmitshur
Copy link
Contributor

dmitshur commented Apr 28, 2021 •
edited
Loading

The x/net fix still needs to be pulled into the main repo for Go 1.17 (which can be done individually like in CL 314596 or together with more x/net updates as part of #36905), reopening for that and adding release-blocker so it's not missed.

@dmitshur dmitshur reopened this Apr 28, 2021
gopherbot pushed a commit to golang/net that referenced this issue Apr 28, 2021
@katiehockman
[internal-branch.go1.16-vendor] http/httpguts: remove recursion in He…
3f4a416 
…aderValuesContainsToken

Previously, httpguts.HeaderValuesContainsToken called a
function which could recurse to the point of a stack
overflow when given a very large header (~10MB).

Credit to Guido Vranken who reported the crash as
part of the Ethereum 2.0 bounty program.

Fixes CVE-2021-31525

Updates golang/go#45710
Updates golang/go#45712

Change-Id: I2c54ce3b2acf1c5efdea66db0595b93a3f5ae5f3
Reviewed-on: https://go-review.googlesource.com/c/net/+/313069
Trust: Katie Hockman <[email protected]>
Run-TryBot: Katie Hockman <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
(cherry picked from commit 89ef3d9)
Reviewed-on: https://go-review.googlesource.com/c/net/+/314649
Reviewed-by: Dmitri Shuralyov <[email protected]>
gopherbot pushed a commit to golang/net that referenced this issue Apr 28, 2021
@katiehockman
[release-branch.go1.15] http/httpguts: remove recursion in HeaderValu…
261fb51 
…esContainsToken

Previously, httpguts.HeaderValuesContainsToken called a
function which could recurse to the point of a stack
overflow when given a very large header (~10MB).

Credit to Guido Vranken who reported the crash as
part of the Ethereum 2.0 bounty program.

Fixes CVE-2021-31525

Updates golang/go#45710
Updates golang/go#45711

Change-Id: I2c54ce3b2acf1c5efdea66db0595b93a3f5ae5f3
Reviewed-on: https://go-review.googlesource.com/c/net/+/313069
Trust: Katie Hockman <[email protected]>
Run-TryBot: Katie Hockman <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
(cherry picked from commit 89ef3d9)
Reviewed-on: https://go-review.googlesource.com/c/net/+/314650
Reviewed-by: Dmitri Shuralyov <[email protected]>
@dmitshur
Copy link
Contributor

dmitshur commented May 6, 2021

The x/net fix has been pulled into the main repo for Go 1.17 as part of CL 316251. Closing as fixed.

@dmitshur dmitshur closed this as completed May 6, 2021
This was referenced May 6, 2021
Merged
Merged
AkihiroSuda pushed a commit to containerd/containerd that referenced this issue May 7, 2021
@tao12345666333
Update Go to 1.16.4
79d800b 
fix [#45710](golang/go#45710) and CVE-2021-31525.

Signed-off-by: Jintao Zhang <[email protected]>
@tao12345666333 tao12345666333 mentioned this issue May 7, 2021
Closed
15 tasks
@carnil
Copy link

carnil commented May 7, 2021

This issue appears to have assigned CVE-2021-31525 according to https://bugzilla.redhat.com/show_bug.cgi?id=1958341

buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue May 8, 2021
@jacmet
package/go: security bump to version 1.16.4
1cfc01a 
Fixes the following security issues:

- CVE-2021-31525: ReadRequest and ReadResponse in net/http can hit an
  unrecoverable panic when reading a very large header (over 7MB on 64-bit
  architectures, or over 4MB on 32-bit ones).  Transport and Client are
  vulnerable and the program can be made to crash by a malicious server.
  Server is not vulnerable by default, but can be if the default max header
  of 1MB is overridden by setting Server.MaxHeaderBytes to a higher value,
  in which case the program can be made to crash by a malicious client.

  golang/go#45710

Signed-off-by: Peter Korsgaard <[email protected]>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue May 8, 2021
@jacmet
package/go: security bump to version 1.15.12
91eb510 
Fixes the following security issues:

- CVE-2021-31525: ReadRequest and ReadResponse in net/http can hit an
  unrecoverable panic when reading a very large header (over 7MB on 64-bit
  architectures, or over 4MB on 32-bit ones).  Transport and Client are
  vulnerable and the program can be made to crash by a malicious server.
  Server is not vulnerable by default, but can be if the default max header
  of 1MB is overridden by setting Server.MaxHeaderBytes to a higher value,
  in which case the program can be made to crash by a malicious client.

  golang/go#45710

Signed-off-by: Peter Korsgaard <[email protected]>
@katiehockman
Copy link
Contributor Author

katiehockman commented May 10, 2021 •
edited
Loading

Yes, it has been assigned CVE-2021-31525.

I've gone ahead and update the original comment with the content of the announcement, which had the full details.

dmcgowan pushed a commit to dmcgowan/containerd that referenced this issue May 10, 2021
@tao12345666333 @dmcgowan
Update Go to 1.16.4
3a86110 
fix [#45710](golang/go#45710) and CVE-2021-31525.

Signed-off-by: Jintao Zhang <[email protected]>
(cherry picked from commit 79d800b)
Signed-off-by: Derek McGowan <[email protected]>
olix0r added a commit to linkerd/linkerd2 that referenced this issue May 24, 2021
@olix0r
Update Go to 1.16.4
6531d49 
Go 1.16.4 includes a fix for a denial-of-service in net/http: golang/go#45710

Go's error file-line formatting changed in 1.16.3, so this change
updates tests to only do suffix matching on these error strings.
@olix0r olix0r mentioned this issue May 24, 2021
Merged
olix0r added a commit to linkerd/linkerd2 that referenced this issue May 24, 2021
@olix0r
Update Go to 1.16.4 (#6170)
da6d8e5 
Go 1.16.4 includes a fix for a denial-of-service in net/http: golang/go#45710

Go's error file-line formatting changed in 1.16.3, so this change
updates tests to only do suffix matching on these error strings.
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Sep 14, 2021
@jacmet
package/go: security bump to version 1.16.4
f52be85 
Fixes the following security issues:

- CVE-2021-31525: ReadRequest and ReadResponse in net/http can hit an
  unrecoverable panic when reading a very large header (over 7MB on 64-bit
  architectures, or over 4MB on 32-bit ones).  Transport and Client are
  vulnerable and the program can be made to crash by a malicious server.
  Server is not vulnerable by default, but can be if the default max header
  of 1MB is overridden by setting Server.MaxHeaderBytes to a higher value,
  in which case the program can be made to crash by a malicious client.

  golang/go#45710

Signed-off-by: Peter Korsgaard <[email protected]>
(cherry picked from commit 1cfc01a)
Signed-off-by: Peter Korsgaard <[email protected]>
thaJeztah pushed a commit to thaJeztah/containerd that referenced this issue Sep 15, 2021
@tao12345666333 @thaJeztah
Update Go to 1.16.4
8cc99a8 
fix [#45710](golang/go#45710) and CVE-2021-31525.

Signed-off-by: Jintao Zhang <[email protected]>
(cherry picked from commit 79d800b)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
fahedouch pushed a commit to fahedouch/containerd that referenced this issue Oct 15, 2021
@tao12345666333 @fahedouch
Update Go to 1.16.4
3f12a51 
fix [#45710](golang/go#45710) and CVE-2021-31525.

Signed-off-by: Jintao Zhang <[email protected]>
@poberg poberg mentioned this issue Apr 7, 2022
Closed
@fredriksvantes fredriksvantes mentioned this issue Apr 15, 2022
Closed
@golang golang locked and limited conversation to collaborators May 10, 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. release-blocker Security
Projects
None yet
Milestone
Go1.17
Development

Successfully merging a pull request may close this issue.

Update Go to 1.16.4 tao12345666333/containerd
4 participants
@carnil @dmitshur @gopherbot @katiehockman