diff --git a/src/html/template/context.go b/src/html/template/context.go
index 8b3af2feabd8aa..b42e3077c3fc44 100644
--- a/src/html/template/context.go
+++ b/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
+ // where branches end with
+ // different partialName values but identical security contexts.
return c.state == d.state &&
c.delim == d.delim &&
c.urlPart == d.urlPart &&
diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index d8e1b8cb547db2..5ef7f4d3ee81a5 100644
--- a/src/html/template/escape.go
+++ b/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:
diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index 43781f38a5436c..07ff289e3af35a 100644
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -643,12 +643,8 @@ func TestEscape(t *testing.T) {
``,
``,
},
- {
- "dynamic attribute name",
- ``,
- // Treated as JS since quotes are inserted.
- ``,
- },
+ // "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) {
``,
``,
},
- {
- "dynamic element name",
- `
...`,
- `
...`,
- },
+ // "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
diff --git a/src/html/template/transition.go b/src/html/template/transition.go
index 7fbab1df7b06ee..d132413a4d97fc 100644
--- a/src/html/template/transition.go
+++ b/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
}