Skip to content

Commit

Permalink
[release-branch.go1.20] html/template: emit filterFailsafe for empty …
Browse files Browse the repository at this point in the history
…unquoted attr value

An unquoted action used as an attribute value can result in unsafe
behavior if it is empty, as HTML normalization will result in unexpected
attributes, and may allow attribute injection. If executing a template
results in a empty unquoted attribute value, emit filterFailsafe
instead.

Thanks to Juho Nurminen of Mattermost for reporting this issue.

For golang#59722
Fixes golang#59816
Fixes CVE-2023-29400

Change-Id: Ia38d1b536ae2b4af5323a6c6d861e3c057c2570a
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1826631
Reviewed-by: Julie Qiu <julieqiu@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1851494
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/491358
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
rolandshoemaker authored and bradfitz committed May 25, 2023
1 parent 0060b24 commit 7754b9c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/html/template/escape.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,8 @@ func normalizeEscFn(e string) string {
// for all x.
var redundantFuncs = map[string]map[string]bool{
"_html_template_commentescaper": {
"_html_template_attrescaper": true,
"_html_template_nospaceescaper": true,
"_html_template_htmlescaper": true,
"_html_template_attrescaper": true,
"_html_template_htmlescaper": true,
},
"_html_template_cssescaper": {
"_html_template_attrescaper": true,
Expand Down
15 changes: 15 additions & 0 deletions src/html/template/escape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,21 @@ func TestEscape(t *testing.T) {
`<img srcset={{",,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"}}>`,
`<img srcset=,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,>`,
},
{
"unquoted empty attribute value (plaintext)",
"<p name={{.U}}>",
"<p name=ZgotmplZ>",
},
{
"unquoted empty attribute value (url)",
"<p href={{.U}}>",
"<p href=ZgotmplZ>",
},
{
"quoted empty attribute value",
"<p name=\"{{.U}}\">",
"<p name=\"\">",
},
}

for _, test := range tests {
Expand Down
3 changes: 3 additions & 0 deletions src/html/template/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import (
// htmlNospaceEscaper escapes for inclusion in unquoted attribute values.
func htmlNospaceEscaper(args ...any) string {
s, t := stringify(args...)
if s == "" {
return filterFailsafe
}
if t == contentTypeHTML {
return htmlReplacer(stripTags(s), htmlNospaceNormReplacementTable, false)
}
Expand Down

0 comments on commit 7754b9c

Please sign in to comment.