exp/template/html: tighten rules on dynamic attr names.

R=nigeltao
CC=golang-dev
https://golang.org/cl/5076049

diff --git a/src/pkg/exp/template/html/Makefile b/src/pkg/exp/template/html/Makefile
index 9032aead8700aba80646a781cba5a73f1d3321c4..2ccbdd3e85af543f1eff8a1d9c5b26956a8a8a69 100644 (file)
--- a/src/pkg/exp/template/html/Makefile
+++ b/src/pkg/exp/template/html/Makefile
@@ -6,6 +6,7 @@ include ../../../../Make.inc
 
 TARG=exp/template/html
 GOFILES=\
+       attr.go\
        clone.go\
        content.go\
        context.go\

diff --git a/src/pkg/exp/template/html/attr.go b/src/pkg/exp/template/html/attr.go
new file mode 100644 (file)
index 0000000..cc57f8b
--- /dev/null
+++ b/src/pkg/exp/template/html/attr.go
@@ -0,0 +1,184 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package html
+
+// attrType[n] describes the value of the given attribute.
+// If an attribute affects (or can mask) the encoding or interpretation of
+// other content, or affects the contents, idempotency, or credentials of a
+// network message, then the value in this map is contentTypeUnsafe.
+// This map is derived from HTML5, specifically
+// http://www.w3.org/TR/html5/Overview.html#attributes-1 and
+// http://www.w3.org/TR/html5/Overview.html#event-handlers-on-elements-document-objects-and-window-objects
+// as well as "%URI"-typed attributes from
+// http://www.w3.org/TR/html4/index/attributes.html
+var attrType = map[string]contentType{
+       "accept":             contentTypePlain,
+       "accept-charset":     contentTypeUnsafe,
+       "action":             contentTypeURL,
+       "alt":                contentTypePlain,
+       "archive":            contentTypeURL,
+       "async":              contentTypeUnsafe,
+       "autocomplete":       contentTypePlain,
+       "autofocus":          contentTypePlain,
+       "autoplay":           contentTypePlain,
+       "background":         contentTypeURL,
+       "border":             contentTypePlain,
+       "checked":            contentTypePlain,
+       "cite":               contentTypeURL,
+       "challenge":          contentTypeUnsafe,
+       "charset":            contentTypeUnsafe,
+       "class":              contentTypePlain,
+       "classid":            contentTypeURL,
+       "codebase":           contentTypeURL,
+       "cols":               contentTypePlain,
+       "colspan":            contentTypePlain,
+       "content":            contentTypeUnsafe,
+       "contenteditable":    contentTypePlain,
+       "contextmenu":        contentTypePlain,
+       "controls":           contentTypePlain,
+       "coords":             contentTypePlain,
+       "crossorigin":        contentTypeUnsafe,
+       "data":               contentTypeURL,
+       "datetime":           contentTypePlain,
+       "default":            contentTypePlain,
+       "defer":              contentTypeUnsafe,
+       "dir":                contentTypePlain,
+       "dirname":            contentTypePlain,
+       "disabled":           contentTypePlain,
+       "draggable":          contentTypePlain,
+       "dropzone":           contentTypePlain,
+       "enctype":            contentTypeUnsafe,
+       "for":                contentTypePlain,
+       "form":               contentTypeUnsafe,
+       "formaction":         contentTypeURL,
+       "formenctype":        contentTypeUnsafe,
+       "formmethod":         contentTypeUnsafe,
+       "formnovalidate":     contentTypeUnsafe,
+       "formtarget":         contentTypePlain,
+       "headers":            contentTypePlain,
+       "height":             contentTypePlain,
+       "hidden":             contentTypePlain,
+       "high":               contentTypePlain,
+       "href":               contentTypeURL,
+       "hreflang":           contentTypePlain,
+       "http-equiv":         contentTypeUnsafe,
+       "icon":               contentTypeURL,
+       "id":                 contentTypePlain,
+       "ismap":              contentTypePlain,
+       "keytype":            contentTypeUnsafe,
+       "kind":               contentTypePlain,
+       "label":              contentTypePlain,
+       "lang":               contentTypePlain,
+       "language":           contentTypeUnsafe,
+       "list":               contentTypePlain,
+       "longdesc":           contentTypeURL,
+       "loop":               contentTypePlain,
+       "low":                contentTypePlain,
+       "manifest":           contentTypeURL,
+       "max":                contentTypePlain,
+       "maxlength":          contentTypePlain,
+       "media":              contentTypePlain,
+       "mediagroup":         contentTypePlain,
+       "method":             contentTypeUnsafe,
+       "min":                contentTypePlain,
+       "multiple":           contentTypePlain,
+       "name":               contentTypePlain,
+       "novalidate":         contentTypeUnsafe,
+       "onabort":            contentTypeJS,
+       "onblur":             contentTypeJS,
+       "oncanplay":          contentTypeJS,
+       "oncanplaythrough":   contentTypeJS,
+       "onchange":           contentTypeJS,
+       "onclick":            contentTypeJS,
+       "oncontextmenu":      contentTypeJS,
+       "oncuechange":        contentTypeJS,
+       "ondblclick":         contentTypeJS,
+       "ondrag":             contentTypeJS,
+       "ondragend":          contentTypeJS,
+       "ondragenter":        contentTypeJS,
+       "ondragleave":        contentTypeJS,
+       "ondragover":         contentTypeJS,
+       "ondragstart":        contentTypeJS,
+       "ondrop":             contentTypeJS,
+       "ondurationchange":   contentTypeJS,
+       "onemptied":          contentTypeJS,
+       "onended":            contentTypeJS,
+       "onerror":            contentTypeJS,
+       "onfocus":            contentTypeJS,
+       "oninput":            contentTypeJS,
+       "oninvalid":          contentTypeJS,
+       "onkeydown":          contentTypeJS,
+       "onkeypress":         contentTypeJS,
+       "onkeyup":            contentTypeJS,
+       "onload":             contentTypeJS,
+       "onloadeddata":       contentTypeJS,
+       "onloadedmetadata":   contentTypeJS,
+       "onloadstart":        contentTypeJS,
+       "onmousedown":        contentTypeJS,
+       "onmousemove":        contentTypeJS,
+       "onmouseout":         contentTypeJS,
+       "onmouseover":        contentTypeJS,
+       "onmouseup":          contentTypeJS,
+       "onmousewheel":       contentTypeJS,
+       "onpause":            contentTypeJS,
+       "onplay":             contentTypeJS,
+       "onplaying":          contentTypeJS,
+       "onprogress":         contentTypeJS,
+       "onratechange":       contentTypeJS,
+       "onreadystatechange": contentTypeJS,
+       "onreset":            contentTypeJS,
+       "onscroll":           contentTypeJS,
+       "onseeked":           contentTypeJS,
+       "onseeking":          contentTypeJS,
+       "onselect":           contentTypeJS,
+       "onshow":             contentTypeJS,
+       "onstalled":          contentTypeJS,
+       "onsubmit":           contentTypeJS,
+       "onsuspend":          contentTypeJS,
+       "ontimeupdate":       contentTypeJS,
+       "onvolumechange":     contentTypeJS,
+       "onwaiting":          contentTypeJS,
+       "open":               contentTypePlain,
+       "optimum":            contentTypePlain,
+       "pattern":            contentTypeUnsafe,
+       "placeholder":        contentTypePlain,
+       "poster":             contentTypeURL,
+       "profile":            contentTypeURL,
+       "preload":            contentTypePlain,
+       "pubdate":            contentTypePlain,
+       "radiogroup":         contentTypePlain,
+       "readonly":           contentTypePlain,
+       "rel":                contentTypeUnsafe,
+       "required":           contentTypePlain,
+       "reversed":           contentTypePlain,
+       "rows":               contentTypePlain,
+       "rowspan":            contentTypePlain,
+       "sandbox":            contentTypeUnsafe,
+       "spellcheck":         contentTypePlain,
+       "scope":              contentTypePlain,
+       "scoped":             contentTypePlain,
+       "seamless":           contentTypePlain,
+       "selected":           contentTypePlain,
+       "shape":              contentTypePlain,
+       "size":               contentTypePlain,
+       "sizes":              contentTypePlain,
+       "span":               contentTypePlain,
+       "src":                contentTypeURL,
+       "srcdoc":             contentTypeHTML,
+       "srclang":            contentTypePlain,
+       "start":              contentTypePlain,
+       "step":               contentTypePlain,
+       "style":              contentTypeCSS,
+       "tabindex":           contentTypePlain,
+       "target":             contentTypePlain,
+       "title":              contentTypePlain,
+       "type":               contentTypeUnsafe,
+       "usemap":             contentTypeURL,
+       "value":              contentTypeUnsafe,
+       "width":              contentTypePlain,
+       "wrap":               contentTypePlain,
+
+       // TODO: data-* attrs?  Recognize data-foo-url and similar.
+}

diff --git a/src/pkg/exp/template/html/content.go b/src/pkg/exp/template/html/content.go
index 8b9809b982e5883feb8320de825712e2c10b0b1d..dcaff8c15c638adc08ba8c29f2aae6e551a4d645 100644 (file)
--- a/src/pkg/exp/template/html/content.go
+++ b/src/pkg/exp/template/html/content.go
@@ -64,6 +64,10 @@ const (
        contentTypeJS
        contentTypeJSStr
        contentTypeURL
+       // contentTypeUnsafe is used in attr.go for values that affect how
+       // embedded content and network messages are formed, vetted,
+       // or interpreted; or which credentials network messages carry.
+       contentTypeUnsafe
 )
 
 // stringify converts its arguments to a string and the type of the content.

diff --git a/src/pkg/exp/template/html/escape_test.go b/src/pkg/exp/template/html/escape_test.go
index 8a64515dec972ae1608931797b9e235b98678cb1..1ce66c5fb19fd79f1ff134ee395028cd806d4ce2 100644 (file)
--- a/src/pkg/exp/template/html/escape_test.go
+++ b/src/pkg/exp/template/html/escape_test.go
@@ -553,11 +553,51 @@ func TestEscape(t *testing.T) {
                        // Treated as JS since quotes are inserted.
                        `<img onload="alert(&#34;loaded&#34;)">`,
                },
+               {
+                       "bad dynamic attribute name 1",
+                       // Allow checked, selected, disabled, but not JS or
+                       // CSS attributes.
+                       `<input {{"onchange"}}="{{"doEvil()"}}">`,
+                       `<input ZgotmplZ="doEvil()">`,
+               },
+               {
+                       "bad dynamic attribute name 2",
+                       `<div {{"sTyle"}}="{{"color: expression(alert(1337))"}}">`,
+                       `<div ZgotmplZ="color: expression(alert(1337))">`,
+               },
+               {
+                       "bad dynamic attribute name 3",
+                       // Allow title or alt, but not a URL.
+                       `<img {{"src"}}="{{"javascript:doEvil()"}}">`,
+                       `<img ZgotmplZ="javascript:doEvil()">`,
+               },
+               {
+                       "bad dynamic attribute name 4",
+                       // Structure preservation requires values to associate
+                       // with a consistent attribute.
+                       `<input checked {{""}}="Whose value am I?">`,
+                       `<input checked ZgotmplZ="Whose value am I?">`,
+               },
                {
                        "dynamic element name",
                        `<h{{3}}><table><t{{"head"}}>...</h{{3}}>`,
                        `<h3><table><thead>...</h3>`,
                },
+               {
+                       "bad dynamic element name",
+                       // Dynamic element names are typically used to switch
+                       // between (thead, tfoot, tbody), (ul, ol), (th, td),
+                       // and other replaceable sets.
+                       // We do not currently easily support (ul, ol).
+                       // If we do change to support that, this test should
+                       // catch failures to filter out special tag names which
+                       // would violate the structure preservation property --
+                       // if any special tag name could be substituted, then
+                       // the content could be raw text/RCDATA for some inputs
+                       // and regular HTML content for others.
+                       `<{{"script"}}>{{"doEvil()"}}</{{"script"}}>`,
+                       `&lt;script>doEvil()&lt;/script>`,
+               },
        }
 
        for _, test := range tests {

diff --git a/src/pkg/exp/template/html/html.go b/src/pkg/exp/template/html/html.go
index 3924b193db0a19f7f8096afddf3a5d2123079fcd..6ef66dd6c3ca74da17eff73eaaae6c16f2c6854d 100644 (file)
--- a/src/pkg/exp/template/html/html.go
+++ b/src/pkg/exp/template/html/html.go
@@ -7,6 +7,7 @@ package html
 import (
        "bytes"
        "fmt"
+       "strings"
        "utf8"
 )
 
@@ -220,10 +221,23 @@ func htmlNameFilter(args ...interface{}) string {
        if t == contentTypeHTMLAttr {
                return s
        }
+       if len(s) == 0 {
+               // Avoid violation of structure preservation.
+               // <input checked {{.K}}={{.V}}>.
+               // Without this, if .K is empty then .V is the value of
+               // checked, but otherwise .V is the value of the attribute
+               // named .K.
+               return filterFailsafe
+       }
+       s = strings.ToLower(s)
+       if t := attrType[s]; t != contentTypePlain && attrType["on"+s] != contentTypeJS {
+               // TODO: Split attr and element name part filters so we can whitelist
+               // attributes.
+               return filterFailsafe
+       }
        for _, r := range s {
                switch {
                case '0' <= r && r <= '9':
-               case 'A' <= r && r <= 'Z':
                case 'a' <= r && r <= 'z':
                default:
                        return filterFailsafe

diff --git a/src/pkg/exp/template/html/transition.go b/src/pkg/exp/template/html/transition.go
index 3be3a01a8aecc62f2528fa801f811405c93abd2a..dd8cd59a6f5af8accfbf26d381c8eb9065047c4c 100644 (file)
--- a/src/pkg/exp/template/html/transition.go
+++ b/src/pkg/exp/template/html/transition.go
@@ -105,12 +105,17 @@ func tTag(c context, s []byte) (context, int) {
        state, attr := stateTag, attrNone
        if i != j {
                canonAttrName := strings.ToLower(string(s[i:j]))
-               if urlAttr[canonAttrName] {
+               switch attrType[canonAttrName] {
+               case contentTypeURL:
                        attr = attrURL
-               } else if strings.HasPrefix(canonAttrName, "on") {
-                       attr = attrScript
-               } else if canonAttrName == "style" {
+               case contentTypeCSS:
                        attr = attrStyle
+               case contentTypeJS:
+                       attr = attrScript
+               default:
+                       if strings.HasPrefix(canonAttrName, "on") {
+                               attr = attrScript
+                       }
                }
                if j == len(s) {
                        state = stateAttrName
@@ -532,29 +537,3 @@ func eatWhiteSpace(s []byte, i int) int {
        }
        return len(s)
 }
-
-// urlAttr is the set of attribute names whose values are URLs.
-// It consists of all "%URI"-typed attributes from
-// http://www.w3.org/TR/html4/index/attributes.html
-// as well as those attributes defined at
-// http://dev.w3.org/html5/spec/index.html#attributes-1
-// whose Value column in that table matches
-// "Valid [non-empty] URL potentially surrounded by spaces".
-var urlAttr = map[string]bool{
-       "action":     true,
-       "archive":    true,
-       "background": true,
-       "cite":       true,
-       "classid":    true,
-       "codebase":   true,
-       "data":       true,
-       "formaction": true,
-       "href":       true,
-       "icon":       true,
-       "longdesc":   true,
-       "manifest":   true,
-       "poster":     true,
-       "profile":    true,
-       "src":        true,
-       "usemap":     true,
-}