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: Lookup functions may return invalid host names #46241

Closed
rolandshoemaker opened this issue May 18, 2021 · 17 comments
Closed

net: Lookup functions may return invalid host names #46241

rolandshoemaker opened this issue May 18, 2021 · 17 comments
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
Go1.17

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented May 18, 2021 •
edited
Loading

The net.Lookup{Addr,CNAME,Host} functions don't do any filtering of returned host name string types when using the pure Go resolver, allowing for invalid names to be returned to the caller. If the caller expects these names to be valid they may use them in an unsanitized context, allowing for injection of unexpected content. Depending on the implementation, the cgo resolver may do some level of filtering, for instance the glibc implementation of getaddrinfo does impose its own filtering.

The simple approach to this is to check returned names with the existing isDomainName function, which applies RFC 1035 LDH rules (as well as allowing underscores for SRV style names), and failing out if the returned names are not considered proper. This should mostly match glibc behavior. In order to avoid diverging behavior across implementations, the check should probably be done at the Resolver level, rather than just in the pure Go Lookup... implementations.

This is CVE-2021-33195.
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/320949 mentions this issue: net: verify results from Lookup* are valid domain names

@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

Roland, should this issue (and #46242) block Go 1.17 Beta 1 (target date of June 1), or is okay not to block beta 1 as long as it's fixed for final 1.17 release?

@katiehockman
Copy link
Contributor

@dmitshur this will be backported to 1.16 and 1.15 for the upcoming minor releases (targeted to June 1 as well), so if all goes according to plan, it should be merged before the 1.17 beta anyway.

@dmitshur
Copy link
Contributor

dmitshur commented May 24, 2021 •
edited
Loading

@katiehockman If I understand correctly, your comment is suggesting that it would be safe to apply both release-blocker and okay-after-beta1 labels to this issue, so that automated tooling only reports a problem if this issue is unexpectedly still not resolved by the time the final Go 1.17 release is about to be made. Is my understanding right?

@katiehockman
Copy link
Contributor

@dmitshur yes that sounds right. @rolandshoemaker can correct me if I'm wrong there.

@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
@dmitshur
Copy link
Contributor

Thanks. I applied those labels here and in #46242. Please make changes if needed (e.g., you can remove okay-after-beta1 if we shouldn't release Go 1.17 Beta 1 without this fix).

Having these labels helps ensure release tooling will not stay quietl if something doesn't go as originally anticipated, so @rolandshoemaker please feel free to apply them as needed when filing issues in the future.

@dmitshur dmitshur mentioned this issue May 24, 2021
Closed
@rolandshoemaker
Copy link
Member Author

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

@gopherbot gopherbot mentioned this issue May 24, 2021
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #46356 (for 1.15), #46357 (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 May 24, 2021
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/322230 mentions this issue: [release-branch.go1.16] net: verify results from Lookup* are valid domain names

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/322231 mentions this issue: [release-branch.go1.15] net: verify results from Lookup* are valid domain names

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/323013 mentions this issue: net: apply name validity check to non-Resolver lookups

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/323131 mentions this issue: net: verify results from Lookup* are valid domain names

gopherbot pushed a commit that referenced this issue May 27, 2021
@rolandshoemaker
net: verify results from Lookup* are valid domain names
cdcd028 
For the methods LookupCNAME, LookupSRV, LookupMX, LookupNS, and
LookupAddr check that the returned domain names are in fact valid DNS
names using the existing isDomainName function.

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

Fixes #46241
Fixes CVE-2021-33195

Change-Id: I47a4f58c031cb752f732e88bbdae7f819f0af4f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/323131
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/323269 mentions this issue: [release-branch.go1.15] net: verify results from Lookup* are valid domain names

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/323270 mentions this issue: [release-branch.go1.16] net: verify results from Lookup* are valid domain names

gopherbot pushed a commit that referenced this issue May 27, 2021
@rolandshoemaker
[release-branch.go1.16] net: verify results from Lookup* are valid do…
df6a737 
…main names

For the methods LookupCNAME, LookupSRV, LookupMX, LookupNS, and
LookupAddr check that the returned domain names are in fact valid DNS
names using the existing isDomainName function.

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

Updates #46241
Fixes #46357
Fixes CVE-2021-33195

Change-Id: I47a4f58c031cb752f732e88bbdae7f819f0af4f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/323131
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
(cherry picked from commit cdcd028)
Reviewed-on: https://go-review.googlesource.com/c/go/+/323270
gopherbot pushed a commit that referenced this issue May 27, 2021
@rolandshoemaker
[release-branch.go1.15] net: verify results from Lookup* are valid do…
31d60cd 
…main names

For the methods LookupCNAME, LookupSRV, LookupMX, LookupNS, and
LookupAddr check that the returned domain names are in fact valid DNS
names using the existing isDomainName function.

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

Updates #46241
Fixes #46356
Fixes CVE-2021-33195

Change-Id: I47a4f58c031cb752f732e88bbdae7f819f0af4f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/323131
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
(cherry picked from commit cdcd028)
Reviewed-on: https://go-review.googlesource.com/c/go/+/323269
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jun 4, 2021
@bsiegert
Update go115 to 1.15.13.
35127b1 
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.
@pjediny pjediny mentioned this issue Jun 4, 2021
Closed
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jun 5, 2021
@bsiegert
Update go116 to 1.16.5.
f32db56 
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.
@ianlancetaylor ianlancetaylor mentioned this issue Jun 30, 2021
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/332549 mentions this issue: net: filter bad names from Lookup functions instead of hard failing

gopherbot pushed a commit that referenced this issue Jul 8, 2021
@rolandshoemaker
net: filter bad names from Lookup functions instead of hard failing
296ddf2 
Instead of hard failing on a single bad record, filter the bad records
and return anything valid. This only applies to the methods which can
return multiple records, LookupMX, LookupNS, LookupSRV, and LookupAddr.

When bad results are filtered out, also return an error, indicating
that this filtering has happened.

Updates #46241
Fixes #46979

Change-Id: I6493e0002beaf89f5a9795333a93605abd30d171
Reviewed-on: https://go-review.googlesource.com/c/go/+/332549
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/333330 mentions this issue: [release-branch.go1.16] net: filter bad names from Lookup functions instead of hard failing

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/333331 mentions this issue: [release-branch.go1.15] net: filter bad names from Lookup functions instead of hard failing

gopherbot pushed a commit that referenced this issue Jul 8, 2021
@rolandshoemaker
[release-branch.go1.16] net: filter bad names from Lookup functions i…
fb052db 
…nstead of hard failing

Instead of hard failing on a single bad record, filter the bad records
and return anything valid. This only applies to the methods which can
return multiple records, LookupMX, LookupNS, LookupSRV, and LookupAddr.

When bad results are filtered out, also return an error, indicating
that this filtering has happened.

Updates #46241
Updates #46979
Fixes #46999

Change-Id: I6493e0002beaf89f5a9795333a93605abd30d171
Reviewed-on: https://go-review.googlesource.com/c/go/+/332549
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 296ddf2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/333330
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jul 8, 2021
@rolandshoemaker
[release-branch.go1.15] net: filter bad names from Lookup functions i…
3d1d066 
…nstead of hard failing

Instead of hard failing on a single bad record, filter the bad records
and return anything valid. This only applies to the methods which can
return multiple records, LookupMX, LookupNS, LookupSRV, and LookupAddr.

When bad results are filtered out, also return an error, indicating
that this filtering has happened.

Updates #46241
Updates #46979
Fixes #47012

Change-Id: I6493e0002beaf89f5a9795333a93605abd30d171
Reviewed-on: https://go-review.googlesource.com/c/go/+/332549
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
(cherry picked from commit 296ddf2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/333331
Run-TryBot: Roland Shoemaker <roland@golang.org>
@golang golang locked and limited conversation to collaborators Jul 8, 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. 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
Milestone
Go1.17
Development

No branches or pull requests
4 participants
@dmitshur @rolandshoemaker @gopherbot @katiehockman