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

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

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\

src/pkg/exp/template/html/attr.go

+// 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.
+}

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.

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 {

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

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,
-}