From: Brad Fitzpatrick Date: Wed, 24 Jun 2015 14:50:49 +0000 (+0200) Subject: net/url: validate ports in URLs and bytes after IPv6 literals X-Git-Tag: go1.5beta1~135 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=703166ea14e510a4aee805f956475570130f2df2;p=gostls13.git net/url: validate ports in URLs and bytes after IPv6 literals Fixes #11208 Change-Id: I35cc94129577b2a977fd35aafb0a5fb02c534a7c Reviewed-on: https://go-review.googlesource.com/11414 Reviewed-by: Dmitry Vyukov --- diff --git a/src/net/http/readrequest_test.go b/src/net/http/readrequest_test.go index eeb91e4a42..492d6d2aed 100644 --- a/src/net/http/readrequest_test.go +++ b/src/net/http/readrequest_test.go @@ -362,3 +362,11 @@ func TestReadRequest(t *testing.T) { } } } + +func TestReadRequest_BadConnectHost(t *testing.T) { + data := []byte("CONNECT []%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a HTTP/1.0\n\n") + r, err := ReadRequest(bufio.NewReader(bytes.NewReader(data))) + if err == nil { + t.Fatal("Got unexpected request = %#v", r) + } +} diff --git a/src/net/url/url.go b/src/net/url/url.go index c67a2fcf59..7eb5b7f176 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -9,6 +9,7 @@ package url import ( "bytes" "errors" + "fmt" "sort" "strconv" "strings" @@ -142,7 +143,7 @@ func unescape(s string, mode encoding) (string, error) { if i+2 >= len(s) || !ishex(s[i+1]) || !ishex(s[i+2]) { s = s[i:] if len(s) > 3 { - s = s[0:3] + s = s[:3] } return "", EscapeError(s) } @@ -328,7 +329,7 @@ func getscheme(rawurl string) (scheme, path string, err error) { if i == 0 { return "", "", errors.New("missing protocol scheme") } - return rawurl[0:i], rawurl[i+1:], nil + return rawurl[:i], rawurl[i+1:], nil default: // we have encountered an invalid character, // so there is no valid scheme @@ -347,9 +348,9 @@ func split(s string, c string, cutc bool) (string, string) { return s, "" } if cutc { - return s[0:i], s[i+len(c):] + return s[:i], s[i+len(c):] } - return s[0:i], s[i:] + return s[:i], s[i:] } // Parse parses rawurl into a URL structure. @@ -467,9 +468,11 @@ func parseAuthority(authority string) (user *Userinfo, host string, err error) { return user, host, nil } -// parseHost parses host as an authority without user information. +// parseHost parses host as an authority without user +// information. That is, as host[:port]. func parseHost(host string) (string, error) { litOrName := host + var colonPort string // ":80" or "" if strings.HasPrefix(host, "[") { // Parse an IP-Literal in RFC 3986 and RFC 6874. // E.g., "[fe80::1], "[fe80::1%25en0]" @@ -477,18 +480,23 @@ func parseHost(host string) (string, error) { // RFC 4007 defines "%" as a delimiter character in // the textual representation of IPv6 addresses. // Per RFC 6874, in URIs that "%" is encoded as "%25". - i := strings.LastIndex(host[1:], "]") + i := strings.LastIndex(host, "]") if i < 0 { return "", errors.New("missing ']' in host") } + colonPort = host[i+1:] // Parse a host subcomponent without a ZoneID in RFC // 6874 because the ZoneID is allowed to use the // percent encoded form. - j := strings.Index(host[1:1+i], "%25") + j := strings.Index(host[:i], "%25") if j < 0 { - litOrName = host[1 : 1+i] + litOrName = host[1:i] } else { - litOrName = host[1 : 1+j] + litOrName = host[1:j] + } + } else { + if i := strings.Index(host, ":"); i != -1 { + colonPort = host[i:] } } // A URI containing an IP-Literal without a ZoneID or @@ -503,6 +511,9 @@ func parseHost(host string) (string, error) { if strings.Contains(litOrName, "%") { return "", errors.New("percent-encoded characters in host") } + if !validOptionalPort(colonPort) { + return "", fmt.Errorf("invalid port %q after host", colonPort) + } var err error if host, err = unescape(host, encodeHost); err != nil { return "", err @@ -540,6 +551,23 @@ func validEncodedPath(s string) bool { return true } +// validOptionalPort reports whether port is either an empty string +// or matches /^:\d+$/ +func validOptionalPort(port string) bool { + if port == "" { + return true + } + if port[0] != ':' || len(port) == 1 { + return false + } + for _, b := range port[1:] { + if b < '0' || b > '9' { + return false + } + } + return true +} + // String reassembles the URL into a valid URL string. // The general form of the result is one of: // diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index 97ab7cc95a..31ef4c27c1 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -1074,6 +1074,39 @@ func TestParseFailure(t *testing.T) { } } +func TestParseAuthority(t *testing.T) { + tests := []struct { + in string + wantErr bool + }{ + {"http://[::1]", false}, + {"http://[::1]:80", false}, + {"http://[::1]:namedport", true}, // rfc3986 3.2.3 + {"http://[::1]/", false}, + {"http://[::1]a", true}, + {"http://[::1]%23", true}, + {"http://[::1%25en0]", false}, // valid zone id + {"http://[::1]:", true}, // colon, but no port + {"http://[::1]:%38%30", true}, // no hex in port + {"http://[::1%25%10]", false}, // TODO: reject the %10 after the valid zone %25 separator? + {"http://[%10::1]", true}, // no %xx escapes in IP address + {"http://[::1]/%48", false}, // %xx in path is fine + {"http://%41:8080/", true}, // TODO: arguably we should accept reg-name with %xx + } + for _, tt := range tests { + u, err := Parse(tt.in) + if tt.wantErr { + if err == nil { + t.Errorf("Parse(%q) = %#v; want an error", tt.in, u) + } + continue + } + if err != nil { + t.Logf("Parse(%q) = %v; want no error", tt.in, err) + } + } +} + type shouldEscapeTest struct { in byte mode encoding