html/template: disallow actions in JS template literals

ECMAScript 6 introduced template literals[0][1] which are delimited with
backticks. These need to be escaped in a similar fashion to the
delimiters for other string literals. Additionally template literals can
contain special syntax for string interpolation.

There is no clear way to allow safe insertion of actions within JS
template literals, as handling (JS) string interpolation inside of these
literals is rather complex. As such we've chosen to simply disallow
template actions within these template literals.

A new error code is added for this parsing failure case, errJsTmplLit,
but it is unexported as it is not backwards compatible with other minor
release versions to introduce an API change in a minor release. We will
export this code in the next major release.

The previous behavior (with the cavet that backticks are now escaped
properly) can be re-enabled with GODEBUG=jstmpllitinterp=1.

This change subsumes CL471455.

Thanks to Sohom Datta, Manipal Institute of Technology, for reporting
this issue.

Fixes CVE-2023-24538
Fixes #59234

[0] https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals

Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802457
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Julie Qiu <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Change-Id: Ia221fefdb273bd0f066dffc2abcf2a616801d2f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/482079
TryBot-Bypass: Michael Knyszek <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>

Author: rolandshoemaker

src/html/template/context.go

@@ -120,6 +120,8 @@ const (
 	stateJSDqStr
 	// stateJSSqStr occurs inside a JavaScript single quoted string.
 	stateJSSqStr
+	// stateJSBqStr occurs inside a JavaScript back quoted string.
+	stateJSBqStr
 	// stateJSRegexp occurs inside a JavaScript regexp literal.
 	stateJSRegexp
 	// stateJSBlockCmt occurs inside a JavaScript /* block comment */.

src/html/template/error.go

@@ -214,6 +214,19 @@ const (
 	//   pipeline occurs in an unquoted attribute value context, "html" is
 	//   disallowed. Avoid using "html" and "urlquery" entirely in new templates.
 	ErrPredefinedEscaper
+
+	// errJSTmplLit: "... appears in a JS template literal"
+	// Example:
+	//     <script>var tmpl = `{{.Interp}`</script>
+	// Discussion:
+	//   Package html/template does not support actions inside of JS template
+	//   literals.
+	//
+	// TODO(rolandshoemaker): we cannot add this as an exported error in a minor
+	// release, since it is backwards incompatible with the other minor
+	// releases. As such we need to leave it unexported, and then we'll add it
+	// in the next major release.
+	errJSTmplLit
 )
 
 func (e *Error) Error() string {

src/html/template/escape.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"fmt"
 	"html"
+	"internal/godebug"
 	"io"
 	"text/template"
 	"text/template/parse"
@@ -160,6 +161,8 @@ func (e *escaper) escape(c context, n parse.Node) context {
 	panic("escaping " + n.String() + " is unimplemented")
 }
 
+var debugAllowActionJSTmpl = godebug.New("jstmpllitinterp")
+
 // escapeAction escapes an action template node.
 func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
 	if len(n.Pipe.Decl) != 0 {
@@ -223,6 +226,16 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
 		c.jsCtx = jsCtxDivOp
 	case stateJSDqStr, stateJSSqStr:
 		s = append(s, "_html_template_jsstrescaper")
+	case stateJSBqStr:
+		if debugAllowActionJSTmpl.Value() == "1" {
+			debugAllowActionJSTmpl.IncNonDefault()
+			s = append(s, "_html_template_jsstrescaper")
+		} else {
+			return context{
+				state: stateError,
+				err:   errorf(errJSTmplLit, n, n.Line, "%s appears in a JS template literal", n),
+			}
+		}
 	case stateJSRegexp:
 		s = append(s, "_html_template_jsregexpescaper")
 	case stateCSS:

src/html/template/escape_test.go

@@ -681,35 +681,31 @@ func TestEscape(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		tmpl := New(test.name)
-		tmpl = Must(tmpl.Parse(test.input))
-		// Check for bug 6459: Tree field was not set in Parse.
-		if tmpl.Tree != tmpl.text.Tree {
-			t.Errorf("%s: tree not set properly", test.name)
-			continue
-		}
-		b := new(strings.Builder)
-		if err := tmpl.Execute(b, data); err != nil {
-			t.Errorf("%s: template execution failed: %s", test.name, err)
-			continue
-		}
-		if w, g := test.output, b.String(); w != g {
-			t.Errorf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g)
-			continue
-		}
-		b.Reset()
-		if err := tmpl.Execute(b, pdata); err != nil {
-			t.Errorf("%s: template execution failed for pointer: %s", test.name, err)
-			continue
-		}
-		if w, g := test.output, b.String(); w != g {
-			t.Errorf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g)
-			continue
-		}
-		if tmpl.Tree != tmpl.text.Tree {
-			t.Errorf("%s: tree mismatch", test.name)
-			continue
-		}
+		t.Run(test.name, func(t *testing.T) {
+			tmpl := New(test.name)
+			tmpl = Must(tmpl.Parse(test.input))
+			// Check for bug 6459: Tree field was not set in Parse.
+			if tmpl.Tree != tmpl.text.Tree {
+				t.Fatalf("%s: tree not set properly", test.name)
+			}
+			b := new(strings.Builder)
+			if err := tmpl.Execute(b, data); err != nil {
+				t.Fatalf("%s: template execution failed: %s", test.name, err)
+			}
+			if w, g := test.output, b.String(); w != g {
+				t.Fatalf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g)
+			}
+			b.Reset()
+			if err := tmpl.Execute(b, pdata); err != nil {
+				t.Fatalf("%s: template execution failed for pointer: %s", test.name, err)
+			}
+			if w, g := test.output, b.String(); w != g {
+				t.Fatalf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g)
+			}
+			if tmpl.Tree != tmpl.text.Tree {
+				t.Fatalf("%s: tree mismatch", test.name)
+			}
+		})
 	}
 }
 
@@ -936,6 +932,10 @@ func TestErrors(t *testing.T) {
 			"{{range .Items}}<a{{if .X}}{{end}}>{{if .X}}{{break}}{{end}}{{end}}",
 			"",
 		},
+		{
+			"<script>var a = `${a+b}`</script>`",
+			"",
+		},
 		// Error cases.
 		{
 			"{{if .Cond}}<a{{end}}",
@@ -1082,6 +1082,10 @@ func TestErrors(t *testing.T) {
 			// html is allowed since it is the last command in the pipeline, but urlquery is not.
 			`predefined escaper "urlquery" disallowed in template`,
 		},
+		{
+			"<script>var tmpl = `asd {{.}}`;</script>",
+			`{{.}} appears in a JS template literal`,
+		},
 	}
 	for _, test := range tests {
 		buf := new(bytes.Buffer)
@@ -1303,6 +1307,10 @@ func TestEscapeText(t *testing.T) {
 			`<a onclick="'foo&quot;`,
 			context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript},
 		},
+		{
+			"<a onclick=\"`foo",
+			context{state: stateJSBqStr, delim: delimDoubleQuote, attr: attrScript},
+		},
 		{
 			`<A ONCLICK="'`,
 			context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript},

src/html/template/js.go

@@ -308,6 +308,7 @@ var jsStrReplacementTable = []string{
 	// Encode HTML specials as hex so the output can be embedded
 	// in HTML attributes without further encoding.
 	'"':  `\u0022`,
+	'`':  `\u0060`,
 	'&':  `\u0026`,
 	'\'': `\u0027`,
 	'+':  `\u002b`,
@@ -331,6 +332,7 @@ var jsStrNormReplacementTable = []string{
 	'"':  `\u0022`,
 	'&':  `\u0026`,
 	'\'': `\u0027`,
+	'`':  `\u0060`,
 	'+':  `\u002b`,
 	'/':  `\/`,
 	'<':  `\u003c`,

src/html/template/js_test.go

@@ -291,7 +291,7 @@ func TestEscapersOnLower7AndSelectHighCodepoints(t *testing.T) {
 				`0123456789:;\u003c=\u003e?` +
 				`@ABCDEFGHIJKLMNO` +
 				`PQRSTUVWXYZ[\\]^_` +
-				"`abcdefghijklmno" +
+				"\\u0060abcdefghijklmno" +
 				"pqrstuvwxyz{|}~\u007f" +
 				"\u00A0\u0100\\u2028\\u2029\ufeff\U0001D11E",
 		},

src/html/template/jsctx_string.go

@@ -27,6 +27,7 @@ var transitionFunc = [...]func(context, []byte) (context, int){
 	stateJS:          tJS,
 	stateJSDqStr:     tJSDelimited,
 	stateJSSqStr:     tJSDelimited,
+	stateJSBqStr:     tJSDelimited,
 	stateJSRegexp:    tJSDelimited,
 	stateJSBlockCmt:  tBlockCmt,
 	stateJSLineCmt:   tLineCmt,
@@ -262,7 +263,7 @@ func tURL(c context, s []byte) (context, int) {
 
 // tJS is the context transition function for the JS state.
 func tJS(c context, s []byte) (context, int) {
-	i := bytes.IndexAny(s, `"'/`)
+	i := bytes.IndexAny(s, "\"`'/")
 	if i == -1 {
 		// Entire input is non string, comment, regexp tokens.
 		c.jsCtx = nextJSCtx(s, c.jsCtx)
@@ -274,6 +275,8 @@ func tJS(c context, s []byte) (context, int) {
 		c.state, c.jsCtx = stateJSDqStr, jsCtxRegexp
 	case '\'':
 		c.state, c.jsCtx = stateJSSqStr, jsCtxRegexp
+	case '`':
+		c.state, c.jsCtx = stateJSBqStr, jsCtxRegexp
 	case '/':
 		switch {
 		case i+1 < len(s) && s[i+1] == '/':
@@ -303,6 +306,8 @@ func tJSDelimited(c context, s []byte) (context, int) {
 	switch c.state {
 	case stateJSSqStr:
 		specials = `\'`
+	case stateJSBqStr:
+		specials = "`\\"
 	case stateJSRegexp:
 		specials = `\/[]`
 	}

src/html/template/state_string.go

@@ -290,6 +290,7 @@ func initMetrics() {
 		"/godebug/non-default-behavior/http2client:events":          {compute: compute0},
 		"/godebug/non-default-behavior/http2server:events":          {compute: compute0},
 		"/godebug/non-default-behavior/installgoroot:events":        {compute: compute0},
+		"/godebug/non-default-behavior/jstmpllitinterp:events":      {compute: compute0},
 		"/godebug/non-default-behavior/multipartfiles:events":       {compute: compute0},
 		"/godebug/non-default-behavior/multipartmaxheaders:events":  {compute: compute0},
 		"/godebug/non-default-behavior/multipartmaxparts:events":    {compute: compute0},

src/html/template/transition.go

@@ -305,6 +305,13 @@ var allDesc = []Description{
 		Kind:       KindUint64,
 		Cumulative: true,
 	},
+	{
+		Name: "/godebug/non-default-behavior/jstmpllitinterp:events",
+		Description: "The number of non-default behaviors executed by the html/template" +
+			"package due to a non-default GODEBUG=jstmpllitinterp=... setting.",
+		Kind:       KindUint64,
+		Cumulative: true,
+	},
 	{
 		Name: "/godebug/non-default-behavior/multipartfiles:events",
 		Description: "The number of non-default behaviors executed by the mime/multipart package " +

src/runtime/metrics.go

@@ -219,6 +219,11 @@ Below is the full list of supported metrics, ordered lexicographically.
 		The number of non-default behaviors executed by the go/build
 		package due to a non-default GODEBUG=installgoroot=... setting.
 
+	/godebug/non-default-behavior/jstmpllitinterp:events
+		The number of non-default behaviors executed by
+		the html/templatepackage due to a non-default
+		GODEBUG=jstmpllitinterp=... setting.
+
 	/godebug/non-default-behavior/multipartfiles:events
 		The number of non-default behaviors executed by
 		the mime/multipart package due to a non-default