[release-branch.go1.20] 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 #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>

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index 23ece7a72fbacc9e62ea272780f976dda7696417..c262d1698de6140cc7e1f4faa4b6caade3202794 100644 (file)
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -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,

diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index 3dd212bac94061e87de155269341d9e63a703b6c..f8b2b448f2dfa7394053fd75b48b76cd9fefef80 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 bcca0b51a0ef901de193509f3fa709748b679381..a181699a5bda82fb0ad9df25a9eb6f576138c6bd 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)
        }