You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
net: reject leading zeros in IP address parsers [freeze exception] #30999
changed the title [-]net/http: octal literals in IP addresses are interpreted as decimal ones[/-][+]net/url: Parse interprets octal literals in IP addresses as decimal ones[/+]on Mar 22, 2019
As shown in this recent article, this behavior could be used in server-side request forgery, local file inclusion and remote file inclusion vulnerabilities.
All the forms except for decimal octets are seen as non-standard (despite being quite widely interoperable) and undesirable.
I'd argue Go net.ParseIP/ParseCIDR etc should return an error on zero-prefixed input. It avoids ambiguity and since Go has historically parsed them differently than BSD, an error is a safer change in behavior than silently giving different results.
And as I discussed with @secenv on IRC, that article is naive. Typical "attacks" that 0127.0.0.1 enables are enabled also by evil.example.com A 127.0.0.1 in DNS, and the fix for both is to check the target IP after resolving, basically &http.Client{Transport: &http.Transport{DialContext: dialOnlySafeIPs}}
I forgot to add that it is indeed an issue that affects net.ParseCIDR https://play.golang.org/p/HpWqhr9tZ53 . I agree with @tv42, those functions should return errors. The documentation should at least warn the developer.
Guess I should mention @FiloSottile for further discussion on the security impact of this issue.
Since Go doesn't use the "loose" syntax of RFC6943, it's non-conforming already. Rejecting non-dotted-decimal inputs would make Go use the "strict" syntax.
I agree about changing Go's IP address parsers
(ParseIP, ParseCIDR, any others) to reject leading zeros (except "0"),
because:
(1) the RFCs are mostly quiet but in a few places hint that decimal is the right interpretation,
(2) Go interprets leading zeros as decimal, and
(3) BSD stacks nonetheless interpret leading zeros as octal.
(4) The fact that basically no one has noticed this divergence implies
that essentially no one uses leading zeros in IP addresses.
It seems like an open question whether this should be done
in a point release or saved for the next major release (Go 1.17).
But to start, we should agree to do it at all.
Adding to the proposal process.
Reacted by Matthew Dempsky, Ainar Garipov, antichris, Colin Arnott and 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко)
Activity
[-]net/http: octal literals in IP addresses are interpreted as decimal ones[/-][+]net/url: Parse interprets octal literals in IP addresses as decimal ones[/+]agnivade commentedon Mar 22, 2019
This is more of a net/url issue rather than net/http.
What does the RFC say regarding this ?
secenv commentedon Mar 31, 2021
As shown in this recent article, this behavior could be used in server-side request forgery, local file inclusion and remote file inclusion vulnerabilities.
tv42 commentedon Mar 31, 2021
There is no real RFC on textual IP address representation. The best we have is https://tools.ietf.org/html/draft-main-ipaddr-text-rep-02 which says
I'd argue Go net.ParseIP/ParseCIDR etc should return an error on zero-prefixed input. It avoids ambiguity and since Go has historically parsed them differently than BSD, an error is a safer change in behavior than silently giving different results.
See also https://man7.org/linux/man-pages/man3/inet_pton.3.html which does not accept zero-prefixed IPs.
tv42 commentedon Mar 31, 2021
And as I discussed with @secenv on IRC, that article is naive. Typical "attacks" that 0127.0.0.1 enables are enabled also by
evil.example.com A 127.0.0.1in DNS, and the fix for both is to check the target IP after resolving, basically&http.Client{Transport: &http.Transport{DialContext: dialOnlySafeIPs}}secenv commentedon Mar 31, 2021
I forgot to add that it is indeed an issue that affects net.ParseCIDR https://play.golang.org/p/HpWqhr9tZ53 . I agree with @tv42, those functions should return errors. The documentation should at least warn the developer.
Guess I should mention @FiloSottile for further discussion on the security impact of this issue.
tv42 commentedon Mar 31, 2021
I found a more authoritative RFC on IP address textual representation -- although it's only Informational not Standards Track: https://tools.ietf.org/html/rfc6943#section-3.1.1
Since Go doesn't use the "loose" syntax of RFC6943, it's non-conforming already. Rejecting non-dotted-decimal inputs would make Go use the "strict" syntax.
rsc commentedon Apr 5, 2021
I agree about changing Go's IP address parsers
(ParseIP, ParseCIDR, any others) to reject leading zeros (except "0"),
because:
(1) the RFCs are mostly quiet but in a few places hint that decimal is the right interpretation,
(2) Go interprets leading zeros as decimal, and
(3) BSD stacks nonetheless interpret leading zeros as octal.
(4) The fact that basically no one has noticed this divergence implies
that essentially no one uses leading zeros in IP addresses.
It seems like an open question whether this should be done
in a point release or saved for the next major release (Go 1.17).
But to start, we should agree to do it at all.
Adding to the proposal process.
54 remaining items
benjsmi commentedon Sep 22, 2021
So I'm not seeing this issue specifically mentioned in the Go 1.16 release notes -- is CVE-2021-29923 addressed in Go 1.16.x? And if so, which x?
ianlancetaylor commentedon Sep 22, 2021
The net.ParseIP function rejects IPv4 addresses that contain decimal components with leading zeros in Go 1.17 but not in Go 1.16.
Per #30999 (comment) we do not plan to backport this change to Go 1.16.
gopherbot commentedon Nov 5, 2021
Change https://golang.org/cl/361534 mentions this issue:
net/netip: don't accept ParseAddr with leading zerosnet/netip: don't accept ParseAddr with leading zeros
Remove broken IP address parse test
Remove broken IP address parse test
Remove broken IP address parse test
go: Exclude CVE-2021-29923 from report list
net: reject leading zeros in IP address parsers