html/template: inserted escapers are not properly merged with predefined escapers with explicit arguments #19353
Labels
FrozenDueToAge
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Comments
ALTree
added
the
NeedsInvestigation
|
CL https://golang.org/cl/37880 mentions this issue. |
gopherbot
pushed a commit
that referenced
this issue
Apr 10, 2017
…ring 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 <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
|
This is no longer an issue now that golang.org/cl/37880 has landed. |
|
CL https://golang.org/cl/40936 mentions this issue. |
gopherbot
pushed a commit
that referenced
this issue
May 5, 2017
Allow the predefined escapers "html", "urlquery", and "js" to be used in pipelines when they have no potential to affect the correctness or safety of the escaped pipeline output. Specifically: - "urlquery" may be used if it is the last command in the pipeline. - "html" may be used if it is the last command in the pipeline, and the pipeline does not occur in an unquoted HTML attribute value context. - "js" may be used in any pipeline, since it does not affect the merging of contextual escapers. This change will loosens the restrictions on predefined escapers introduced in golang.org/cl/37880, which will hopefully ease the upgrade path for existing template users. This change brings back the escaper-merging logic, and associated unit tests, that were removed in golang.org/cl/37880. However, a few notable changes have been made: - "_html_template_nospaceescaper" is no longer considered equivalent to "html", since the former escapes spaces, while the latter does not (see #19345). This change should not silently break any templates, since pipelines where this substituion will happen will already trigger an explicit error. - An "_eval_args_" internal directive has been added to handle pipelines containing a single explicit call to a predefined escaper, e.g. {{html .X}} (see #19353). Also, the HTMLEscape function called by the predefined text/template "html" function now escapes the NULL character as well. This effectively makes it as secure as the internal html/template HTML escapers (see #19345). While this change is backward-incompatible, it will only affect illegitimate uses of this escaper, since the NULL character is always illegal in valid HTML. Fixes #19952 Change-Id: I9b5570a80a3ea284b53901e6a1f842fc59b33d3a Reviewed-on: https://go-review.googlesource.com/40936 Reviewed-by: Russ Cox <rsc@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
version: go version devel +4cce27a3fa Sat Jan 21 03:20:55 2017 +0000 linux/amd64
env: linux amd64
When we have
the template
is rewritten into
after contextual auto-escaping, which evaluates to
However, the template
is rewritten into
after contextual auto-escaping, which evaluates to
which is incorrect, since the URL escapers will be called with no arguments, and will thus have no effect on the user-supplied value
.X. This happens because of the html/template escaper's attempt to merge inserted escapers with user-supplied escapers, described in #19336.The text was updated successfully, but these errors were encountered: