| From 16f4882984569f179d73967c9eee679bb9b098c5 Mon Sep 17 00:00:00 2001 |
| From: Roland Shoemaker <bracewell@google.com> |
| Date: Mon, 20 Mar 2023 11:01:13 -0700 |
| Subject: [PATCH 3/3] 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 |
| For #59234 |
| Fixes #59271 |
| |
| [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 <dneil@google.com> |
| Run-TryBot: Damien Neil <dneil@google.com> |
| Reviewed-by: Julie Qiu <julieqiu@google.com> |
| Reviewed-by: Roland Shoemaker <bracewell@google.com> |
| Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802612 |
| Run-TryBot: Roland Shoemaker <bracewell@google.com> |
| Change-Id: Ic7f10595615f2b2740d9c85ad7ef40dc0e78c04c |
| Reviewed-on: https://go-review.googlesource.com/c/go/+/481987 |
| Auto-Submit: Michael Knyszek <mknyszek@google.com> |
| TryBot-Result: Gopher Robot <gobot@golang.org> |
| Run-TryBot: Michael Knyszek <mknyszek@google.com> |
| Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
| |
| Upstream-Status: Backport from https://github.com/golang/go/commit/b1e3ecfa06b67014429a197ec5e134ce4303ad9b |
| CVE: CVE-2023-24538 |
| Signed-off-by: Shubham Kulkarni <skulkarni@mvista.com> |
| --- |
| src/html/template/context.go | 2 ++ |
| src/html/template/error.go | 13 +++++++++++++ |
| src/html/template/escape.go | 11 +++++++++++ |
| src/html/template/js.go | 2 ++ |
| src/html/template/jsctx_string.go | 9 +++++++++ |
| src/html/template/transition.go | 7 ++++++- |
| 6 files changed, 43 insertions(+), 1 deletion(-) |
| |
| diff --git a/src/html/template/context.go b/src/html/template/context.go |
| index f7d4849..0b65313 100644 |
| --- a/src/html/template/context.go |
| +++ b/src/html/template/context.go |
| @@ -116,6 +116,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 */. |
| diff --git a/src/html/template/error.go b/src/html/template/error.go |
| index 0e52706..fd26b64 100644 |
| --- a/src/html/template/error.go |
| +++ b/src/html/template/error.go |
| @@ -211,6 +211,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 { |
| diff --git a/src/html/template/escape.go b/src/html/template/escape.go |
| index f12dafa..29ca5b3 100644 |
| --- a/src/html/template/escape.go |
| +++ b/src/html/template/escape.go |
| @@ -8,6 +8,7 @@ import ( |
| "bytes" |
| "fmt" |
| "html" |
| + "internal/godebug" |
| "io" |
| "text/template" |
| "text/template/parse" |
| @@ -203,6 +204,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: |
| + debugAllowActionJSTmpl := godebug.Get("jstmpllitinterp") |
| + if debugAllowActionJSTmpl == "1" { |
| + 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: |
| diff --git a/src/html/template/js.go b/src/html/template/js.go |
| index ea9c183..b888eaf 100644 |
| --- a/src/html/template/js.go |
| +++ b/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`, |
| diff --git a/src/html/template/jsctx_string.go b/src/html/template/jsctx_string.go |
| index dd1d87e..2394893 100644 |
| --- a/src/html/template/jsctx_string.go |
| +++ b/src/html/template/jsctx_string.go |
| @@ -4,6 +4,15 @@ package template |
| |
| import "strconv" |
| |
| +func _() { |
| + // An "invalid array index" compiler error signifies that the constant values have changed. |
| + // Re-run the stringer command to generate them again. |
| + var x [1]struct{} |
| + _ = x[jsCtxRegexp-0] |
| + _ = x[jsCtxDivOp-1] |
| + _ = x[jsCtxUnknown-2] |
| +} |
| + |
| const _jsCtx_name = "jsCtxRegexpjsCtxDivOpjsCtxUnknown" |
| |
| var _jsCtx_index = [...]uint8{0, 11, 21, 33} |
| diff --git a/src/html/template/transition.go b/src/html/template/transition.go |
| index 06df679..92eb351 100644 |
| --- a/src/html/template/transition.go |
| +++ b/src/html/template/transition.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 = `\/[]` |
| } |
| -- |
| 2.7.4 |