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: improper sanitization of Transfer-Encoding header #53188

Closed
neild opened this issue Jun 1, 2022 · 8 comments
Closed

net/http: improper sanitization of Transfer-Encoding header #53188

neild opened this issue Jun 1, 2022 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Milestone
Go1.19

Comments

@neild
Copy link
Contributor

neild commented Jun 1, 2022

The net/http server improperly strips CRs surrounding the Transfer-Encoding header value, treating "Transfer-Encoding: \rchunked" as indicating a chunked body.

For example, this request is interpreted as containing the body a.

echo -ne "POST /post HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: \rchunked\r\n\r\n1\r\na\r\n0\r\n\r\n" | nc localhost 8080

This is a weak vector for request smuggling: CRs are not permitted in headers aside from in the CRLF line terminators, so this request is invalid. We should still fix this as a general hardening measure.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for reporting this issue.
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/409874 mentions this issue: net/http: don't strip whitespace from Transfer-Encoding headers

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 3, 2022
@dmitshur dmitshur added this to the Go1.19 milestone Jun 3, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/410714 mentions this issue: net/textproto: reject invalid header keys/values in ReadMIMEHeader

@neild
Copy link
Contributor Author

neild commented Jun 17, 2022

@gopherbot please open backport issues.

@gopherbot gopherbot mentioned this issue Jun 17, 2022
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #53432 (for 1.17), #53433 (for 1.18).

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

@gopherbot gopherbot mentioned this issue Jun 17, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/415217 mentions this issue: [release-branch.go1.17] net/http: don't strip whitespace from Transfer-Encoding headers

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/415218 mentions this issue: [release-branch.go1.18] net/http: don't strip whitespace from Transfer-Encoding headers

gopherbot pushed a commit that referenced this issue Jul 12, 2022
@neild @mknyszek
[release-branch.go1.17] net/http: don't strip whitespace from Transfe…
d13431c 
…r-Encoding headers

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For #53188
For CVE-2022-1705
Fixes #53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jul 12, 2022
@neild @mknyszek
[release-branch.go1.18] net/http: don't strip whitespace from Transfe…
222ee24 
…r-Encoding headers

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For #53188
For CVE-2022-1705
Fixes #53433

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415218
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Jul 14, 2022
@neild @bradfitz
[release-branch.go1.18] net/http: don't strip whitespace from Transfe…
61d05bd 
…r-Encoding headers

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53433

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415218
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@tatianab tatianab mentioned this issue Jul 14, 2022
Closed
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
@neild @jproberts
net/http: don't strip whitespace from Transfer-Encoding headers
2ded4a8 
Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

Fixes golang#53188
Fixes CVE-2022-1705

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
danbudris pushed a commit to danbudris/go that referenced this issue Sep 9, 2022
@neild @danbudris
[release-branch.go1.17] net/http: don't strip whitespace from Transfe…
d51c433 
…r-Encoding headers

EKS: retain original use of strings.ToLower present in 1.16. See: golang@5c48951

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
danbudris pushed a commit to danbudris/go that referenced this issue Sep 9, 2022
@neild @danbudris
[release-branch.go1.17] net/http: don't strip whitespace from Transfe…
36ad18d 
…r-Encoding headers

EKS: retain original use of strings.ToLower present in 1.16. See: golang@5c48951

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
danbudris pushed a commit to danbudris/go that referenced this issue Sep 12, 2022
@neild @danbudris
[release-branch.go1.17] net/http: don't strip whitespace from Transfe…
6ede48b 
…r-Encoding headers

EKS: retain original use of strings.ToLower present in 1.16. See: golang@5c48951

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
danbudris pushed a commit to danbudris/go that referenced this issue Sep 14, 2022
@neild @danbudris
[release-branch.go1.17] net/http: don't strip whitespace from Transfe…
b8c48c1 
…r-Encoding headers

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
rcrozean pushed a commit to rcrozean/go that referenced this issue Oct 5, 2022
@neild @rcrozean
net/http: don't strip whitespace from Transfer-Encoding headers
c3c7086 
# AWS EKS
Backported To: go-1.15.15-eks
Backported On: Thu, 22 Sept 2022
Backported By: budris@amazon.com
Backported From: release-branch.go1.17
EKS Patch Source Commit: danbudris@b8c48c1
Upstream Source Commit: golang@d13431c

# Original Information

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
rcrozean pushed a commit to rcrozean/go that referenced this issue Oct 12, 2022
@neild @rcrozean
net/http: don't strip whitespace from Transfer-Encoding headers
895d987 
# AWS EKS
Backported To: go-1.15.15-eks
Backported On: Thu, 22 Sept 2022
Backported By: budris@amazon.com
Backported From: release-branch.go1.17
EKS Patch Source Commit: danbudris@b8c48c1
Upstream Source Commit: golang@d13431c

# Original Information

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
rcrozean pushed a commit to rcrozean/go that referenced this issue Oct 12, 2022
@neild @rcrozean
net/http: don't strip whitespace from Transfer-Encoding headers
891dd82 
# AWS EKS
Backported To: go-1.16.15-eks
Backported On: Tue, 04 Oct 2022
Backported By: budris@amazon.com
Backported From: release-branch.go1.17
EKS Patch Source Commit: danbudris@d51c433
Upstream Source Commit: tailscale@61d05bd

# Original Information

EKS: retain original use of strings.ToLower present in 1.16. See: golang@5c48951

Do not accept "Transfer-Encoding: \rchunked" as a valid TE header
setting chunked encoding.

Thanks to Zeyu Zhang (https://www.zeyu2001.com/) for identifying
the issue.

For golang#53188
For CVE-2022-1705
Fixes golang#53432

Change-Id: I1a16631425159267f2eca68056b057192a7edf6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/409874
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e5017a9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/415217
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Nov 8, 2022
@neild
net/textproto: reject invalid header keys/values in ReadMIMEHeader
a6642e6 
Return an error when parsing a MIME header containing bytes in the
key or value outside the set allowed by RFC 7230.

For historical compatibility, accept spaces in keys (but do not
canonicalize the key in this case).

For #53188.

Change-Id: I195319362a2fc69c4e506644f78c5026db070379
Reviewed-on: https://go-review.googlesource.com/c/go/+/410714
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
@iredmail iredmail mentioned this issue Mar 8, 2023
Closed
@seankhliao seankhliao mentioned this issue May 22, 2023
Closed
@realbodo realbodo mentioned this issue May 26, 2023
Open
@gopherbot gopherbot mentioned this issue Jun 19, 2023
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/504416 mentions this issue: net/mail: permit more characters in mail headers

@gopherbot gopherbot mentioned this issue Jun 19, 2023
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/504881 mentions this issue: [release-branch.go1.20] net/mail: permit more characters in mail headers

gopherbot pushed a commit that referenced this issue Jun 21, 2023
@ianlancetaylor @gopherbot
net/mail: permit more characters in mail headers
78c3aba 
We parse mail messages using net/textproto. For #53188, we tightened
up the bytes permitted by net/textproto to match RFC 7230.
However, this package uses RFC 5322 which is more permissive.
Restore the permisiveness we used to have, so that older code
continues to work.

Fixes #58862
Fixes #60332

Change-Id: I5437f5e18a756f6ca61c13c4d8ba727be73eff9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/504416
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Jun 21, 2023
@ianlancetaylor
[release-branch.go1.20] net/mail: permit more characters in mail headers
b59efe6 
We parse mail messages using net/textproto. For #53188, we tightened
up the bytes permitted by net/textproto to match RFC 7230.
However, this package uses RFC 5322 which is more permissive.
Restore the permisiveness we used to have, so that older code
continues to work.

For #58862
For #60332
Fixes #60874
Fixes #60875

Change-Id: I5437f5e18a756f6ca61c13c4d8ba727be73eff9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/504881
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Jul 15, 2023
@ianlancetaylor @bradfitz
net/mail: permit more characters in mail headers
cdf3273 
We parse mail messages using net/textproto. For golang#53188, we tightened
up the bytes permitted by net/textproto to match RFC 7230.
However, this package uses RFC 5322 which is more permissive.
Restore the permisiveness we used to have, so that older code
continues to work.

Fixes golang#58862
Fixes golang#60332

Change-Id: I5437f5e18a756f6ca61c13c4d8ba727be73eff9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/504416
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@golang golang locked and limited conversation to collaborators Jun 20, 2024
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
Release Blockers
Status: Done
Milestone
Go1.19
Development

No branches or pull requests
4 participants
@neild @dmitshur @tatianab @gopherbot