From 6fc21505614f36178df0dad7034b6b8e3f7588d5 Mon Sep 17 00:00:00 2001 From: empijei Date: Fri, 27 Mar 2020 19:27:55 +0100 Subject: [PATCH 2/6] html/template,text/template: switch to Unicode escapes for JSON compatibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing implementation is not compatible with JSON escape as it uses hex escaping. Unicode escape, instead, is valid for both JSON and JS. This fix avoids creating a separate escaping context for scripts of type "application/ld+json" and it is more future-proof in case more JSON+JS contexts get added to the platform (e.g. import maps). Fixes #33671 Fixes #37634 Change-Id: Id6f6524b4abc52e81d9d744d46bbe5bf2e081543 Reviewed-on: https://go-review.googlesource.com/c/go/+/226097 Reviewed-by: Carl Johnson Reviewed-by: Daniel Martí Run-TryBot: Daniel Martí TryBot-Result: Gobot Gobot Dependency Patch #2 Upstream-Status: Backport from https://github.com/golang/go/commit/d4d298040d072ddacea0e0d6b55fb148fff18070 CVE: CVE-2023-24538 Signed-off-by: Shubham Kulkarni --- src/html/template/content_test.go | 70 +++++++++++++++++++------------------- src/html/template/escape_test.go | 6 ++-- src/html/template/example_test.go | 6 ++-- src/html/template/js.go | 70 +++++++++++++++++++++++--------------- src/html/template/js_test.go | 68 ++++++++++++++++++------------------ src/html/template/template_test.go | 39 +++++++++++++++++++++ src/text/template/exec_test.go | 6 ++-- src/text/template/funcs.go | 8 ++--- 8 files changed, 163 insertions(+), 110 deletions(-) diff --git a/src/html/template/content_test.go b/src/html/template/content_test.go index 72d56f5..bd86527 100644 --- a/src/html/template/content_test.go +++ b/src/html/template/content_test.go @@ -18,7 +18,7 @@ func TestTypedContent(t *testing.T) { HTML(`Hello, World &tc!`), HTMLAttr(` dir="ltr"`), JS(`c && alert("Hello, World!");`), - JSStr(`Hello, World & O'Reilly\x21`), + JSStr(`Hello, World & O'Reilly\u0021`), URL(`greeting=H%69,&addressee=(World)`), Srcset(`greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`), URL(`,foo/,`), @@ -70,7 +70,7 @@ func TestTypedContent(t *testing.T) { `Hello, World &tc!`, ` dir="ltr"`, `c && alert("Hello, World!");`, - `Hello, World & O'Reilly\x21`, + `Hello, World & O'Reilly\u0021`, `greeting=H%69,&addressee=(World)`, `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, `,foo/,`, @@ -100,7 +100,7 @@ func TestTypedContent(t *testing.T) { `Hello, World &tc!`, ` dir="ltr"`, `c && alert("Hello, World!");`, - `Hello, World & O'Reilly\x21`, + `Hello, World & O'Reilly\u0021`, `greeting=H%69,&addressee=(World)`, `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, `,foo/,`, @@ -115,7 +115,7 @@ func TestTypedContent(t *testing.T) { `Hello, World &tc!`, ` dir="ltr"`, `c && alert("Hello, World!");`, - `Hello, World & O'Reilly\x21`, + `Hello, World & O'Reilly\u0021`, `greeting=H%69,&addressee=(World)`, `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, `,foo/,`, @@ -130,7 +130,7 @@ func TestTypedContent(t *testing.T) { `Hello, <b>World</b> &tc!`, ` dir="ltr"`, `c && alert("Hello, World!");`, - `Hello, World & O'Reilly\x21`, + `Hello, World & O'Reilly\u0021`, `greeting=H%69,&addressee=(World)`, `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, `,foo/,`, @@ -146,7 +146,7 @@ func TestTypedContent(t *testing.T) { // Not escaped. `c && alert("Hello, World!");`, // Escape sequence not over-escaped. - `"Hello, World & O'Reilly\x21"`, + `"Hello, World & O'Reilly\u0021"`, `"greeting=H%69,\u0026addressee=(World)"`, `"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`, `",foo/,"`, @@ -162,7 +162,7 @@ func TestTypedContent(t *testing.T) { // Not JS escaped but HTML escaped. `c && alert("Hello, World!");`, // Escape sequence not over-escaped. - `"Hello, World & O'Reilly\x21"`, + `"Hello, World & O'Reilly\u0021"`, `"greeting=H%69,\u0026addressee=(World)"`, `"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`, `",foo/,"`, @@ -171,30 +171,30 @@ func TestTypedContent(t *testing.T) { { ``, []string{ - `\x3cb\x3e \x22foo%\x22 O\x27Reilly \x26bar;`, - `a[href =~ \x22\/\/example.com\x22]#foo`, - `Hello, \x3cb\x3eWorld\x3c\/b\x3e \x26amp;tc!`, - ` dir=\x22ltr\x22`, - `c \x26\x26 alert(\x22Hello, World!\x22);`, + `\u003cb\u003e \u0022foo%\u0022 O\u0027Reilly \u0026bar;`, + `a[href =~ \u0022\/\/example.com\u0022]#foo`, + `Hello, \u003cb\u003eWorld\u003c\/b\u003e \u0026amp;tc!`, + ` dir=\u0022ltr\u0022`, + `c \u0026\u0026 alert(\u0022Hello, World!\u0022);`, // Escape sequence not over-escaped. - `Hello, World \x26 O\x27Reilly\x21`, - `greeting=H%69,\x26addressee=(World)`, - `greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, + `Hello, World \u0026 O\u0027Reilly\u0021`, + `greeting=H%69,\u0026addressee=(World)`, + `greeting=H%69,\u0026addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, `,foo\/,`, }, }, { ``, []string{ - `\x3cb\x3e \x22foo%\x22 O\x27Reilly \x26bar;`, - `a[href =~ \x22\/\/example.com\x22]#foo`, - `Hello, \x3cb\x3eWorld\x3c\/b\x3e \x26amp;tc!`, - ` dir=\x22ltr\x22`, - `c \x26\x26 alert(\x22Hello, World!\x22);`, + `\u003cb\u003e \u0022foo%\u0022 O\u0027Reilly \u0026bar;`, + `a[href =~ \u0022\/\/example.com\u0022]#foo`, + `Hello, \u003cb\u003eWorld\u003c\/b\u003e \u0026amp;tc!`, + ` dir=\u0022ltr\u0022`, + `c \u0026\u0026 alert(\u0022Hello, World!\u0022);`, // Escape sequence not over-escaped. - `Hello, World \x26 O\x27Reilly\x21`, - `greeting=H%69,\x26addressee=(World)`, - `greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, + `Hello, World \u0026 O\u0027Reilly\u0021`, + `greeting=H%69,\u0026addressee=(World)`, + `greeting=H%69,\u0026addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`, `,foo\/,`, }, }, @@ -208,7 +208,7 @@ func TestTypedContent(t *testing.T) { // Not escaped. `c && alert("Hello, World!");`, // Escape sequence not over-escaped. - `"Hello, World & O'Reilly\x21"`, + `"Hello, World & O'Reilly\u0021"`, `"greeting=H%69,\u0026addressee=(World)"`, `"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`, `",foo/,"`, @@ -224,7 +224,7 @@ func TestTypedContent(t *testing.T) { `Hello, World &tc!`, ` dir="ltr"`, `c && alert("Hello, World!");`, - `Hello, World & O'Reilly\x21`, + `Hello, World & O'Reilly\u0021`, `greeting=H%69,&addressee=(World)`, `greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`, `,foo/,`, @@ -233,15 +233,15 @@ func TestTypedContent(t *testing.T) { { ``, "helper": `{{11}} of {{"<100>"}}`, }, - ``, + ``, }, // A non-recursive template that ends in a different context. // helper starts in jsCtxRegexp and ends in jsCtxDivOp. diff --git a/src/html/template/example_test.go b/src/html/template/example_test.go index 9d965f1..6cf936f 100644 --- a/src/html/template/example_test.go +++ b/src/html/template/example_test.go @@ -116,9 +116,9 @@ func Example_escape() { // "Fran & Freddie's Diner" <tasty@example.com> // "Fran & Freddie's Diner" <tasty@example.com> // "Fran & Freddie's Diner"32<tasty@example.com> - // \"Fran \x26 Freddie\'s Diner\" \x3Ctasty@example.com\x3E - // \"Fran \x26 Freddie\'s Diner\" \x3Ctasty@example.com\x3E - // \"Fran \x26 Freddie\'s Diner\"32\x3Ctasty@example.com\x3E + // \"Fran \u0026 Freddie\'s Diner\" \u003Ctasty@example.com\u003E + // \"Fran \u0026 Freddie\'s Diner\" \u003Ctasty@example.com\u003E + // \"Fran \u0026 Freddie\'s Diner\"32\u003Ctasty@example.com\u003E // %22Fran+%26+Freddie%27s+Diner%2232%3Ctasty%40example.com%3E } diff --git a/src/html/template/js.go b/src/html/template/js.go index 0e91458..ea9c183 100644 --- a/src/html/template/js.go +++ b/src/html/template/js.go @@ -163,7 +163,6 @@ func jsValEscaper(args ...interface{}) string { } // TODO: detect cycles before calling Marshal which loops infinitely on // cyclic data. This may be an unacceptable DoS risk. - b, err := json.Marshal(a) if err != nil { // Put a space before comment so that if it is flush against @@ -178,8 +177,8 @@ func jsValEscaper(args ...interface{}) string { // TODO: maybe post-process output to prevent it from containing // "", "", or "': `\x3e`, + '<': `\u003c`, + '>': `\u003e`, '\\': `\\`, } // jsStrNormReplacementTable is like jsStrReplacementTable but does not // overencode existing escapes since this table has no entry for `\`. var jsStrNormReplacementTable = []string{ - 0: `\0`, + 0: `\u0000`, '\t': `\t`, '\n': `\n`, - '\v': `\x0b`, // "\v" == "v" on IE 6. + '\v': `\u000b`, // "\v" == "v" on IE 6. '\f': `\f`, '\r': `\r`, // Encode HTML specials as hex so the output can be embedded // in HTML attributes without further encoding. - '"': `\x22`, - '&': `\x26`, - '\'': `\x27`, - '+': `\x2b`, + '"': `\u0022`, + '&': `\u0026`, + '\'': `\u0027`, + '+': `\u002b`, '/': `\/`, - '<': `\x3c`, - '>': `\x3e`, + '<': `\u003c`, + '>': `\u003e`, } - var jsRegexpReplacementTable = []string{ - 0: `\0`, + 0: `\u0000`, '\t': `\t`, '\n': `\n`, - '\v': `\x0b`, // "\v" == "v" on IE 6. + '\v': `\u000b`, // "\v" == "v" on IE 6. '\f': `\f`, '\r': `\r`, // Encode HTML specials as hex so the output can be embedded // in HTML attributes without further encoding. - '"': `\x22`, + '"': `\u0022`, '$': `\$`, - '&': `\x26`, - '\'': `\x27`, + '&': `\u0026`, + '\'': `\u0027`, '(': `\(`, ')': `\)`, '*': `\*`, - '+': `\x2b`, + '+': `\u002b`, '-': `\-`, '.': `\.`, '/': `\/`, - '<': `\x3c`, - '>': `\x3e`, + '<': `\u003c`, + '>': `\u003e`, '?': `\?`, '[': `\[`, '\\': `\\`, diff --git a/src/html/template/js_test.go b/src/html/template/js_test.go index 075adaa..d7ee47b 100644 --- a/src/html/template/js_test.go +++ b/src/html/template/js_test.go @@ -137,7 +137,7 @@ func TestJSValEscaper(t *testing.T) { {"foo", `"foo"`}, // Newlines. {"\r\n\u2028\u2029", `"\r\n\u2028\u2029"`}, - // "\v" == "v" on IE 6 so use "\x0b" instead. + // "\v" == "v" on IE 6 so use "\u000b" instead. {"\t\x0b", `"\t\u000b"`}, {struct{ X, Y int }{1, 2}, `{"X":1,"Y":2}`}, {[]interface{}{}, "[]"}, @@ -173,7 +173,7 @@ func TestJSStrEscaper(t *testing.T) { }{ {"", ``}, {"foo", `foo`}, - {"\u0000", `\0`}, + {"\u0000", `\u0000`}, {"\t", `\t`}, {"\n", `\n`}, {"\r", `\r`}, @@ -183,14 +183,14 @@ func TestJSStrEscaper(t *testing.T) { {"\\n", `\\n`}, {"foo\r\nbar", `foo\r\nbar`}, // Preserve attribute boundaries. - {`"`, `\x22`}, - {`'`, `\x27`}, + {`"`, `\u0022`}, + {`'`, `\u0027`}, // Allow embedding in HTML without further escaping. - {`&`, `\x26amp;`}, + {`&`, `\u0026amp;`}, // Prevent breaking out of text node and element boundaries. - {"", `\x3c\/script\x3e`}, - {"", `]]\x3e`}, + {"", `\u003c\/script\u003e`}, + {"", `]]\u003e`}, // https://dev.w3.org/html5/markup/aria/syntax.html#escaping-text-span // "The text in style, script, title, and textarea elements // must not have an escaping text span start that is not @@ -201,11 +201,11 @@ func TestJSStrEscaper(t *testing.T) { // allow regular text content to be interpreted as script // allowing script execution via a combination of a JS string // injection followed by an HTML text injection. - {"", `--\x3e`}, + {"", `--\u003e`}, // From https://code.google.com/p/doctype/wiki/ArticleUtf7 {"+ADw-script+AD4-alert(1)+ADw-/script+AD4-", - `\x2bADw-script\x2bAD4-alert(1)\x2bADw-\/script\x2bAD4-`, + `\u002bADw-script\u002bAD4-alert(1)\u002bADw-\/script\u002bAD4-`, }, // Invalid UTF-8 sequence {"foo\xA0bar", "foo\xA0bar"}, @@ -228,7 +228,7 @@ func TestJSRegexpEscaper(t *testing.T) { }{ {"", `(?:)`}, {"foo", `foo`}, - {"\u0000", `\0`}, + {"\u0000", `\u0000`}, {"\t", `\t`}, {"\n", `\n`}, {"\r", `\r`}, @@ -238,19 +238,19 @@ func TestJSRegexpEscaper(t *testing.T) { {"\\n", `\\n`}, {"foo\r\nbar", `foo\r\nbar`}, // Preserve attribute boundaries. - {`"`, `\x22`}, - {`'`, `\x27`}, + {`"`, `\u0022`}, + {`'`, `\u0027`}, // Allow embedding in HTML without further escaping. - {`&`, `\x26amp;`}, + {`&`, `\u0026amp;`}, // Prevent breaking out of text node and element boundaries. - {"", `\x3c\/script\x3e`}, - {"", `\]\]\x3e`}, + {"", `\u003c\/script\u003e`}, + {"", `\]\]\u003e`}, // Escaping text spans. - {"", `\-\-\x3e`}, + {"", `\-\-\u003e`}, {"*", `\*`}, - {"+", `\x2b`}, + {"+", `\u002b`}, {"?", `\?`}, {"[](){}", `\[\]\(\)\{\}`}, {"$foo|x.y", `\$foo\|x\.y`}, @@ -284,27 +284,27 @@ func TestEscapersOnLower7AndSelectHighCodepoints(t *testing.T) { { "jsStrEscaper", jsStrEscaper, - "\\0\x01\x02\x03\x04\x05\x06\x07" + - "\x08\\t\\n\\x0b\\f\\r\x0E\x0F" + - "\x10\x11\x12\x13\x14\x15\x16\x17" + - "\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" + - ` !\x22#$%\x26\x27()*\x2b,-.\/` + - `0123456789:;\x3c=\x3e?` + + `\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007` + + `\u0008\t\n\u000b\f\r\u000e\u000f` + + `\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017` + + `\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f` + + ` !\u0022#$%\u0026\u0027()*\u002b,-.\/` + + `0123456789:;\u003c=\u003e?` + `@ABCDEFGHIJKLMNO` + `PQRSTUVWXYZ[\\]^_` + "`abcdefghijklmno" + - "pqrstuvwxyz{|}~\x7f" + + "pqrstuvwxyz{|}~\u007f" + "\u00A0\u0100\\u2028\\u2029\ufeff\U0001D11E", }, { "jsRegexpEscaper", jsRegexpEscaper, - "\\0\x01\x02\x03\x04\x05\x06\x07" + - "\x08\\t\\n\\x0b\\f\\r\x0E\x0F" + - "\x10\x11\x12\x13\x14\x15\x16\x17" + - "\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" + - ` !\x22#\$%\x26\x27\(\)\*\x2b,\-\.\/` + - `0123456789:;\x3c=\x3e\?` + + `\u0000\u0001\u0002\u0003\u0004\u0005\u0006\u0007` + + `\u0008\t\n\u000b\f\r\u000e\u000f` + + `\u0010\u0011\u0012\u0013\u0014\u0015\u0016\u0017` + + `\u0018\u0019\u001a\u001b\u001c\u001d\u001e\u001f` + + ` !\u0022#\$%\u0026\u0027\(\)\*\u002b,\-\.\/` + + `0123456789:;\u003c=\u003e\?` + `@ABCDEFGHIJKLMNO` + `PQRSTUVWXYZ\[\\\]\^_` + "`abcdefghijklmno" + diff --git a/src/html/template/template_test.go b/src/html/template/template_test.go index 13e6ba4..86bd4db 100644 --- a/src/html/template/template_test.go +++ b/src/html/template/template_test.go @@ -6,6 +6,7 @@ package template_test import ( "bytes" + "encoding/json" . "html/template" "strings" "testing" @@ -121,6 +122,44 @@ func TestNumbers(t *testing.T) { c.mustExecute(c.root, nil, "12.34 7.5") } +func TestStringsInScriptsWithJsonContentTypeAreCorrectlyEscaped(t *testing.T) { + // See #33671 and #37634 for more context on this. + tests := []struct{ name, in string }{ + {"empty", ""}, + {"invalid", string(rune(-1))}, + {"null", "\u0000"}, + {"unit separator", "\u001F"}, + {"tab", "\t"}, + {"gt and lt", "<>"}, + {"quotes", `'"`}, + {"ASCII letters", "ASCII letters"}, + {"Unicode", "ʕ⊙ϖ⊙ʔ"}, + {"Pizza", "P"}, + } + const ( + prefix = `` + templ = prefix + `"{{.}}"` + suffix + ) + tpl := Must(New("JS string is JSON string").Parse(templ)) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + if err := tpl.Execute(&buf, tt.in); err != nil { + t.Fatalf("Cannot render template: %v", err) + } + trimmed := bytes.TrimSuffix(bytes.TrimPrefix(buf.Bytes(), []byte(prefix)), []byte(suffix)) + var got string + if err := json.Unmarshal(trimmed, &got); err != nil { + t.Fatalf("Cannot parse JS string %q as JSON: %v", trimmed[1:len(trimmed)-1], err) + } + if got != tt.in { + t.Errorf("Serialization changed the string value: got %q want %q", got, tt.in) + } + }) + } +} + type testCase struct { t *testing.T root *Template diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go index 77294ed..b8a809e 100644 --- a/src/text/template/exec_test.go +++ b/src/text/template/exec_test.go @@ -911,9 +911,9 @@ func TestJSEscaping(t *testing.T) { {`Go "jump" \`, `Go \"jump\" \\`}, {`Yukihiro says "今日は世界"`, `Yukihiro says \"今日は世界\"`}, {"unprintable \uFDFF", `unprintable \uFDFF`}, - {``, `\x3Chtml\x3E`}, - {`no = in attributes`, `no \x3D in attributes`}, - {`' does not become HTML entity`, `\x26#x27; does not become HTML entity`}, + {``, `\u003Chtml\u003E`}, + {`no = in attributes`, `no \u003D in attributes`}, + {`' does not become HTML entity`, `\u0026#x27; does not become HTML entity`}, } for _, tc := range testCases { s := JSEscapeString(tc.in) diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go index 46125bc..f3de9fb 100644 --- a/src/text/template/funcs.go +++ b/src/text/template/funcs.go @@ -640,10 +640,10 @@ var ( jsBackslash = []byte(`\\`) jsApos = []byte(`\'`) jsQuot = []byte(`\"`) - jsLt = []byte(`\x3C`) - jsGt = []byte(`\x3E`) - jsAmp = []byte(`\x26`) - jsEq = []byte(`\x3D`) + jsLt = []byte(`\u003C`) + jsGt = []byte(`\u003E`) + jsAmp = []byte(`\u0026`) + jsEq = []byte(`\u003D`) ) // JSEscape writes to w the escaped JavaScript equivalent of the plain text data b. -- 2.7.4