[release-branch.go1.19] html/template: emit filterFailsafe for empty unquoted attr...

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 #59722
Fixes #59815
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/+/1851498
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/491357
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index 3d4cc19b5da85c22859b1182d053b1a19adba2ef..bcba8db4aa00bca3850b27e5ee77be5c6e7a113a 100644 (file)
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -380,9 +380,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,

diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index 972b00b921d35be8eeec5a462b6d7657826a0ea6..a1a6c1cd1600416a94524f730691799d20016496 100644 (file)
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -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 {

diff --git a/src/html/template/html.go b/src/html/template/html.go
index 46e9d931511cfa3403ead650d4d3451d1812fb73..6fb9237bdac005116f8f09e52c3da3ec018612da 100644 (file)
--- a/src/html/template/html.go
+++ b/src/html/template/html.go
@@ -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)
        }