From 0d347544cbca0f42b160424f6bc2458ebcc7b3fc Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 13 Apr 2023 14:01:50 -0700 Subject: [PATCH] html/template: emit filterFailsafe for empty 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. Fixes #59722 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 Run-TryBot: Roland Shoemaker Reviewed-by: Damien Neil Reviewed-on: https://go-review.googlesource.com/c/go/+/491617 Run-TryBot: Carlos Amedee Reviewed-by: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov TryBot-Result: Gopher Robot Upstream-Status: Backport from [https://github.com/golang/go/commit/0d347544cbca0f42b160424f6bc2458ebcc7b3fc] CVE: CVE-2023-29400 Signed-off-by: Ashish Sharma --- src/html/template/escape.go | 5 ++--- src/html/template/escape_test.go | 15 +++++++++++++++ src/html/template/html.go | 3 +++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/html/template/escape.go b/src/html/template/escape.go index 4ba1d6b31897e..a62ef159f0dcd 100644 --- a/src/html/template/escape.go +++ b/src/html/template/escape.go @@ -382,9 +382,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 3dd212bac9406..f8b2b448f2dfa 100644 --- a/src/html/template/escape_test.go +++ b/src/html/template/escape_test.go @@ -678,6 +678,21 @@ func TestEscape(t *testing.T) { ``, ``, }, + { + "unquoted empty attribute value (plaintext)", + "

", + "

", + }, + { + "unquoted empty attribute value (url)", + "

", + "

", + }, + { + "quoted empty attribute value", + "

", + "

", + }, } for _, test := range tests { diff --git a/src/html/template/html.go b/src/html/template/html.go index bcca0b51a0ef9..a181699a5bda8 100644 --- 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 ...interface{}) string { s, t := stringify(args...) + if s == "" { + return filterFailsafe + } if t == contentTypeHTML { return htmlReplacer(stripTags(s), htmlNospaceNormReplacementTable, false) }