This is more of a net/url issue rather than net/http.

What does the RFC say regarding this ?

I believe that Parse doesn't try to interpret the hostname at all.

As shown in this recent article, this behavior could be used in server-side request forgery, local file inclusion and remote file inclusion vulnerabilities.

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

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.

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.

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.

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.

Related: #43389 ("net: limit the size of ParseIP input?")

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

One notable use of the stdlib methods is to validate API fields expected to contain IP / CIDR data. When stdlib methods change to reject data they previously accepted, that makes previously validated and persisted data turn invalid when checked by the same validation code.

While I agree with the goals of bounding input and ensuring data handled by diverse implementations is unambiguous, there should be a reasonable migration path for callers using stdlib methods for this purpose. What would we expect these callers to do before/after the proposed stdlib changes to move away from accepting 0-prefixed IP octets in new data, and to start detecting/migrating existing data?

Options that come to mind:

• Prevalidate IP/CIDR inputs to detect/reject leading 0's in addition to calling stdlib Parse methods? This seems like it reimplements bits of IP parsing, which is fragile/complex, especially for ipv6 addresses (which was why stdlib methods were used in the first place)
• Require normalized form (e.g. ParseIP().String())? This seems burdensome for ipv6 addresses, which are quite flexible and make many accommodations to simplify textual expression

Prevalidate IP/CIDR inputs to detect/reject leading 0's in addition to calling stdlib Parse methods? This seems like it reimplements bits of IP parsing, which is fragile/complex, especially for ipv6 addresses (which was why stdlib methods were used in the first place)

This seems fine to me if really needed. Instead of reimplementing those bits, though, just make a copy of the existing standard library routines. The license permits that.

The problem with "validate API fields" here is that if the validation disagrees with the eventual use, the validation is wrong. It's probably better for everyone involved to reject those than to mutually misunderstand them.

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?

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.

Change https://golang.org/cl/361534 mentions this issue: net/netip: don't accept ParseAddr with leading zeros