From: Mike Samuel Date: Sun, 19 Nov 2017 21:37:07 +0000 (+0900) Subject: html/template: add srcset content type X-Git-Tag: go1.10beta2~106 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c0cda71dab6785c4b7a400b79796b23affe7f664;p=gostls13.git html/template: add srcset content type Srcset is largely the same as a URL, but is escaped in URL contexts. Inside a srcset attribute, URLs have their commas percent-escaped to avoid having the URL be interpreted as multiple URLs. Srcset is placed in a srcset attribute literally. Fixes #17441 Change-Id: I676b544784c7e54954ddb91eeff242cab25d02c4 Reviewed-on: https://go-review.googlesource.com/38324 Reviewed-by: Kunpei Sakai Reviewed-by: Mike Samuel Reviewed-by: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot --- diff --git a/src/html/template/attr.go b/src/html/template/attr.go index 7438f51f6a..92d2789e80 100644 --- a/src/html/template/attr.go +++ b/src/html/template/attr.go @@ -120,6 +120,7 @@ var attrTypeMap = map[string]contentType{ "src": contentTypeURL, "srcdoc": contentTypeHTML, "srclang": contentTypePlain, + "srcset": contentTypeSrcset, "start": contentTypePlain, "step": contentTypePlain, "style": contentTypeCSS, diff --git a/src/html/template/content.go b/src/html/template/content.go index 2e14bd1231..e7cdedc3b6 100644 --- a/src/html/template/content.go +++ b/src/html/template/content.go @@ -83,6 +83,14 @@ type ( // the encapsulated content should come from a trusted source, // as it will be included verbatim in the template output. URL string + + // Srcset encapsulates a known safe srcset attribute + // (see http://w3c.github.io/html/semantics-embedded-content.html#element-attrdef-img-srcset). + // + // Use of this type presents a security risk: + // the encapsulated content should come from a trusted source, + // as it will be included verbatim in the template output. + Srcset string ) type contentType uint8 @@ -95,6 +103,7 @@ const ( contentTypeJS contentTypeJSStr contentTypeURL + contentTypeSrcset // 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. @@ -156,6 +165,8 @@ func stringify(args ...interface{}) (string, contentType) { return string(s), contentTypeJSStr case URL: return string(s), contentTypeURL + case Srcset: + return string(s), contentTypeSrcset } } for i, arg := range args { diff --git a/src/html/template/content_test.go b/src/html/template/content_test.go index 0b4365c83b..cc092f50c0 100644 --- a/src/html/template/content_test.go +++ b/src/html/template/content_test.go @@ -19,7 +19,9 @@ func TestTypedContent(t *testing.T) { HTMLAttr(` dir="ltr"`), JS(`c && alert("Hello, World!");`), JSStr(`Hello, World & O'Reilly\x21`), - URL(`greeting=H%69&addressee=(World)`), + URL(`greeting=H%69,&addressee=(World)`), + Srcset(`greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`), + URL(`,foo/,`), } // For each content sensitive escaper, see how it does on @@ -40,6 +42,8 @@ func TestTypedContent(t *testing.T) { `ZgotmplZ`, `ZgotmplZ`, `ZgotmplZ`, + `ZgotmplZ`, + `ZgotmplZ`, }, }, { @@ -53,6 +57,8 @@ func TestTypedContent(t *testing.T) { `ZgotmplZ`, `ZgotmplZ`, `ZgotmplZ`, + `ZgotmplZ`, + `ZgotmplZ`, }, }, { @@ -65,7 +71,9 @@ func TestTypedContent(t *testing.T) { ` dir="ltr"`, `c && alert("Hello, World!");`, `Hello, World & O'Reilly\x21`, - `greeting=H%69&addressee=(World)`, + `greeting=H%69,&addressee=(World)`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `,foo/,`, }, }, { @@ -79,6 +87,8 @@ func TestTypedContent(t *testing.T) { `ZgotmplZ`, `ZgotmplZ`, `ZgotmplZ`, + `ZgotmplZ`, + `ZgotmplZ`, }, }, { @@ -91,7 +101,9 @@ func TestTypedContent(t *testing.T) { ` dir="ltr"`, `c && alert("Hello, World!");`, `Hello, World & O'Reilly\x21`, - `greeting=H%69&addressee=(World)`, + `greeting=H%69,&addressee=(World)`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `,foo/,`, }, }, { @@ -104,7 +116,9 @@ func TestTypedContent(t *testing.T) { ` dir="ltr"`, `c && alert("Hello, World!");`, `Hello, World & O'Reilly\x21`, - `greeting=H%69&addressee=(World)`, + `greeting=H%69,&addressee=(World)`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `,foo/,`, }, }, { @@ -117,7 +131,9 @@ func TestTypedContent(t *testing.T) { ` dir="ltr"`, `c && alert("Hello, World!");`, `Hello, World & O'Reilly\x21`, - `greeting=H%69&addressee=(World)`, + `greeting=H%69,&addressee=(World)`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `,foo/,`, }, }, { @@ -131,7 +147,9 @@ func TestTypedContent(t *testing.T) { `c && alert("Hello, World!");`, // Escape sequence not over-escaped. `"Hello, World & O'Reilly\x21"`, - `"greeting=H%69\u0026addressee=(World)"`, + `"greeting=H%69,\u0026addressee=(World)"`, + `"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`, + `",foo/,"`, }, }, { @@ -145,7 +163,9 @@ func TestTypedContent(t *testing.T) { `c && alert("Hello, World!");`, // Escape sequence not over-escaped. `"Hello, World & O'Reilly\x21"`, - `"greeting=H%69\u0026addressee=(World)"`, + `"greeting=H%69,\u0026addressee=(World)"`, + `"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`, + `",foo/,"`, }, }, { @@ -158,7 +178,9 @@ func TestTypedContent(t *testing.T) { `c \x26\x26 alert(\x22Hello, World!\x22);`, // Escape sequence not over-escaped. `Hello, World \x26 O\x27Reilly\x21`, - `greeting=H%69\x26addressee=(World)`, + `greeting=H%69,\x26addressee=(World)`, + `greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, + `,foo\/,`, }, }, { @@ -171,7 +193,9 @@ func TestTypedContent(t *testing.T) { `c \x26\x26 alert(\x22Hello, World!\x22);`, // Escape sequence not over-escaped. `Hello, World \x26 O\x27Reilly\x21`, - `greeting=H%69\x26addressee=(World)`, + `greeting=H%69,\x26addressee=(World)`, + `greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, + `,foo\/,`, }, }, { @@ -185,7 +209,9 @@ func TestTypedContent(t *testing.T) { `c && alert("Hello, World!");`, // Escape sequence not over-escaped. `"Hello, World & O'Reilly\x21"`, - `"greeting=H%69\u0026addressee=(World)"`, + `"greeting=H%69,\u0026addressee=(World)"`, + `"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`, + `",foo/,"`, }, }, { @@ -199,7 +225,9 @@ func TestTypedContent(t *testing.T) { ` dir="ltr"`, `c && alert("Hello, World!");`, `Hello, World & O'Reilly\x21`, - `greeting=H%69&addressee=(World)`, + `greeting=H%69,&addressee=(World)`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `,foo/,`, }, }, { @@ -212,7 +240,9 @@ func TestTypedContent(t *testing.T) { `c \x26\x26 alert(\x22Hello, World!\x22);`, // Escape sequence not over-escaped. `Hello, World \x26 O\x27Reilly\x21`, - `greeting=H%69\x26addressee=(World)`, + `greeting=H%69,\x26addressee=(World)`, + `greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, + `,foo\/,`, }, }, { @@ -225,7 +255,9 @@ func TestTypedContent(t *testing.T) { `c%20%26%26%20alert%28%22Hello%2c%20World%21%22%29%3b`, `Hello%2c%20World%20%26%20O%27Reilly%5cx21`, // Quotes and parens are escaped but %69 is not over-escaped. HTML escaping is done. - `greeting=H%69&addressee=%28World%29`, + `greeting=H%69,&addressee=%28World%29`, + `greeting%3dH%2569%2c%26addressee%3d%28World%29%202x%2c%20https%3a%2f%2fgolang.org%2ffavicon.ico%20500.5w`, + `,foo/,`, }, }, { @@ -238,7 +270,113 @@ func TestTypedContent(t *testing.T) { `c%20%26%26%20alert%28%22Hello%2c%20World%21%22%29%3b`, `Hello%2c%20World%20%26%20O%27Reilly%5cx21`, // Quotes and parens are escaped but %69 is not over-escaped. HTML escaping is not done. - `greeting=H%69&addressee=%28World%29`, + `greeting=H%69,&addressee=%28World%29`, + `greeting%3dH%2569%2c%26addressee%3d%28World%29%202x%2c%20https%3a%2f%2fgolang.org%2ffavicon.ico%20500.5w`, + `,foo/,`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + // Commas are not esacped + `Hello,#ZgotmplZ`, + // Leading spaces are not percent escapes. + ` dir=%22ltr%22`, + // Spaces after commas are not percent escaped. + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + // Metadata is not escaped. + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `%2cfoo/%2c`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + `Hello,#ZgotmplZ`, + // Spaces are HTML escaped not %-escaped + ` dir=%22ltr%22`, + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + // Commas are escaped. + `%2cfoo/%2c`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + `Hello,#ZgotmplZ`, + ` dir=%22ltr%22`, + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `%2cfoo/%2c`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + `Hello,#ZgotmplZ`, + ` dir=%22ltr%22`, + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `%2cfoo/%2c`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + `Hello,#ZgotmplZ`, + ` dir=%22ltr%22`, + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `%2cfoo/%2c`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + `Hello,#ZgotmplZ`, + ` dir=%22ltr%22`, + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `%2cfoo/%2c`, + }, + }, + { + ``, + []string{ + `#ZgotmplZ`, + `#ZgotmplZ`, + `Hello,#ZgotmplZ`, + ` dir=%22ltr%22`, + `#ZgotmplZ, World!%22%29;`, + `Hello,#ZgotmplZ`, + `greeting=H%69%2c&addressee=%28World%29`, + `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, + `%2cfoo/%2c`, }, }, } diff --git a/src/html/template/context.go b/src/html/template/context.go index 37a3faf88b..50730d3f2b 100644 --- a/src/html/template/context.go +++ b/src/html/template/context.go @@ -102,6 +102,8 @@ const ( stateAttr // stateURL occurs inside an HTML attribute whose content is a URL. stateURL + // stateSrcset occurs inside an HTML srcset attribute. + stateSrcset // stateJS occurs inside an event handler or script element. stateJS // stateJSDqStr occurs inside a JavaScript double quoted string. @@ -145,6 +147,7 @@ var stateNames = [...]string{ stateRCDATA: "stateRCDATA", stateAttr: "stateAttr", stateURL: "stateURL", + stateSrcset: "stateSrcset", stateJS: "stateJS", stateJSDqStr: "stateJSDqStr", stateJSSqStr: "stateJSSqStr", @@ -326,6 +329,8 @@ const ( attrStyle // attrURL corresponds to an attribute whose value is a URL. attrURL + // attrSrcset corresponds to a srcset attribute. + attrSrcset ) var attrNames = [...]string{ @@ -334,6 +339,7 @@ var attrNames = [...]string{ attrScriptType: "attrScriptType", attrStyle: "attrStyle", attrURL: "attrURL", + attrSrcset: "attrSrcset", } func (a attr) String() string { diff --git a/src/html/template/escape.go b/src/html/template/escape.go index b51a37039b..1241fa7713 100644 --- a/src/html/template/escape.go +++ b/src/html/template/escape.go @@ -71,6 +71,7 @@ var funcMap = template.FuncMap{ "_html_template_jsvalescaper": jsValEscaper, "_html_template_nospaceescaper": htmlNospaceEscaper, "_html_template_rcdataescaper": rcdataEscaper, + "_html_template_srcsetescaper": srcsetFilterAndEscaper, "_html_template_urlescaper": urlEscaper, "_html_template_urlfilter": urlFilter, "_html_template_urlnormalizer": urlNormalizer, @@ -215,6 +216,8 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context { case stateAttrName, stateTag: c.state = stateAttrName s = append(s, "_html_template_htmlnamefilter") + case stateSrcset: + s = append(s, "_html_template_srcsetescaper") default: if isComment(c.state) { s = append(s, "_html_template_commentescaper") diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go index bd075661c6..949985fe4a 100644 --- a/src/html/template/escape_test.go +++ b/src/html/template/escape_test.go @@ -650,6 +650,12 @@ func TestEscape(t *testing.T) { `<{{"script"}}>{{"doEvil()"}}`, `<script>doEvil()</script>`, }, + { + "srcset bad URL in second position", + ``, + // The second URL is also filtered. + ``, + }, } for _, test := range tests { diff --git a/src/html/template/transition.go b/src/html/template/transition.go index df7ac2289b..c72cf1ea60 100644 --- a/src/html/template/transition.go +++ b/src/html/template/transition.go @@ -23,6 +23,7 @@ var transitionFunc = [...]func(context, []byte) (context, int){ stateRCDATA: tSpecialTagEnd, stateAttr: tAttr, stateURL: tURL, + stateSrcset: tURL, stateJS: tJS, stateJSDqStr: tJSDelimited, stateJSSqStr: tJSDelimited, @@ -117,6 +118,8 @@ func tTag(c context, s []byte) (context, int) { attr = attrStyle case contentTypeJS: attr = attrScript + case contentTypeSrcset: + attr = attrSrcset } } @@ -161,6 +164,7 @@ var attrStartStates = [...]state{ attrScriptType: stateAttr, attrStyle: stateCSS, attrURL: stateURL, + attrSrcset: stateSrcset, } // tBeforeValue is the context transition function for stateBeforeValue. diff --git a/src/html/template/url.go b/src/html/template/url.go index a0bfe7672e..69a6ff49b8 100644 --- a/src/html/template/url.go +++ b/src/html/template/url.go @@ -37,13 +37,23 @@ func urlFilter(args ...interface{}) string { if t == contentTypeURL { return s } + if !isSafeUrl(s) { + return "#" + filterFailsafe + } + return s +} + +// isSafeUrl is true if s is a relative URL or if URL has a protocol in +// (http, https, mailto). +func isSafeUrl(s string) bool { if i := strings.IndexRune(s, ':'); i >= 0 && !strings.ContainsRune(s[:i], '/') { - protocol := strings.ToLower(s[:i]) - if protocol != "http" && protocol != "https" && protocol != "mailto" { - return "#" + filterFailsafe + + protocol := s[:i] + if !strings.EqualFold(protocol, "http") && !strings.EqualFold(protocol, "https") && !strings.EqualFold(protocol, "mailto") { + return false } } - return s + return true } // urlEscaper produces an output that can be embedded in a URL query. @@ -69,6 +79,16 @@ func urlProcessor(norm bool, args ...interface{}) string { norm = true } var b bytes.Buffer + if processUrlOnto(s, norm, &b) { + return b.String() + } + return s +} + +// processUrlOnto appends a normalized URL corresponding to its input to b +// and returns true if the appended content differs from s. +func processUrlOnto(s string, norm bool, b *bytes.Buffer) bool { + b.Grow(b.Cap() + len(s) + 16) written := 0 // The byte loop below assumes that all URLs use UTF-8 as the // content-encoding. This is similar to the URI to IRI encoding scheme @@ -114,12 +134,86 @@ func urlProcessor(norm bool, args ...interface{}) string { } } b.WriteString(s[written:i]) - fmt.Fprintf(&b, "%%%02x", c) + fmt.Fprintf(b, "%%%02x", c) written = i + 1 } - if written == 0 { + b.WriteString(s[written:]) + return written != 0 +} + +// Filters and normalizes srcset values which are comma separated +// URLs followed by metadata. +func srcsetFilterAndEscaper(args ...interface{}) string { + s, t := stringify(args...) + switch t { + case contentTypeSrcset: return s + case contentTypeURL: + // Normalizing gets rid of all HTML whitespace + // which separate the image URL from its metadata. + var b bytes.Buffer + if processUrlOnto(s, true, &b) { + s = b.String() + } + // Additionally, commas separate one source from another. + return strings.Replace(s, ",", "%2c", -1) } - b.WriteString(s[written:]) + + var b bytes.Buffer + written := 0 + for i := 0; i < len(s); i++ { + if s[i] == ',' { + filterSrcsetElement(s, written, i, &b) + b.WriteString(",") + written = i + 1 + } + } + filterSrcsetElement(s, written, len(s), &b) return b.String() } + +// Derived from https://play.golang.org/p/Dhmj7FORT5 +const htmlSpaceAndAsciiAlnumBytes = "\x00\x36\x00\x00\x01\x00\xff\x03\xfe\xff\xff\x07\xfe\xff\xff\x07" + +// isHtmlSpace is true iff c is a whitespace character per +// https://infra.spec.whatwg.org/#ascii-whitespace +func isHtmlSpace(c byte) bool { + return (c <= 0x20) && 0 != (htmlSpaceAndAsciiAlnumBytes[c>>3]&(1<>3]&(1<