Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

html/template: <script> tags with type "text/template" now escapes EJS templates #18569

Closed
anthonybishopric opened this issue Jan 9, 2017 · 18 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Security
Milestone

Comments

@anthonybishopric
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

Go 1.8

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build705350648=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

When emitting an EJS template (interpolation happens through the <%= directive) inside a Golang HTML template, the <%= is escaped to &lt;%=. This does not happen on versions of Go < 1.8. The following play.golang.org link works as expected.

https://play.golang.org/p/BXzy9OWSSq

What did you expect to see?

  <script id="new-key" type="text/template">
      <tr class="api-key">
        <td class="api-key-cell"><%= key %></td>
      </tr>
   </script>

What did you see instead?

  <script id="new-key" type="text/template">
      <tr class="api-key">
        <td class="api-key-cell">&lt;%= key %></td>
      </tr>
   </script>

I'm content with this being a desired security addition to Golang HTML templates, but I wanted to raise this as an issue for existing users who embed Javascript templates into their Golang templates.

@titanous
Copy link
Member

titanous commented Jan 9, 2017

This appears to have been intentionally implemented in ffd1c78 (https://golang.org/cl/14336) by @rsc

@titanous titanous added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 9, 2017
@kennygrant
Copy link
Contributor

kennygrant commented Jan 12, 2017

One possible workaround is to insert a template data key to insert the script type at evaluation time (tested on 1.8rc1)

https://play.golang.org/p/HbcDfwbXwq

this outputs the html you want without escaping on 1.8rc1.

<td class="api-key-cell"><%= key %></td>

@anthonybishopric
Copy link
Author

@kennygrant that might be acceptable for smaller projects, but I have a number of these templates and also rely on some amount of template interpolation within them too. Consider:

<script type="text/template" id="form-template">
    <form class="new-form" path="/resource/{{.Id}}">
         <div><%= key %></div>    
    </form>
</script>

@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 12, 2017
@bradfitz bradfitz added this to the Go1.8Maybe milestone Jan 12, 2017
@bradfitz
Copy link
Contributor

Assigning to @rsc to decide whether this is an acceptable change/regression for Go 1.8.

@rsc
Copy link
Contributor

rsc commented Jan 17, 2017

Wait, is the claim that "text/template" is the script type for EJS templates? When I saw that before I thought people were using that for Go templates.

@anthonybishopric
Copy link
Author

This bug is filed with respect to the string "text/template" as in <script type="text/template">, not the Go package text/template.

@anthonybishopric
Copy link
Author

anthonybishopric commented Jan 17, 2017

From what I understand, marking script tags that you intend to use HTML templates as <script type="text/template"> is a common practice and not necessarily associated with EJS or any one other Javascript template framework. If the solution is just to make up a different value for the type attribute so that it won't be escaped, that is an acceptable solution.

Not sure if that answers your question - let me know if I can clarify anything! Thanks @rsc

@ghost
Copy link

ghost commented Jan 17, 2017

It is a common practice to store HTML fragments inside script tags with a special type (usually text/template), like <script id="some-tmpl" type="text/template"><div class="...">...</div></script>, and then somewhere in the JavaScript code retrieving the contents and using it (e.g. in jQuery: $('#some-tmpl').html()).

@rsc
Copy link
Contributor

rsc commented Jan 18, 2017

OK, so what was happening before is that the <script> body was being treated as JavaScript, preserving things like < > symbols. Now it's being treated like HTML, and html/template doesn't recognize <%= %> and <% %> tags, so it de-tags them. This happens both inside and outside <script>s. The only workaround would be to tag your script javascript explicitly (or use the odd .Type hack, which probably shouldn't work).

$ cat /tmp/x.go
package main

import (
	"html/template"
	"os"
)

const tmpl = `
  <script id="new-key" type="text/template">
  <%= text/template %>
  </script>
  <script id="new-key" type="application/javascript">
  <%= application/javascript %>
  </script>
  <div id="my-div">
  <%= div %>
  </div>
`

func main() {
	template.Must(template.New("").Parse(tmpl)).Execute(os.Stdout, nil)
}
$ go1.7 run /tmp/x.go

  <script id="new-key" type="text/template">
  <%= text/template %>
  </script>
  <script id="new-key" type="application/javascript">
  <%= application/javascript %>
  </script>
  <div id="my-div">
  &lt;%= div %>
  </div>
$ go run /tmp/x.go

  <script id="new-key" type="text/template">
  &lt;%= text/template %>
  </script>
  <script id="new-key" type="application/javascript">
  <%= application/javascript %>
  </script>
  <div id="my-div">
  &lt;%= div %>
  </div>
$ 

The implementation of ffd1c78 makes scripts with unrecognized (non-JS) types treated like any other tag, meaning the body is HTML like the rest of the page. That seems about right, actually, but not here. I wonder if html/template should further understand that <% %> and <%= %> contain Javascript, or if that's too specialized. It sounds like otherwise we have to look for the </script> tag and leave everything uninterpreted until then.

@ghost
Copy link

ghost commented Jan 18, 2017

I think the new behavior is the correct one. <% is neither valid HTML nor valid JS and should not be left unescaped by html/template, so as to not open the road to HTML/JS injection, and allowing specifically <% and %> seems too specialized (and error-prone). Those who wish to process EJS templates can make a copy of the html/template package and modify it accordingly to their needs. Another possibility is to opt in to not escaping things - say, a field in template.Template that you need to set to true before executing (parsing?) the template.

@anthonybishopric
Copy link
Author

Mostly I'm confused by the intention of the this statement from the html/template documentation:

The security model used by this package assumes that template authors are trusted, while Execute's data parameter is not.

Escaping content that is actually in the raw template file itself seems like it presumes that the author is also not trusted and that this package is attempting to correct something the author has done wrong. If that statement were true, then an injected template.Javascript value should be treated the same was as a literal in the template itself, right? It seems redundant.

Agreed that treating <% or %> as special is probably an over-specialized thing to do. But there should be some fairly bold warning presented to people upgrading to 1.8, since I specifically selected EJS specifically to avoid using a different Javascript template framework that uses handlebars as delimiters, which overlaps with Go's template engines.

@rsc
Copy link
Contributor

rsc commented Jan 18, 2017

html/template must understand the meaning of the html tags in the input in order to understand which places need which escaping (for example <script> vs <div> or onclick= vs href=). My educated guess (without going searching) is that <% %> is escaped because html/template does not see it as a tag and does not want to emit something it doesn't understand that invalidates the security assertions it is making about the overall output.

@ghost
Copy link

ghost commented Jan 20, 2017

I wonder if text/template could introduce a new action, {{ verbatim }}, which would allow everything between it and the {{ end }} action to be included in the result verbatim. A warning of the security risk would have to go to the docs, of course.

@rsc
Copy link
Contributor

rsc commented Jan 23, 2017

I think for this release the fix is going to be to use template.HTML (or the type indirection) to insert <% ArbitraryJS %> into your output.

@rsc rsc modified the milestones: Go1.9Early, Go1.8Maybe Jan 23, 2017
@philpearl
Copy link
Contributor

philpearl commented Feb 26, 2017

Same problem for text/x-jsrender - which is the recommended type for jsrender templates - https://github.com/BorisMoore/jsrender#define-a-template. Context has switched from template.JS to template.HTML, which caused a failure for my app.

I'm switching to use template.HTML to fix this. Would really like a "just stick this text in the template output without escaping it" option that didn't care what the context was.

@AlexJakeGreen
Copy link

AlexJakeGreen commented Mar 14, 2017

Looks like possible workaround is to temporary use mimetype from the list

go/src/html/template/js.go

Lines 383 to 399 in 37dbc7b

"application/ecmascript",
"application/javascript",
"application/json",
"application/x-ecmascript",
"application/x-javascript",
"text/ecmascript",
"text/javascript",
"text/javascript1.0",
"text/javascript1.1",
"text/javascript1.2",
"text/javascript1.3",
"text/javascript1.4",
"text/javascript1.5",
"text/jscript",
"text/livescript",
"text/x-ecmascript",
"text/x-javascript":

Example:
<script type="application/json"

$ go version
go version go1.8 darwin/amd64

prymitive added a commit to cloudflare/unsee that referenced this issue Mar 24, 2017
Go 1.8 introduced checking of script type and templates are no longer loading, as the script type is not allowed by Go 1.8
golang/go#18569 captures the details.
Change template script type to one of the allowed types, it doesn't matter for clientside-haml-js, it only needs to match the script id
@bradfitz bradfitz modified the milestones: Go1.9, Go1.9Early May 3, 2017
@rsc
Copy link
Contributor

rsc commented Jun 26, 2017

Based on the discussion above and on the fact that there's a known workaround for non-JS, it seems like we should accept the new behavior, namely that only javascript is escaped as javascript.

@rsc rsc closed this as completed Jun 26, 2017
@srikanth32

This comment has been minimized.

@golang golang locked and limited conversation to collaborators Jun 19, 2019
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Security
Projects
None yet
Development

No branches or pull requests

9 participants