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

proposal: text/template: harden JSEscape #35665

Closed
empijei opened this issue Nov 18, 2019 · 1 comment
Closed

proposal: text/template: harden JSEscape #35665

empijei opened this issue Nov 18, 2019 · 1 comment

Comments

@empijei
Copy link
Contributor

empijei commented Nov 18, 2019

This was reported by a finder via security@golang.org

text/template.JSEscape escapes 5 characters in the "safe" printable ascii, plus all characters considered risky.

Current code to check if a rune needs escaping:

func jsIsSpecial(r rune) bool {
	switch r {
	case '\\', '\'', '"', '<', '>':
		return true
	}
	return r < ' ' || utf8.RuneSelf <= r
}

This leaves out = and &, which are also commonly escaped in this context.

This is not a vulnerability as & and = can only create problems when they appear in HTML context.

That said I would assume that it is not uncommon to have inline event handlers like these:

<button onclick=%s> click me! </button>

in which people forget to also apply HTML escaping after JS escaping when doing "manual" escaping instead of using html/template.

We should probably approach this in a more conservative way and err on the side of escaping a little bit more than less, hence adding two more characters to the escape set.

Note that this change might break some tests if they rely on stored responses.

CL: https://go-review.googlesource.com/c/go/+/207637

cc @FiloSottile @rsc @mvdan @t1ddl3r

@gopherbot gopherbot added this to the Proposal milestone Nov 18, 2019
@gopherbot
Copy link

Change https://golang.org/cl/207637 mentions this issue: text/template: harden JSEscape to also escape ampersand and equal

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants