net/http: do not send malformed cookie domain attribute

Malformed domain attributes are not sent in a Set-Cookie header.
Instead the domain attribute is dropped which turns the cookie
into a host-only cookie. This is much safer than dropping characters
from domain attribute.

Domain attributes with a leading dot '.' are still allowed, even
if discouraged by RFC 6265 section 4.1.1.

Fixes #6013

R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/12745043

diff --git a/src/pkg/net/http/cookie.go b/src/pkg/net/http/cookie.go
index 540a8f7a9a18c19ada3580409c65481ced2325af..2074c149c268e42f15ba70541c21e0071f68849c 100644 (file)
--- a/src/pkg/net/http/cookie.go
+++ b/src/pkg/net/http/cookie.go
@@ -8,6 +8,7 @@ import (
        "bytes"
        "fmt"
        "log"
+       "net"
        "strconv"
        "strings"
        "time"
@@ -145,7 +146,15 @@ func (c *Cookie) String() string {
                fmt.Fprintf(&b, "; Path=%s", sanitizeCookiePath(c.Path))
        }
        if len(c.Domain) > 0 {
-               fmt.Fprintf(&b, "; Domain=%s", sanitizeCookieDomain(c.Domain))
+               if validCookieDomain(c.Domain) {
+                       // A c.Domain containing illegal characters is not
+                       // sanitized but simply dropped which turns the cookie
+                       // into a host-only cookie.
+                       fmt.Fprintf(&b, "; Domain=%s", c.Domain)
+               } else {
+                       log.Printf("net/http: invalid Cookie.Domain %q; dropping domain attribute",
+                               c.Domain)
+               }
        }
        if c.Expires.Unix() > 0 {
                fmt.Fprintf(&b, "; Expires=%s", c.Expires.UTC().Format(time.RFC1123))
@@ -208,26 +217,78 @@ func readCookies(h Header, filter string) []*Cookie {
        return cookies
 }
 
-var cookieNameSanitizer = strings.NewReplacer("\n", "-", "\r", "-")
+// validCookieDomain returns wheter v is a valid cookie domain-value.
+func validCookieDomain(v string) bool {
+       if isCookieDomainName(v) {
+               return true
+       }
+       if net.ParseIP(v) != nil && !strings.Contains(v, ":") {
+               return true
+       }
+       return false
+}
 
-// http://tools.ietf.org/html/rfc6265#section-4.1.1
-// domain-av         = "Domain=" domain-value
-// domain-value      = <subdomain>
-//     ; defined in [RFC1034], Section 3.5, as
-//      ; enhanced by [RFC1123], Section 2.1
-func sanitizeCookieDomain(v string) string {
-       // TODO: implement http://tools.ietf.org/html/rfc1034#section-3.5
-       return oldCookieValueSanitizer.Replace(v)
+// isCookieDomainName returns whether s is a valid domain name or a valid
+// domain name with a leading dot '.'.  It is almost a direct copy of
+// package net's isDomainName.
+func isCookieDomainName(s string) bool {
+       if len(s) == 0 {
+               return false
+       }
+       if len(s) > 255 {
+               return false
+       }
+
+       if s[0] == '.' {
+               // A cookie a domain attribute may start with a leading dot.
+               s = s[1:]
+       }
+       last := byte('.')
+       ok := false // Ok once we've seen a letter.
+       partlen := 0
+       for i := 0; i < len(s); i++ {
+               c := s[i]
+               switch {
+               default:
+                       return false
+               case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z':
+                       // No '_' allowed here (in contrast to package net).
+                       ok = true
+                       partlen++
+               case '0' <= c && c <= '9':
+                       // fine
+                       partlen++
+               case c == '-':
+                       // Byte before dash cannot be dot.
+                       if last == '.' {
+                               return false
+                       }
+                       partlen++
+               case c == '.':
+                       // Byte before dot cannot be dot, dash.
+                       if last == '.' || last == '-' {
+                               return false
+                       }
+                       if partlen > 63 || partlen == 0 {
+                               return false
+                       }
+                       partlen = 0
+               }
+               last = c
+       }
+       if last == '-' || partlen > 63 {
+               return false
+       }
+
+       return ok
 }
 
+var cookieNameSanitizer = strings.NewReplacer("\n", "-", "\r", "-")
+
 func sanitizeCookieName(n string) string {
        return cookieNameSanitizer.Replace(n)
 }
 
-// This is the replacer used in the original Go cookie code.
-// It's not correct, but it's here for now until it's replaced.
-var oldCookieValueSanitizer = strings.NewReplacer("\n", " ", "\r", " ", ";", " ")
-
 // http://tools.ietf.org/html/rfc6265#section-4.1.1
 // cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
 // cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E

diff --git a/src/pkg/net/http/cookie_test.go b/src/pkg/net/http/cookie_test.go
index 0e68ff05d1cdccb9b644dfacc70beded707a3e03..7a4827cb6b4792b0d34a7e79fc8d51821fe4ea5b 100644 (file)
--- a/src/pkg/net/http/cookie_test.go
+++ b/src/pkg/net/http/cookie_test.go
@@ -32,6 +32,22 @@ var writeSetCookiesTests = []struct {
                &Cookie{Name: "cookie-4", Value: "four", Path: "/restricted/"},
                "cookie-4=four; Path=/restricted/",
        },
+       {
+               &Cookie{Name: "cookie-5", Value: "five", Domain: "wrong;bad.abc"},
+               "cookie-5=five",
+       },
+       {
+               &Cookie{Name: "cookie-6", Value: "six", Domain: "bad-.abc"},
+               "cookie-6=six",
+       },
+       {
+               &Cookie{Name: "cookie-7", Value: "seven", Domain: "127.0.0.1"},
+               "cookie-7=seven; Domain=127.0.0.1",
+       },
+       {
+               &Cookie{Name: "cookie-8", Value: "eight", Domain: "::1"},
+               "cookie-8=eight",
+       },
 }
 
 func TestWriteSetCookies(t *testing.T) {