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

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/+/207637Reviewed-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>

text/template: harden JSEscape to also escape ampersand and equal
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/+/207637Reviewed-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>
94e9a5e1 · Roberto Clapis · Filippo Valsorda · f4a8bf12 · 94e9a5e1 · 94e9a5e1
Commit 94e9a5e1 authored Nov 18, 2019 by Roberto Clapis Committed by Filippo Valsorda Nov 21, 2019
Showing with 12 additions and 4 deletions

src/html/template/example_test.go src/html/template/example_test.go +3 -3

src/text/template/exec_test.go src/text/template/exec_test.go +2 -0

src/text/template/funcs.go src/text/template/funcs.go +7 -1

No files found.
--- 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

 }

--- 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)

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