Skip to content

Commit

Permalink
net: reject leading zeros in IP address parsers
Browse files Browse the repository at this point in the history
In both net.ParseIP and net.ParseCIDR reject leading zeros in the
dot-decimal notation of IPv4 addresses.

Fixes #30999
Fixes #43389

Change-Id: I2b6a31fe84db89ac828cf5ed03eaa586ee96ab68
Reviewed-on: https://go-review.googlesource.com/c/go/+/325829
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
  • Loading branch information
rolandshoemaker committed Jun 8, 2021
1 parent da4a640 commit d3e3d03
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 10 deletions.
10 changes: 10 additions & 0 deletions doc/go1.17.html
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,16 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
<a href="/pkg/net/#ParseError"><code>ParseError</code></a> error type now implement
the <a href="/pkg/net/#Error"><code>net.Error</code></a> interface.
</p>

<p><!-- CL325829 -->
The <a href="/pkg/net/#ParseIP"><code>ParseIP</code></a> and <a href="/pkg/net/#ParseCIDR"><code>ParseCIDR</code></a>
functions now reject IPv4 addresses which contain decimal components with leading zeros.

These components were always interpreted as decimal, but some operating systems treat them as octal.
This mismatch could hypothetically lead to security issues if a Go application was used to validate IP addresses
which were then used in their original form with non-Go applications which interpreted components as octal. Generally,
it is advisable to always re-encoded values after validation, which avoids this class of parser misalignment issues.

This comment has been minimized.

Copy link
@ianlancetaylor

ianlancetaylor Jun 10, 2021

Contributor

We use Gerritt for code review and mirror to GitHub. Very few people see comments on GitHub commits. If you want to comment on this change, please use https://golang.org/cl/325829. Thanks.

</p>
</dd>
</dl><!-- net -->

Expand Down
4 changes: 2 additions & 2 deletions src/net/hosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var lookupStaticHostTests = []struct {
},
},
{
"testdata/ipv4-hosts", // see golang.org/issue/8996
"testdata/ipv4-hosts",
[]staticHostEntry{
{"localhost", []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"}},
{"localhost.localdomain", []string{"127.0.0.3"}},
Expand Down Expand Up @@ -102,7 +102,7 @@ var lookupStaticAddrTests = []struct {
},
},
{
"testdata/ipv4-hosts", // see golang.org/issue/8996
"testdata/ipv4-hosts",
[]staticHostEntry{
{"127.0.0.1", []string{"localhost"}},
{"127.0.0.2", []string{"localhost"}},
Expand Down
4 changes: 4 additions & 0 deletions src/net/ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,10 @@ func parseIPv4(s string) IP {
if !ok || n > 0xFF {
return nil
}
if c > 1 && s[0] == '0' {
// Reject non-zero components with leading zeroes.
return nil
}
s = s[c:]
p[i] = byte(n)
}
Expand Down
8 changes: 6 additions & 2 deletions src/net/ip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ var parseIPTests = []struct {
}{
{"127.0.1.2", IPv4(127, 0, 1, 2)},
{"127.0.0.1", IPv4(127, 0, 0, 1)},
{"127.001.002.003", IPv4(127, 1, 2, 3)},
{"::ffff:127.1.2.3", IPv4(127, 1, 2, 3)},
{"::ffff:127.001.002.003", IPv4(127, 1, 2, 3)},
{"::ffff:7f01:0203", IPv4(127, 1, 2, 3)},
{"0:0:0:0:0000:ffff:127.1.2.3", IPv4(127, 1, 2, 3)},
{"0:0:0:0:000000:ffff:127.1.2.3", IPv4(127, 1, 2, 3)},
Expand All @@ -43,6 +41,11 @@ var parseIPTests = []struct {
{"fe80::1%911", nil},
{"", nil},
{"a1:a2:a3:a4::b1:b2:b3:b4", nil}, // Issue 6628
{"127.001.002.003", nil},
{"::ffff:127.001.002.003", nil},
{"123.000.000.000", nil},
{"1.2..4", nil},
{"0123.0.0.1", nil},
}

func TestParseIP(t *testing.T) {
Expand Down Expand Up @@ -358,6 +361,7 @@ var parseCIDRTests = []struct {
{"0.0.-2.0/32", nil, nil, &ParseError{Type: "CIDR address", Text: "0.0.-2.0/32"}},
{"0.0.0.-3/32", nil, nil, &ParseError{Type: "CIDR address", Text: "0.0.0.-3/32"}},
{"0.0.0.0/-0", nil, nil, &ParseError{Type: "CIDR address", Text: "0.0.0.0/-0"}},
{"127.000.000.001/32", nil, nil, &ParseError{Type: "CIDR address", Text: "127.000.000.001/32"}},
{"", nil, nil, &ParseError{Type: "CIDR address", Text: ""}},
}

Expand Down
8 changes: 2 additions & 6 deletions src/net/testdata/ipv4-hosts
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
# See https://tools.ietf.org/html/rfc1123.
#
# The literal IPv4 address parser in the net package is a relaxed
# one. It may accept a literal IPv4 address in dotted-decimal notation
# with leading zeros such as "001.2.003.4".

# internet address and host name
127.0.0.1 localhost # inline comment separated by tab
127.000.000.002 localhost # inline comment separated by space
127.0.0.2 localhost # inline comment separated by space

# internet address, host name and aliases
127.000.000.003 localhost localhost.localdomain
127.0.0.3 localhost localhost.localdomain

0 comments on commit d3e3d03

Please sign in to comment.