html/template: reject dynamic substrings in attribute names

src/html/template/context.go

@@ -30,6 +30,14 @@ type context struct {
 	element      element
 	n            parse.Node // for range break/continue
 	err          *Error
+	// partialName is true when the context is inside an attribute
+	// name that was started in static template text but was not completed
+	// before a template action. A template action must not appear in this
+	// context because the escaper determines the security context of
+	// attributes from the static text only, and a dynamic substring could
+	// complete the name into something security-sensitive.
+	// See https://go.dev/issue/19669.
+	partialName bool
 }
 
 func (c context) String() string {
@@ -42,6 +50,13 @@ func (c context) String() string {
 
 // eq reports whether two contexts are equal.
 func (c context) eq(d context) bool {
+	// partialName is intentionally excluded from comparison.
+	// It is metadata about whether the current attribute name is
+	// a split name started in static text. It does not affect the
+	// security-relevant escaping context, and including it would
+	// cause false ErrBranchEnd errors in templates like
+	// <input{{if .T}} checked{{end}}> where branches end with
+	// different partialName values but identical security contexts.
 	return c.state == d.state &&
 		c.delim == d.delim &&
 		c.urlPart == d.urlPart &&

src/html/template/escape.go

@@ -149,16 +149,22 @@ func (e *escaper) escape(c context, n parse.Node) context {
 		e.rangeContext.continues = append(e.rangeContext.continues, c)
 		return context{state: stateDead}
 	case *parse.IfNode:
+		// A branch node cannot continue a partial attribute name,
+		// so clear the flag to avoid false positives.
+		c.partialName = false
 		return e.escapeBranch(c, &n.BranchNode, "if")
 	case *parse.ListNode:
 		return e.escapeList(c, n)
 	case *parse.RangeNode:
+		c.partialName = false
 		return e.escapeBranch(c, &n.BranchNode, "range")
 	case *parse.TemplateNode:
+		c.partialName = false
 		return e.escapeTemplate(c, n)
 	case *parse.TextNode:
 		return e.escapeText(c, n)
 	case *parse.WithNode:
+		c.partialName = false
 		return e.escapeBranch(c, &n.BranchNode, "with")
 	}
 	panic("escaping " + n.String() + " is unimplemented")
@@ -255,7 +261,24 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
 		s = append(s, "_html_template_rcdataescaper")
 	case stateAttr:
 		// Handled below in delim check.
-	case stateAttrName, stateTag:
+	case stateAttrName:
+		if c.partialName {
+			// A dynamic substring is being used to construct an HTML
+			// attribute name that started in static template text.
+			// This is unsafe because the escaper determines the security
+			// context of attributes (URL, CSS, JS, etc.) from the static
+			// prefix only. The dynamic suffix could complete the name
+			// into a security-sensitive attribute (e.g., "hre" + "f" =
+			// "href") without the proper escaping being applied.
+			// See https://go.dev/issue/19669.
+			return context{
+				state: stateError,
+				err:   errorf(ErrBadHTML, n, n.Line, "%s appears in the middle of an HTML tag or attribute name; dynamic substrings of tag and attribute names are not supported", n),
+			}
+		}
+		c.state = stateAttrName
+		s = append(s, "_html_template_htmlnamefilter")
+	case stateTag:
 		c.state = stateAttrName
 		s = append(s, "_html_template_htmlnamefilter")
 	case stateSrcset:

src/html/template/escape_test.go

@@ -643,12 +643,8 @@ func TestEscape(t *testing.T) {
 			`<input {{if .T}}{{"checked"}} {{end}}name=n>`,
 			`<input checked name=n>`,
 		},
-		{
-			"dynamic attribute name",
-			`<img on{{"load"}}="alert({{"loaded"}})">`,
-			// Treated as JS since quotes are inserted.
-			`<img onload="alert(&#34;loaded&#34;)">`,
-		},
+		// "dynamic attribute name" test moved to TestErrors: split
+		// attribute names are no longer allowed. See #19669.
 		{
 			"bad dynamic attribute name 1",
 			// Allow checked, selected, disabled, but not JS or
@@ -674,11 +670,8 @@ func TestEscape(t *testing.T) {
 			`<input checked {{""}}="Whose value am I?">`,
 			`<input checked ZgotmplZ="Whose value am I?">`,
 		},
-		{
-			"dynamic element name",
-			`<h{{3}}><table><t{{"head"}}>...</h{{3}}>`,
-			`<h3><table><thead>...</h3>`,
-		},
+		// "dynamic element name" test moved to TestErrors: split
+		// tag names are no longer allowed. See #19669.
 		{
 			"bad dynamic element name",
 			// Dynamic element names are typically used to switch

src/html/template/transition.go

@@ -92,6 +92,11 @@ var elementContentType = [...]state{
 func tTag(c context, s []byte) (context, int) {
 	// Find the attribute name.
 	i := eatWhiteSpace(s, 0)
+	if i > 0 {
+		// Whitespace after a tag or attribute name confirms the name
+		// is complete, so clear the partial name flag.
+		c.partialName = false
+	}
 	if i == len(s) {
 		return c, len(s)
 	}
@@ -136,12 +141,16 @@ func tTag(c context, s []byte) (context, int) {
 		}
 	}
 
+	partial := false
 	if j == len(s) {
 		state = stateAttrName
+		// The attribute name runs to the end of the text node and
+		// may be continued by a subsequent template action.
+		partial = true
 	} else {
 		state = stateAfterName
 	}
-	return context{state: state, element: c.element, attr: attr}, j
+	return context{state: state, element: c.element, attr: attr, partialName: partial}, j
 }
 
 // tAttrName is the context transition function for stateAttrName.
@@ -151,6 +160,7 @@ func tAttrName(c context, s []byte) (context, int) {
 		return context{state: stateError, err: err}, len(s)
 	} else if i != len(s) {
 		c.state = stateAfterName
+		c.partialName = false
 	}
 	return c, i
 }