[release-branch.go1.20] html/template: support HTML-like comments in …

…script contexts Per Appendix B.1.1 of the ECMAScript specification, support HTML-like comments in script contexts. Also per section 12.5, support hashbang comments. This brings our parsing in-line with how browsers treat these comment types. Thanks to Takeshi Kaneko (GMO Cybersecurity by Ierae, Inc.) for reporting this issue. Fixes #62196 Fixes #62395 Fixes CVE-2023-39318 Change-Id: Id512702c5de3ae46cf648e268cb10e1eb392a181 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1976593 Run-TryBot: Roland Shoemaker <[email protected]> Reviewed-by: Tatiana Bradley <[email protected]> Reviewed-by: Damien Neil <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2014620 Reviewed-on: https://go-review.googlesource.com/c/go/+/526098 Run-TryBot: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
golang · Sep 6, 2023 · 023b542 · 023b542
1 parent 612da32
commit 023b542
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 43 deletions.
diff --git a/src/html/template/context.go b/src/html/template/context.go
@@ -128,6 +128,10 @@ const (
 	stateJSBlockCmt
 	// stateJSLineCmt occurs inside a JavaScript // line comment.
 	stateJSLineCmt
+	// stateJSHTMLOpenCmt occurs inside a JavaScript <!-- HTML-like comment.
+	stateJSHTMLOpenCmt
+	// stateJSHTMLCloseCmt occurs inside a JavaScript --> HTML-like comment.
+	stateJSHTMLCloseCmt
 	// stateCSS occurs inside a <style> element or style attribute.
 	stateCSS
 	// stateCSSDqStr occurs inside a CSS double quoted string.
@@ -155,7 +159,7 @@ const (
 // authors & maintainers, not for end-users or machines.
 func isComment(s state) bool {
 	switch s {
-	case stateHTMLCmt, stateJSBlockCmt, stateJSLineCmt, stateCSSBlockCmt, stateCSSLineCmt:
+	case stateHTMLCmt, stateJSBlockCmt, stateJSLineCmt, stateJSHTMLOpenCmt, stateJSHTMLCloseCmt, stateCSSBlockCmt, stateCSSLineCmt:
 		return true
 	}
 	return false

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
@@ -776,9 +776,12 @@ func (e *escaper) escapeText(c context, n *parse.TextNode) context {
 		if c.state != c1.state && isComment(c1.state) && c1.delim == delimNone {
 			// Preserve the portion between written and the comment start.
 			cs := i1 - 2
-			if c1.state == stateHTMLCmt {
+			if c1.state == stateHTMLCmt || c1.state == stateJSHTMLOpenCmt {
 				// "<!--" instead of "/*" or "//"
 				cs -= 2
+			} else if c1.state == stateJSHTMLCloseCmt {
+				// "-->" instead of "/*" or "//"
+				cs -= 1
 			}
 			b.Write(s[written:cs])
 			written = i1

diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
@@ -503,6 +503,16 @@ func TestEscape(t *testing.T) {
 			"<script>var a/*b*///c\nd</script>",
 			"<script>var a \nd</script>",
 		},
+		{
+			"JS HTML-like comments",
+			"<script>before <!-- beep\nbetween\nbefore-->boop\n</script>",
+			"<script>before \nbetween\nbefore\n</script>",
+		},
+		{
+			"JS hashbang comment",
+			"<script>#! beep\n</script>",
+			"<script>\n</script>",
+		},
 		{
 			"CSS comments",
 			"<style>p// paragraph\n" +

diff --git a/src/html/template/state_string.go b/src/html/template/state_string.go
diff --git a/src/html/template/transition.go b/src/html/template/transition.go
@@ -14,32 +14,34 @@ import (
 // the updated context and the number of bytes consumed from the front of the
 // input.
 var transitionFunc = [...]func(context, []byte) (context, int){
-	stateText:        tText,
-	stateTag:         tTag,
-	stateAttrName:    tAttrName,
-	stateAfterName:   tAfterName,
-	stateBeforeValue: tBeforeValue,
-	stateHTMLCmt:     tHTMLCmt,
-	stateRCDATA:      tSpecialTagEnd,
-	stateAttr:        tAttr,
-	stateURL:         tURL,
-	stateSrcset:      tURL,
-	stateJS:          tJS,
-	stateJSDqStr:     tJSDelimited,
-	stateJSSqStr:     tJSDelimited,
-	stateJSBqStr:     tJSDelimited,
-	stateJSRegexp:    tJSDelimited,
-	stateJSBlockCmt:  tBlockCmt,
-	stateJSLineCmt:   tLineCmt,
-	stateCSS:         tCSS,
-	stateCSSDqStr:    tCSSStr,
-	stateCSSSqStr:    tCSSStr,
-	stateCSSDqURL:    tCSSStr,
-	stateCSSSqURL:    tCSSStr,
-	stateCSSURL:      tCSSStr,
-	stateCSSBlockCmt: tBlockCmt,
-	stateCSSLineCmt:  tLineCmt,
-	stateError:       tError,
+	stateText:           tText,
+	stateTag:            tTag,
+	stateAttrName:       tAttrName,
+	stateAfterName:      tAfterName,
+	stateBeforeValue:    tBeforeValue,
+	stateHTMLCmt:        tHTMLCmt,
+	stateRCDATA:         tSpecialTagEnd,
+	stateAttr:           tAttr,
+	stateURL:            tURL,
+	stateSrcset:         tURL,
+	stateJS:             tJS,
+	stateJSDqStr:        tJSDelimited,
+	stateJSSqStr:        tJSDelimited,
+	stateJSBqStr:        tJSDelimited,
+	stateJSRegexp:       tJSDelimited,
+	stateJSBlockCmt:     tBlockCmt,
+	stateJSLineCmt:      tLineCmt,
+	stateJSHTMLOpenCmt:  tLineCmt,
+	stateJSHTMLCloseCmt: tLineCmt,
+	stateCSS:            tCSS,
+	stateCSSDqStr:       tCSSStr,
+	stateCSSSqStr:       tCSSStr,
+	stateCSSDqURL:       tCSSStr,
+	stateCSSSqURL:       tCSSStr,
+	stateCSSURL:         tCSSStr,
+	stateCSSBlockCmt:    tBlockCmt,
+	stateCSSLineCmt:     tLineCmt,
+	stateError:          tError,
 }
 
 var commentStart = []byte("<!--")
@@ -263,7 +265,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)
@@ -293,6 +295,26 @@ func tJS(c context, s []byte) (context, int) {
 				err:   errorf(ErrSlashAmbig, nil, 0, "'/' could start a division or regexp: %.32q", s[i:]),
 			}, len(s)
 		}
+	// ECMAScript supports HTML style comments for legacy reasons, see Appendix
+	// B.1.1 "HTML-like Comments". The handling of these comments is somewhat
+	// confusing. Multi-line comments are not supported, i.e. anything on lines
+	// between the opening and closing tokens is not considered a comment, but
+	// anything following the opening or closing token, on the same line, is
+	// ignored. As such we simply treat any line prefixed with "<!--" or "-->"
+	// as if it were actually prefixed with "//" and move on.
+	case '<':
+		if i+3 < len(s) && bytes.Equal(commentStart, s[i:i+4]) {
+			c.state, i = stateJSHTMLOpenCmt, i+3
+		}
+	case '-':
+		if i+2 < len(s) && bytes.Equal(commentEnd, s[i:i+3]) {
+			c.state, i = stateJSHTMLCloseCmt, i+2
+		}
+	// ECMAScript also supports "hashbang" comment lines, see Section 12.5.
+	case '#':
+		if i+1 < len(s) && s[i+1] == '!' {
+			c.state, i = stateJSLineCmt, i+1
+		}
 	default:
 		panic("unreachable")
 	}
@@ -372,12 +394,12 @@ func tBlockCmt(c context, s []byte) (context, int) {
 	return c, i + 2
 }
 
-// tLineCmt is the context transition function for //comment states.
+// tLineCmt is the context transition function for //comment states, and the JS HTML-like comment state.
 func tLineCmt(c context, s []byte) (context, int) {
 	var lineTerminators string
 	var endState state
 	switch c.state {
-	case stateJSLineCmt:
+	case stateJSLineCmt, stateJSHTMLOpenCmt, stateJSHTMLCloseCmt:
 		lineTerminators, endState = "\n\r\u2028\u2029", stateJS
 	case stateCSSLineCmt:
 		lineTerminators, endState = "\n\f\r", stateCSS