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 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 gopherbot added the Proposal label Nov 18, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Nov 18, 2019

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

@gopherbot gopherbot closed this in 94e9a5e Nov 21, 2019
@golang golang locked and limited conversation to collaborators Nov 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.