go: net: reject leading zeros in IP address parsers [freeze exception]

We will not be backporting this issue. We are treating the change as a robustness improvement and not a security fix due to its potential for breaking working use cases.

The situation is not nearly so clear cut as the advocates of CVE-2021-29923 would have people believe. They present it as a bug, plain and simple, not to treat leading zeros in IP addresses as indicating octal numbers, but that’s not obvious. The BSD TCP/IP stack introduced the octal parsing, perhaps even accidentally, and because BSD is the most commonly used code, that interpretation is also the most common one. But it’s not the only interpretation. In fact, the earliest IP RFCs directly contradict the BSD implementation - they are pretty clear that IP addresses with leading zeros are meant to be interpreted as decimal.

Furthermore, the claimed vulnerability is like a TOCTTOU problem where the check and use are handled by different software with differing interpretation of leading zeros. The right fix is, as it always is, to put the check and use together.

Rejecting the leading zeros entirely avoids resolving the radix ambiguity the wrong way, which improves robustness. But it can also break existing code that might be processing config files that contain leading zeros and were happy with the radix-10 interpretation.

Given that

1. the right fix for any security consequence does not involve this change at all (the right fix is to place the check and use in the same program), and that
2. the Go behavior is entirely valid according to some RFCs, and that
3. the change has a very real possibility of breaking existing, valid, working use cases,

we chose to make the change only in a new major version, as a robustness fix, rather than treat it as an urgent security fix that would require backporting. We do not want to push a breaking change that will keep people from being able to pick up critical Go 1.16 patches later.

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.

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

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.

See also https://man7.org/linux/man-pages/man3/inet_pton.3.html which does not accept zero-prefixed IPs.

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}}

Thank you for making this freeze exception request. It is approved.

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

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 find this pretty convincing, especially given that net.Dial and net.Listen will parse the IPs as decimal.

To end up vulnerable due to this mismatch, an application would have to parse the IP with Go, reject any hostnames, apply security-relevant logic to the return value, and then pass the input (not the encoding of the return value) to a different, non-Go application which is happy to parse the IP as octal.

Generally, this is another instance where relying on parser alignment instead of re-encoding outputs is a fragile design.

We are not aware of any application for which this leads to a security issue, if anyone does please let us know at security@golang.org as that would help evaluate whether to backport the fix.

In any case, I definitely agree we should just consider these inputs invalid in Go 1.17.

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?

@benjsmi Go1.15 is unsupported, since Go1.16 and Go1.17 have been released. https://golang.org/doc/devel/release#policy

Change https://golang.org/cl/325829 mentions this issue: net: reject leading zeros in IP address parsers

I’d like to request a freeze exception for this fix. We deemed it not a backportable security fix (also because of the potential for disruption and the limited real-world impact), but it’s conceivable that it could have security value for some applications, so we’d like not to wait for Go 1.18.

/cc @golang/release @rsc

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

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.