text/template: harden JSEscape to also escape ampersand and equal

empijei · FiloSottile · commit 94e9a5e19b83 · 2019-11-21T22:20:17.000Z
Ampersand and equal are not dangerous in a JS/JSString context but they might cause issues if interpolated in HTML attributes. This change makes it harder to introduce XSS by misusing escaping. Thanks to t1ddl3r <t1ddl3r@gmail.com> for reporting this common misuse scenario. Fixes #35665 Change-Id: Ice6416477bba4cb2ba2fe2cfdc20e027957255c0 Reviewed-on: https://go-review.googlesource.com/c/go/+/207637 Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Mike Samuel <mikesamuel@gmail.com> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
diff --git a/src/html/template/example_test.go b/src/html/template/example_test.go
@@ -116,9 +116,9 @@ func Example_escape() {
 	// &#34;Fran &amp; Freddie&#39;s Diner&#34; &lt;tasty@example.com&gt;
 	// &#34;Fran &amp; Freddie&#39;s Diner&#34; &lt;tasty@example.com&gt;
 	// &#34;Fran &amp; Freddie&#39;s Diner&#34;32&lt;tasty@example.com&gt;
-	// \"Fran & Freddie\'s Diner\" \x3Ctasty@example.com\x3E
-	// \"Fran & Freddie\'s Diner\" \x3Ctasty@example.com\x3E
-	// \"Fran & Freddie\'s Diner\"32\x3Ctasty@example.com\x3E
+	// \"Fran \x26 Freddie\'s Diner\" \x3Ctasty@example.com\x3E
+	// \"Fran \x26 Freddie\'s Diner\" \x3Ctasty@example.com\x3E
+	// \"Fran \x26 Freddie\'s Diner\"32\x3Ctasty@example.com\x3E
 	// %22Fran+%26+Freddie%27s+Diner%2232%3Ctasty%40example.com%3E
 
 }
diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
@@ -909,6 +909,8 @@ func TestJSEscaping(t *testing.T) {
 		{`Yukihiro says "今日は世界"`, `Yukihiro says \"今日は世界\"`},
 		{"unprintable \uFDFF", `unprintable \uFDFF`},
 		{`<html>`, `\x3Chtml\x3E`},
+		{`no = in attributes`, `no \x3D in attributes`},
+		{`&#x27; does not become HTML entity`, `\x26#x27; does not become HTML entity`},
 	}
 	for _, tc := range testCases {
 		s := JSEscapeString(tc.in)
diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go
@@ -642,6 +642,8 @@ var (
 	jsQuot      = []byte(`\"`)
 	jsLt        = []byte(`\x3C`)
 	jsGt        = []byte(`\x3E`)
+	jsAmp       = []byte(`\x26`)
+	jsEq        = []byte(`\x3D`)
 )
 
 // JSEscape writes to w the escaped JavaScript equivalent of the plain text data b.
@@ -670,6 +672,10 @@ func JSEscape(w io.Writer, b []byte) {
 				w.Write(jsLt)
 			case '>':
 				w.Write(jsGt)
+			case '&':
+				w.Write(jsAmp)
+			case '=':
+				w.Write(jsEq)
 			default:
 				w.Write(jsLowUni)
 				t, b := c>>4, c&0x0f
@@ -704,7 +710,7 @@ func JSEscapeString(s string) string {
 
 func jsIsSpecial(r rune) bool {
 	switch r {
-	case '\\', '\'', '"', '<', '>':
+	case '\\', '\'', '"', '<', '>', '&', '=':
 		return true
 	}
 	return r < ' ' || utf8.RuneSelf <= r

-Original file line number
+Diff line change
 	// &#34;Fran &amp; Freddie&#39;s Diner&#34; &lt;[email protected]&gt;
 	// &#34;Fran &amp; Freddie&#39;s Diner&#34; &lt;[email protected]&gt;
 	// &#34;Fran &amp; Freddie&#39;s Diner&#34;32&lt;[email protected]&gt;
 -	// \"Fran & Freddie\'s Diner\" \[email protected]\x3E
 -	// \"Fran & Freddie\'s Diner\" \[email protected]\x3E
 -	// \"Fran & Freddie\'s Diner\"32\[email protected]\x3E
 +	// \"Fran \x26 Freddie\'s Diner\" \[email protected]\x3E
 +	// \"Fran \x26 Freddie\'s Diner\" \[email protected]\x3E
 +	// \"Fran \x26 Freddie\'s Diner\"32\[email protected]\x3E
 	// %22Fran+%26+Freddie%27s+Diner%2232%3Ctasty%40example.com%3E
+}
Original file line number	Diff line number	Diff line change
`@@ -909,6 +909,8 @@ func TestJSEscaping(t *testing.T) {`
`909`	`909`	{`Yukihiro says "今日は世界"`, `Yukihiro says \"今日は世界\"`},
`910`	`910`	{"unprintable \uFDFF", `unprintable \uFDFF`},
`911`	`911`	{`<html>`, `\x3Chtml\x3E`},
	`912`	+ {`no = in attributes`, `no \x3D in attributes`},
	`913`	+ {`' does not become HTML entity`, `\x26#x27; does not become HTML entity`},
`912`	`914`	`}`
`913`	`915`	`for _, tc := range testCases {`
`914`	`916`	`s := JSEscapeString(tc.in)`