safe HTML templates

497 views
Skip to first unread message

Mike Samuel

unread,
Sep 15, 2011, 11:42:11 PM9/15/11
to golang-nuts
package exp/template/html, which makes HTML templates robust against
XSS, is feature complete.

Given code like

t, _ := template.New("t").Parse(
`Hello, <a onclick="alert({{.}})">{.}</a>`)
t, _ = html.Escape(t)
t.Execute(os.Stdout, `<World>`)

you will end up with something like

Hello, <a onclick="alert(&quot;\x3cWorld
\x3e&quot;)">&lt;World&gt;</a>

I have yet to finish testing or organize an attack review, so please
don't rely on it to protect valuables; but it's ready for criticism
and for early adopters to have a go.

Most of the changes are not in the most recent stable release, so
you'll have to compile from head, but you can browse the package
documentation at http://code.google.com/p/go/source/browse/src/pkg/exp/template/html/doc.go

OmarShariffDontLikeIt

unread,
Sep 16, 2011, 1:33:03 AM9/16/11
to golang-nuts
> documentation athttp://code.google.com/p/go/source/browse/src/pkg/exp/template/html/d...


This. Looks. AWESOME! I really like the conversion based on context!
Converting to Javascript code is like magic! I take it this is
destined for the new template package?

Andrew Gerrand

unread,
Sep 16, 2011, 2:09:45 AM9/16/11
to golan...@googlegroups.com


On Friday, September 16, 2011 3:33:03 PM UTC+10, OmarShariffDontLikeIt wrote:
Converting to Javascript code is like magic! I take it this is
destined for the new template package?

It is destined to be the "template/html" package.

Andrew 

Mike Samuel

unread,
Sep 16, 2011, 2:19:34 AM9/16/11
to golang-nuts
Yeah. Barring major difficulties, it'll move out of exp. Package
template is more general than just HTML, CSS, and JS though so it'll
remain separate.

Nigel Tao

unread,
Sep 16, 2011, 2:39:13 AM9/16/11
to golan...@googlegroups.com
On 16 September 2011 16:09, Andrew Gerrand <a...@golang.org> wrote:
> It is destined to be the "template/html" package.

<bikeshed>I'm hoping for "template/htmltemplate", the way ioutil and
jsonrpc are "io/ioutil" and "rpc/jsonrpc" and not "io/util" and
"rpc/json".</bikeshed>

David Symonds

unread,
Sep 16, 2011, 2:42:39 AM9/16/11
to Nigel Tao, golan...@googlegroups.com

That would probably be a good idea, along with its package clause
saying "htmltemplate" so you don't have to get annoyed with a
collision with the "html" package.


Dave.

Stefan Scholl

unread,
Sep 16, 2011, 3:05:53 AM9/16/11
to golan...@googlegroups.com

And there's already a package named html <http://golang.org/pkg/html/>.

I would prefer the standard packages to be usable without giving
them another package name on import.


Why not include this one function into the template package
itself? There are already functions like HTMLEscape() and
HTMLEscapeString(). This one could be called HTMLEscapeTemplate()

Andrew Gerrand

unread,
Sep 18, 2011, 12:31:40 AM9/18/11
to golan...@googlegroups.com
Sounds good to me. 

Mike Samuel

unread,
Sep 28, 2011, 6:00:22 PM9/28/11
to golang-nuts


On Sep 15, 8:42 pm, Mike Samuel <mikesam...@gmail.com> wrote:
> package exp/template/html, which makesHTMLtemplates robust against
> XSS, is feature complete.

I think exp/template/html is now usable.
I am still working to get some other security-weenies to bang on it so
I cannot call it solid.

I wanted to get some experience with it on a real app, so I reworked
godoc ( http://codereview.appspot.com/5133048/ ) to use it instead of
explicit escaping directives.

The first thing I did was to turn on auto-escaping when godoc is run
with -html:

t, err := template.New(name).Funcs(fmap).Parse(string(data))
+ if *html && err == nil {
+ t, err = htmltemplate.Escape(t)
+ }

then I removed all the escaping directives. For example, even after

{{with .Title}}
- <h1 id="generatedHeader">{{html .}}</h1>
+ <h1 id="generatedHeader">{{.}}</h1>
{{end}}

the page /<script>alert(1337)</script>.html still shows a properly
escaped error message:

<h1 id="generatedHeader">File &lt;script&gt;alert(1337)&lt;/
script&gt;.html</h1>

so htmltemplate prevents at least one reflected XSS.

Then I wanted to make sure there was no over-escaping. I set up two
godoc, one unpatched and one patched, so I could curl a bunch of files
and diff the output the two produce. (See the codereview.appspot URL
for the exact command line.)

Godoc has one template, lib/godoc/godoc.html, that provides a common
header, footer, and sidebar. It interpolates Content from another
template execution.

<!-- Content is HTML-escaped elsewhere -->
- {{printf "%s" .Content}}
+ {{.Content}}

so I made sure to mark the output of templates used to generate HTML
as known-safe HTML.

+func applyHTMLTemplate(t *template.Template, name string, data
interface{}) htmltemplate.HTML {
+ return htmltemplate.HTML(applyTemplate(t, name, data))
+}

...

- contents := applyTemplate(packageHTML, "packageHTML", info)
+ contents := applyHTMLTemplate(packageHTML, "packageHTML", info)
servePage(w, title, subtitle, "", contents)

The type of content changed too

- Content []byte
+ Content htmltemplate.HTML

though if some callers pass text content and some pass known-safe
HTML, it could just as easily be

- Content []byte
+ Content interface{}

I then reworked some template functions that produce known safe HTML.
{{comment_html .Doc}} is used in the package docs and changed:

-func comment_htmlFunc(comment string) string {
+func comment_htmlFunc(comment string) htmltemplate.HTML {
var buf bytes.Buffer
// TODO(gri) Provide list of words (e.g. function parameters)
// to be emphasized by ToHTML.
doc.ToHTML(&buf, []byte(comment), nil) // does html-escaping
- return buf.String()
+ return htmltemplate.HTML(buf.String())
}

That left only minor differences. Escape elides comments to prevent
internal project details from leaking. For example, the patched godoc
produces this difference:

- <!-- The Table of Contents is automatically inserted in this <div>.
- Do not delete this <div>. -->
+
<div id="nav"></div>

which is not normally a problem, except that the common header uses
special comments to embed IE only styles.

<!--[if lt IE 8]>
<link rel="stylesheet" href="/doc/ie.css" type="text/css">
<![endif]-->

CL http://codereview.appspot.com/4999042/ explains how
htmltemplate.HTML could also solve this problem, but I decided to fix
it using an auditable exemption:

-<!--[if lt IE 8]>
+{{noescape "<!--[if lt IE 8]>"}}
<link rel="stylesheet" href="/doc/ie.css" type="text/css">
-<![endif]-->
+{{noescape "<![endif]-->"}}

With autoescape on, you're secure by default, but noescape allows
exemptions to this policy that can easily be found and audited via
grep.
Reply all
Reply to author
Forward
0 new messages