net: reject leading zeros in IP address parsers

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>

diff --git a/doc/go1.17.html b/doc/go1.17.html
index 3a1b43a4e51c5071f79991423c0508d3dd18a288..56f88e67245585183eed0391e0fafa53d16fb4a1 100644 (file)
--- a/doc/go1.17.html
+++ b/doc/go1.17.html
@@ -639,6 +639,16 @@ Do not send CLs removing the interior tags from such phrases.
       <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.
+    </p>
   </dd>
 </dl><!-- net -->
 

diff --git a/src/net/hosts_test.go b/src/net/hosts_test.go
index f850e2fccfd2baf03e9be8ee8ba02dbdd5555a09..19c43999f9291ca77a13053c92b7efedc1c8e95e 100644 (file)
--- a/src/net/hosts_test.go
+++ b/src/net/hosts_test.go
@@ -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"}},
@@ -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"}},

diff --git a/src/net/ip.go b/src/net/ip.go
index 047726976184d904b325470323963341b84d86e5..38e1aa2247f5c329d9c247de420b2a58ecc8a25b 100644 (file)
--- a/src/net/ip.go
+++ b/src/net/ip.go
@@ -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)
        }

diff --git a/src/net/ip_test.go b/src/net/ip_test.go
index 3af5e41ceb4427d4770448677a1da82a5d8d8adb..5bbda6024dc3d9dd54189eb202afcb5cda572ad7 100644 (file)
--- a/src/net/ip_test.go
+++ b/src/net/ip_test.go
@@ -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)},
@@ -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) {
@@ -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: ""}},
 }
 

diff --git a/src/net/testdata/ipv4-hosts b/src/net/testdata/ipv4-hosts
index 5208bb44ac863dd418b429a54903d8c802fb05e4..6b99675dfc884cda1dc312663ccfdd9423b8659b 100644 (file)
--- a/src/net/testdata/ipv4-hosts
+++ b/src/net/testdata/ipv4-hosts
@@ -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