diff options
Diffstat (limited to 'tpl')
21 files changed, 259 insertions, 142 deletions
diff --git a/tpl/internal/go_templates/htmltemplate/context.go b/tpl/internal/go_templates/htmltemplate/context.go index 146a95d03..9f592b57f 100644 --- a/tpl/internal/go_templates/htmltemplate/context.go +++ b/tpl/internal/go_templates/htmltemplate/context.go @@ -121,6 +121,8 @@ const ( stateJSDqStr // stateJSSqStr occurs inside a JavaScript single quoted string. stateJSSqStr + // stateJSBqStr occurs inside a JavaScript back quoted string. + stateJSBqStr // stateJSRegexp occurs inside a JavaScript regexp literal. stateJSRegexp // stateJSBlockCmt occurs inside a JavaScript /* block comment */. diff --git a/tpl/internal/go_templates/htmltemplate/css.go b/tpl/internal/go_templates/htmltemplate/css.go index 890a0c6b2..f650d8b3e 100644 --- a/tpl/internal/go_templates/htmltemplate/css.go +++ b/tpl/internal/go_templates/htmltemplate/css.go @@ -238,7 +238,7 @@ func cssValueFilter(args ...any) string { // inside a string that might embed JavaScript source. for i, c := range b { switch c { - case 0, '"', '\'', '(', ')', '/', ';', '@', '[', '\\', ']', '`', '{', '}': + case 0, '"', '\'', '(', ')', '/', ';', '@', '[', '\\', ']', '`', '{', '}', '<', '>': return filterFailsafe case '-': // Disallow <!-- or -->. diff --git a/tpl/internal/go_templates/htmltemplate/css_test.go b/tpl/internal/go_templates/htmltemplate/css_test.go index 7d8ad8b59..f44568930 100644 --- a/tpl/internal/go_templates/htmltemplate/css_test.go +++ b/tpl/internal/go_templates/htmltemplate/css_test.go @@ -234,6 +234,8 @@ func TestCSSValueFilter(t *testing.T) { {`-exp\000052 ession(alert(1337))`, "ZgotmplZ"}, {`-expre\0000073sion`, "-expre\x073sion"}, {`@import url evil.css`, "ZgotmplZ"}, + {"<", "ZgotmplZ"}, + {">", "ZgotmplZ"}, } for _, test := range tests { got := cssValueFilter(test.css) diff --git a/tpl/internal/go_templates/htmltemplate/doc.go b/tpl/internal/go_templates/htmltemplate/doc.go index 8422b4921..98b5658f4 100644 --- a/tpl/internal/go_templates/htmltemplate/doc.go +++ b/tpl/internal/go_templates/htmltemplate/doc.go @@ -231,5 +231,12 @@ Least Surprise Property: "A developer (or code reviewer) familiar with HTML, CSS, and JavaScript, who knows that contextual autoescaping happens should be able to look at a {{.}} and correctly infer what sanitization happens." + +As a consequence of the Least Surprise Property, template actions within an +ECMAScript 6 template literal are disabled by default. +Handling string interpolation within these literals is rather complex resulting +in no clear safe way to support it. +To re-enable template actions within ECMAScript 6 template literals, use the +GODEBUG=jstmpllitinterp=1 environment variable. */ package template diff --git a/tpl/internal/go_templates/htmltemplate/error.go b/tpl/internal/go_templates/htmltemplate/error.go index 916b41a82..0a62563cf 100644 --- a/tpl/internal/go_templates/htmltemplate/error.go +++ b/tpl/internal/go_templates/htmltemplate/error.go @@ -215,6 +215,19 @@ const ( // pipeline occurs in an unquoted attribute value context, "html" is // disallowed. Avoid using "html" and "urlquery" entirely in new templates. ErrPredefinedEscaper + + // errJSTmplLit: "... appears in a JS template literal" + // Example: + // <script>var tmpl = `{{.Interp}`</script> + // Discussion: + // Package html/template does not support actions inside of JS template + // literals. + // + // TODO(rolandshoemaker): we cannot add this as an exported error in a minor + // release, since it is backwards incompatible with the other minor + // releases. As such we need to leave it unexported, and then we'll add it + // in the next major release. + errJSTmplLit ) func (e *Error) Error() string { diff --git a/tpl/internal/go_templates/htmltemplate/escape.go b/tpl/internal/go_templates/htmltemplate/escape.go index 3aac865ef..ba9b4bbb8 100644 --- a/tpl/internal/go_templates/htmltemplate/escape.go +++ b/tpl/internal/go_templates/htmltemplate/escape.go @@ -161,6 +161,8 @@ func (e *escaper) escape(c context, n parse.Node) context { panic("escaping " + n.String() + " is unimplemented") } +// var debugAllowActionJSTmpl = godebug.New("jstmpllitinterp") + // escapeAction escapes an action template node. func (e *escaper) escapeAction(c context, n *parse.ActionNode) context { if len(n.Pipe.Decl) != 0 { @@ -224,6 +226,15 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context { c.jsCtx = jsCtxDivOp case stateJSDqStr, stateJSSqStr: s = append(s, "_html_template_jsstrescaper") + case stateJSBqStr: + if SecurityAllowActionJSTmpl.Load() { // .Value() == "1" { + s = append(s, "_html_template_jsstrescaper") + } else { + return context{ + state: stateError, + err: errorf(errJSTmplLit, n, n.Line, "%s appears in a JS template literal", n), + } + } case stateJSRegexp: s = append(s, "_html_template_jsregexpescaper") case stateCSS: @@ -370,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/tpl/internal/go_templates/htmltemplate/escape_test.go b/tpl/internal/go_templates/htmltemplate/escape_test.go index a08ea57ef..680ba6fa7 100644 --- a/tpl/internal/go_templates/htmltemplate/escape_test.go +++ b/tpl/internal/go_templates/htmltemplate/escape_test.go @@ -683,38 +683,49 @@ 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 { - tmpl := New(test.name) - tmpl = Must(tmpl.Parse(test.input)) - // Check for bug 6459: Tree field was not set in Parse. - if tmpl.Tree != tmpl.text.Tree { - t.Errorf("%s: tree not set properly", test.name) - continue - } - b := new(strings.Builder) - if err := tmpl.Execute(b, data); err != nil { - t.Errorf("%s: template execution failed: %s", test.name, err) - continue - } - if w, g := test.output, b.String(); w != g { - t.Errorf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g) - continue - } - b.Reset() - if err := tmpl.Execute(b, pdata); err != nil { - t.Errorf("%s: template execution failed for pointer: %s", test.name, err) - continue - } - if w, g := test.output, b.String(); w != g { - t.Errorf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g) - continue - } - if tmpl.Tree != tmpl.text.Tree { - t.Errorf("%s: tree mismatch", test.name) - continue - } + t.Run(test.name, func(t *testing.T) { + tmpl := New(test.name) + tmpl = Must(tmpl.Parse(test.input)) + // Check for bug 6459: Tree field was not set in Parse. + if tmpl.Tree != tmpl.text.Tree { + t.Fatalf("%s: tree not set properly", test.name) + } + b := new(strings.Builder) + if err := tmpl.Execute(b, data); err != nil { + t.Fatalf("%s: template execution failed: %s", test.name, err) + } + if w, g := test.output, b.String(); w != g { + t.Fatalf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g) + } + b.Reset() + if err := tmpl.Execute(b, pdata); err != nil { + t.Fatalf("%s: template execution failed for pointer: %s", test.name, err) + } + if w, g := test.output, b.String(); w != g { + t.Fatalf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g) + } + if tmpl.Tree != tmpl.text.Tree { + t.Fatalf("%s: tree mismatch", test.name) + } + }) } } @@ -941,6 +952,10 @@ func TestErrors(t *testing.T) { "{{range .Items}}<a{{if .X}}{{end}}>{{if .X}}{{break}}{{end}}{{end}}", "", }, + { + "<script>var a = `${a+b}`</script>`", + "", + }, // Error cases. { "{{if .Cond}}<a{{end}}", @@ -1087,6 +1102,10 @@ func TestErrors(t *testing.T) { // html is allowed since it is the last command in the pipeline, but urlquery is not. `predefined escaper "urlquery" disallowed in template`, }, + { + "<script>var tmpl = `asd {{.}}`;</script>", + `{{.}} appears in a JS template literal`, + }, } for _, test := range tests { buf := new(bytes.Buffer) @@ -1309,6 +1328,10 @@ func TestEscapeText(t *testing.T) { context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript}, }, { + "<a onclick=\"`foo", + context{state: stateJSBqStr, delim: delimDoubleQuote, attr: attrScript}, + }, + { `<A ONCLICK="'`, context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript}, }, diff --git a/tpl/internal/go_templates/htmltemplate/html.go b/tpl/internal/go_templates/htmltemplate/html.go index bcca0b51a..a181699a5 100644 --- a/tpl/internal/go_templates/htmltemplate/html.go +++ b/tpl/internal/go_templates/htmltemplate/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) } diff --git a/tpl/internal/go_templates/htmltemplate/hugo_template.go b/tpl/internal/go_templates/htmltemplate/hugo_template.go index 99edf8f68..98e03ad3c 100644 --- a/tpl/internal/go_templates/htmltemplate/hugo_template.go +++ b/tpl/internal/go_templates/htmltemplate/hugo_template.go @@ -14,9 +14,15 @@ package template import ( + "sync/atomic" + template "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate" ) +// See https://github.com/golang/go/issues/59234 +// Moved here to avoid dependency on Go's internal/debug package. +var SecurityAllowActionJSTmpl atomic.Bool + /* This files contains the Hugo related addons. All the other files in this diff --git a/tpl/internal/go_templates/htmltemplate/js.go b/tpl/internal/go_templates/htmltemplate/js.go index 6187dc036..3b5178eb1 100644 --- a/tpl/internal/go_templates/htmltemplate/js.go +++ b/tpl/internal/go_templates/htmltemplate/js.go @@ -14,6 +14,11 @@ import ( "unicode/utf8" ) +// jsWhitespace contains all of the JS whitespace characters, as defined +// by the \s character class. +// See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions/Character_classes. +const jsWhitespace = "\f\n\r\t\v\u0020\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff" + // nextJSCtx returns the context that determines whether a slash after the // given run of tokens starts a regular expression instead of a division // operator: / or /=. @@ -27,7 +32,8 @@ import ( // JavaScript 2.0 lexical grammar and requires one token of lookbehind: // https://www.mozilla.org/js/language/js20-2000-07/rationale/syntax.html func nextJSCtx(s []byte, preceding jsCtx) jsCtx { - s = bytes.TrimRight(s, "\t\n\f\r \u2028\u2029") + // Trim all JS whitespace characters + s = bytes.TrimRight(s, jsWhitespace) if len(s) == 0 { return preceding } @@ -309,6 +315,7 @@ var jsStrReplacementTable = []string{ // Encode HTML specials as hex so the output can be embedded // in HTML attributes without further encoding. '"': `\u0022`, + '`': `\u0060`, '&': `\u0026`, '\'': `\u0027`, '+': `\u002b`, @@ -332,6 +339,7 @@ var jsStrNormReplacementTable = []string{ '"': `\u0022`, '&': `\u0026`, '\'': `\u0027`, + '`': `\u0060`, '+': `\u002b`, '/': `\/`, '<': `\u003c`, diff --git a/tpl/internal/go_templates/htmltemplate/js_test.go b/tpl/internal/go_templates/htmltemplate/js_test.go index 483a3694f..67a921337 100644 --- a/tpl/internal/go_templates/htmltemplate/js_test.go +++ b/tpl/internal/go_templates/htmltemplate/js_test.go @@ -83,14 +83,17 @@ func TestNextJsCtx(t *testing.T) { {jsCtxDivOp, "0"}, // Dots that are part of a number are div preceders. {jsCtxDivOp, "0."}, + // Some JS interpreters treat NBSP as a normal space, so + // we must too in order to properly escape things. + {jsCtxRegexp, "=\u00A0"}, } for _, test := range tests { - if nextJSCtx([]byte(test.s), jsCtxRegexp) != test.jsCtx { - t.Errorf("want %s got %q", test.jsCtx, test.s) + if ctx := nextJSCtx([]byte(test.s), jsCtxRegexp); ctx != test.jsCtx { + t.Errorf("%q: want %s got %s", test.s, test.jsCtx, ctx) } - if nextJSCtx([]byte(test.s), jsCtxDivOp) != test.jsCtx { - t.Errorf("want %s got %q", test.jsCtx, test.s) + if ctx := nextJSCtx([]byte(test.s), jsCtxDivOp); ctx != test.jsCtx { + t.Errorf("%q: want %s got %s", test.s, test.jsCtx, ctx) } } @@ -294,7 +297,7 @@ func TestEscapersOnLower7AndSelectHighCodepoints(t *testing.T) { `0123456789:;\u003c=\u003e?` + `@ABCDEFGHIJKLMNO` + `PQRSTUVWXYZ[\\]^_` + - "`abcdefghijklmno" + + "\\u0060abcdefghijklmno" + "pqrstuvwxyz{|}~\u007f" + "\u00A0\u0100\\u2028\\u2029\ufeff\U0001D11E", }, diff --git a/tpl/internal/go_templates/htmltemplate/jsctx_string.go b/tpl/internal/go_templates/htmltemplate/jsctx_string.go index dd1d87ee4..23948934c 100644 --- a/tpl/internal/go_templates/htmltemplate/jsctx_string.go +++ b/tpl/internal/go_templates/htmltemplate/jsctx_string.go @@ -4,6 +4,15 @@ package template import "strconv" +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[jsCtxRegexp-0] + _ = x[jsCtxDivOp-1] + _ = x[jsCtxUnknown-2] +} + const _jsCtx_name = "jsCtxRegexpjsCtxDivOpjsCtxUnknown" var _jsCtx_index = [...]uint8{0, 11, 21, 33} diff --git a/tpl/internal/go_templates/htmltemplate/state_string.go b/tpl/internal/go_templates/htmltemplate/state_string.go index 05104be89..6fb1a6eeb 100644 --- a/tpl/internal/go_templates/htmltemplate/state_string.go +++ b/tpl/internal/go_templates/htmltemplate/state_string.go @@ -4,9 +4,42 @@ package template import "strconv" -const _state_name = "stateTextstateTagstateAttrNamestateAfterNamestateBeforeValuestateHTMLCmtstateRCDATAstateAttrstateURLstateSrcsetstateJSstateJSDqStrstateJSSqStrstateJSRegexpstateJSBlockCmtstateJSLineCmtstateCSSstateCSSDqStrstateCSSSqStrstateCSSDqURLstateCSSSqURLstateCSSURLstateCSSBlockCmtstateCSSLineCmtstateError" +func _() { + // An "invalid array index" compiler error signifies that the constant values have changed. + // Re-run the stringer command to generate them again. + var x [1]struct{} + _ = x[stateText-0] + _ = x[stateTag-1] + _ = x[stateAttrName-2] + _ = x[stateAfterName-3] + _ = x[stateBeforeValue-4] + _ = x[stateHTMLCmt-5] + _ = x[stateRCDATA-6] + _ = x[stateAttr-7] + _ = x[stateURL-8] + _ = x[stateSrcset-9] + _ = x[stateJS-10] + _ = x[stateJSDqStr-11] + _ = x[stateJSSqStr-12] + _ = x[stateJSBqStr-13] + _ = x[stateJSRegexp-14] + _ = x[stateJSBlockCmt-15] + _ = x[stateJSLineCmt-16] + _ = x[stateCSS-17] + _ = x[stateCSSDqStr-18] + _ = x[stateCSSSqStr-19] + _ = x[stateCSSDqURL-20] + _ = x[stateCSSSqURL-21] + _ = x[stateCSSURL-22] + _ = x[stateCSSBlockCmt-23] + _ = x[stateCSSLineCmt-24] + _ = x[stateError-25] + _ = x[stateDead-26] +} + +const _state_name = "stateTextstateTagstateAttrNamestateAfterNamestateBeforeValuestateHTMLCmtstateRCDATAstateAttrstateURLstateSrcsetstateJSstateJSDqStrstateJSSqStrstateJSBqStrstateJSRegexpstateJSBlockCmtstateJSLineCmtstateCSSstateCSSDqStrstateCSSSqStrstateCSSDqURLstateCSSSqURLstateCSSURLstateCSSBlockCmtstateCSSLineCmtstateErrorstateDead" -var _state_index = [...]uint16{0, 9, 17, 30, 44, 60, 72, 83, 92, 100, 111, 118, 130, 142, 155, 170, 184, 192, 205, 218, 231, 244, 255, 271, 286, 296} +var _state_index = [...]uint16{0, 9, 17, 30, 44, 60, 72, 83, 92, 100, 111, 118, 130, 142, 154, 167, 182, 196, 204, 217, 230, 243, 256, 267, 283, 298, 308, 317} func (i state) String() string { if i >= state(len(_state_index)-1) { diff --git a/tpl/internal/go_templates/htmltemplate/transition.go b/tpl/internal/go_templates/htmltemplate/transition.go index 06df67933..92eb35190 100644 --- a/tpl/internal/go_templates/htmltemplate/transition.go +++ b/tpl/internal/go_templates/htmltemplate/transition.go @@ -27,6 +27,7 @@ var transitionFunc = [...]func(context, []byte) (context, int){ stateJS: tJS, stateJSDqStr: tJSDelimited, stateJSSqStr: tJSDelimited, + stateJSBqStr: tJSDelimited, stateJSRegexp: tJSDelimited, stateJSBlockCmt: tBlockCmt, stateJSLineCmt: tLineCmt, @@ -262,7 +263,7 @@ func tURL(c context, s []byte) (context, int) { // tJS is the context transition function for the JS state. func tJS(c context, s []byte) (context, int) { - i := bytes.IndexAny(s, `"'/`) + i := bytes.IndexAny(s, "\"`'/") if i == -1 { // Entire input is non string, comment, regexp tokens. c.jsCtx = nextJSCtx(s, c.jsCtx) @@ -274,6 +275,8 @@ func tJS(c context, s []byte) (context, int) { c.state, c.jsCtx = stateJSDqStr, jsCtxRegexp case '\'': c.state, c.jsCtx = stateJSSqStr, jsCtxRegexp + case '`': + c.state, c.jsCtx = stateJSBqStr, jsCtxRegexp case '/': switch { case i+1 < len(s) && s[i+1] == '/': @@ -303,6 +306,8 @@ func tJSDelimited(c context, s []byte) (context, int) { switch c.state { case stateJSSqStr: specials = `\'` + case stateJSBqStr: + specials = "`\\" case stateJSRegexp: specials = `\/[]` } diff --git a/tpl/internal/go_templates/testenv/exec.go b/tpl/internal/go_templates/testenv/exec.go index ca4023647..13b4c7102 100644 --- a/tpl/internal/go_templates/testenv/exec.go +++ b/tpl/internal/go_templates/testenv/exec.go @@ -9,11 +9,9 @@ import ( "os" "os/exec" "runtime" - "strconv" "strings" "sync" "testing" - "time" ) // HasExec reports whether the current system can start new processes @@ -84,87 +82,7 @@ func CleanCmdEnv(cmd *exec.Cmd) *exec.Cmd { // - fails the test if the command does not complete before the test's deadline, and // - sets a Cleanup function that verifies that the test did not leak a subprocess. func CommandContext(t testing.TB, ctx context.Context, name string, args ...string) *exec.Cmd { - t.Helper() - MustHaveExec(t) - - var ( - cancelCtx context.CancelFunc - gracePeriod time.Duration // unlimited unless the test has a deadline (to allow for interactive debugging) - ) - - if t, ok := t.(interface { - testing.TB - Deadline() (time.Time, bool) - }); ok { - if td, ok := t.Deadline(); ok { - // Start with a minimum grace period, just long enough to consume the - // output of a reasonable program after it terminates. - gracePeriod = 100 * time.Millisecond - if s := os.Getenv("GO_TEST_TIMEOUT_SCALE"); s != "" { - scale, err := strconv.Atoi(s) - if err != nil { - t.Fatalf("invalid GO_TEST_TIMEOUT_SCALE: %v", err) - } - gracePeriod *= time.Duration(scale) - } - - // If time allows, increase the termination grace period to 5% of the - // test's remaining time. - testTimeout := time.Until(td) - if gp := testTimeout / 20; gp > gracePeriod { - gracePeriod = gp - } - - // When we run commands that execute subprocesses, we want to reserve two - // grace periods to clean up: one for the delay between the first - // termination signal being sent (via the Cancel callback when the Context - // expires) |