From 9ffd9339da503b50571ec6806e5d6d2cf5d5912a Mon Sep 17 00:00:00 2001 From: Samuel Tan Date: Tue, 4 Apr 2017 18:26:21 -0700 Subject: html/template: panic if predefined escapers are found in pipelines during rewriting Report an error if a predefined escaper (i.e. "html", "urlquery", or "js") is found in a pipeline that will be rewritten by the contextual auto-escaper, instead of trying to merge the escaper-inserted escaping directives with these predefined escapers. This merging behavior is a source of several security and correctness bugs (eee #19336, #19345, #19352, and #19353.) This merging logic was originally intended to ease migration of text/template templates with user-defined escapers to html/template. Now that migration is no longer an issue, this logic can be safely removed. NOTE: this is a backward-incompatible change that fixes known security bugs (see linked issues for more details). It will explicitly break users that attempt to execute templates with pipelines containing predefined escapers. Fixes #19336, #19345, #19352, #19353 Change-Id: I46b0ca8a2809d179c13c0d4f42b63126ed1c3b49 Reviewed-on: https://go-review.googlesource.com/37880 Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot Reviewed-by: Russ Cox --- src/html/template/error.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'src/html/template/error.go') diff --git a/src/html/template/error.go b/src/html/template/error.go index cbcaf92e4a..3b70ba1ec8 100644 --- a/src/html/template/error.go +++ b/src/html/template/error.go @@ -183,6 +183,27 @@ const ( // Look for missing semicolons inside branches, and maybe add // parentheses to make it clear which interpretation you intend. ErrSlashAmbig + + // ErrPredefinedEscaper: "predefined escaper ... disallowed in template" + // Example: + // + // Discussion: + // Package html/template already contextually escapes all pipelines to + // produce HTML output safe against code injection. Manually escaping + // pipeline output using the predefined escapers "html", "urlquery", or "js" + // is unnecessary, and might affect the correctness or safety of the escaped + // pipeline output. In the above example, "urlquery" should simply be + // removed from the pipeline so that escaping is performed solely by the + // contextual autoescaper. + // If the predefined escaper occurs in the middle of a pipeline where + // subsequent commands expect escaped input, e.g. + // {{.X | html | makeALink}} + // where makeALink does + // return "link" + // consider refactoring the surrounding template to make use of the + // contextual autoescaper, i.e. + // link + ErrPredefinedEscaper ) func (e *Error) Error() string { -- cgit v1.3-5-g9baa