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 9032aea..2ccbdd3 100644
--- a/src/pkg/exp/template/html/Makefile
+++ b/src/pkg/exp/template/html/Makefile
@@ -6,6 +6,7 @@
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
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 8b9809b..dcaff8c 100644
--- a/src/pkg/exp/template/html/content.go
+++ b/src/pkg/exp/template/html/content.go
@@ -64,6 +64,10 @@
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 8a64515..1ce66c5 100644
--- a/src/pkg/exp/template/html/escape_test.go
+++ b/src/pkg/exp/template/html/escape_test.go
@@ -554,10 +554,50 @@
`<img onload="alert("loaded")">`,
},
{
+ "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"}}>`,
+ `<script>doEvil()</script>`,
+ },
}
for _, test := range tests {
diff --git a/src/pkg/exp/template/html/html.go b/src/pkg/exp/template/html/html.go
index 3924b19..6ef66dd 100644
--- a/src/pkg/exp/template/html/html.go
+++ b/src/pkg/exp/template/html/html.go
@@ -7,6 +7,7 @@
import (
"bytes"
"fmt"
+ "strings"
"utf8"
)
@@ -220,10 +221,23 @@
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 3be3a01..dd8cd59 100644
--- a/src/pkg/exp/template/html/transition.go
+++ b/src/pkg/exp/template/html/transition.go
@@ -105,12 +105,17 @@
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 @@
}
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,
-}
