html/template: clarify, perhaps expand allowed URLs in urlFilter

[erikformella]

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

1.8

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

darwin
amd64

What did you do?

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

What did you expect to see?

<a href="tel:555-555-5555">click me</a>

What did you see instead?

<a href="#ZgotmplZ">click me</a>

The source here only whitelists http, https, and mailto: https://github.com/golang/go/blob/master/src/html/template/url.go#L22

I also found this thread talking about it and asked if I should work on a fix. No response yet.

[rsc]

For reference: https://tools.ietf.org/html/rfc3966.

[ianlancetaylor]

CC @stjj89

[ianlancetaylor]

CC @mikesamuel

[rsc]

Unless @stjj89 or @mikesamuel object, it seems fine to treat tel: like mailto:.

[stjj89]

Would wrapping the tel URL in template.URL be sufficient? i.e. https://play.golang.org/p/fVJv1ORih2

I would prefer not to whitelist tel, just because those URLs can trigger unexpected actions on behalf of the user of the generated HTML (e.g. initiate a paid phone call). Requiring the developer to explicitly cast such URLs as template.URL gives us a soft guarantee that the developer has considered the implications of allowing this scheme.

[rsc]

I completely understand the escaping in

<a href="https://my.site/search?q={{.}}">

to make that safe regardless of how {{.}} is filled in. In this example, html/template is working with the user to help ensure that the results match what the user intends, namely setting the q parameter.

In contrast I do not understand the sanitizing in

<a href="{{.}}">

Why does "https://evil.site/attack.html" pass through unmodified while "ftp://ftp5.freebsd.org/" and "tel:555-1212" do not? Here it seems like html/template moves from working with the user to second-guessing, passing judgement, or even babysitting.

[rchunping]

the same problem

<img src=“{.}” />

data:image/png;base64,........

[mikesamuel]

Why does "https://evil.site/attack.html" pass through unmodified while "ftp://ftp5.freebsd.org/"
and "tel:555-1212" do not? Here it seems like html/template moves from working with the user
to second-guessing, passing judgement, or even babysitting.

The relevant code is https://golang.org/src/html/template/url.go#L22

https://golang.org/pkg/html/template/#hdr-Security_Model explains the reasoning obliquely

This package assumes that template authors are trusted, that Execute's data parameter
is not, and seeks to preserve the properties below in the face of untrusted data:

Code Effect Property: "... only code specified by the template author should run as a result
of injecting the template output into a page and all code specified by the template author
should run as a result of the same."

So we reject javascript:... URLs in data parameters because it is not code specified by the template authors.

Since writing that, we on the security engineering team have come to the opinion that
the Code Effect property should really be an Unexpected Side Effect property.
Samuel was pointing out that there are multiple known exploits related to tel: URLs.

These exploits are widely understood by security researchers, but not by HTML developers, so qualify as an Unexpected Side Effect.
I think device manufacturers are working to fix these problems in webviews, but I don't know how many affected devices are still out there. Once that number falls low enough though, the unexpected side effect argument would no longer apply.

[stjj89]

In similar internal URL filters, we adopt a more permissive policy that allows http, https, mailto, ftp schemes, as well as some subset of base64 data:image/ and data:video/ URLs. We could consider making the html/template URL filter similarly permissive.

[xtofian]

In general, there's a long history of (often fairly obscure) security-relevant inconsistencies and bugs in specs and implementations of the Web Platform [Zalewski, 2012]. Based on this, we should be inclined to be fairly conservative in the design of frameworks and libraries that interact with the Web Platform.

This is for instance the reason why data: URLs have traditionally not been allowed by URL sanitizers: We know that some data: URLs can result in same-origin script exeuction and hence XSS (data:text/html;...). The syntax and semantics of data: URLs are fairly complex, raising the concern that an apparently safe subset (say, data:image/png;...) might actually include problematic instances (perhaps data:image/png;base64,<invalid base64 encoding> causes some browsers to misbehave). In this particular case, over the past few years, we have gained reasonable confidence in the correctness of current browsers' handling of, say, data:image/* URLs, and have hence started accepting them in various sanitizer implementations (e.g. Closure Templates). It'd make sense to accept this in html/template as well.

As for tel: and sms: URLs: Depending on the situation, the context of the app containing the link may confer some degree of trust, and a user might be tricked into actually dialing a 1900 number that costs them money. True, outbound http://evil.org links can be similarly problematic (and perhaps, in some phishing-related scenarios even worse). This essentially comes down to a tradeoff between being conservative and not being unreasonably restrictive: We can't realistically reject all http(s)?: URLs since they're a fundamental part of the web and essentially everywhere. tel: on the other hand seems to be relatively uncommonly used, and conceivably problematic in some contexts. This suggests that it's reasonable to to not allow tel: by default, and instead mark the URL explicitly safe via template.URL (this also alerts security reviewers to the special consideration) .

Specific concerns around tel: URLs are (as far as I recall) to a large extent due their handling in iOS Safari:

• https://archive.cert.uni-stuttgart.de/bugtraq/2009/06/msg00190.html (long fixed, but illustrates the point that tel: URLs were not always harmless).

• https://software-security.sans.org/blog/2010/11/08/insecure-handling-url-schemes-apples-ios/ (illustrates why it's generally good practice to not accept custom schemes unless known safe).

• https://nakedsecurity.sophos.com/2014/09/03/apple-developer-guidelines-lead-to-rogue-phone-call-risks-in-ios/
  I'm not sure if the latter is still the case; it would be consistent with current documentation (https://developer.apple.com/library/content/featuredarticles/iPhoneURLScheme_Reference/PhoneLinks/PhoneLinks.html) stating that tel: URLs in web documents result in a prompt, but URLs in native apps don't (and WebViews would reasonably be considered part of a native app).
  If this is still the case, it would indicate that tel: URLs (originating from untrusted sources) are indeed not safe in HTML markup that is rendered within an iOS WebView (because in that context, the app and its WebView are considered responsible for generating user prompts).

[Zalewski, 2012] Zalewski, Michal. The tangled Web: A guide to securing modern web applications. No Starch Press, 2012.

[rchunping]

@xtofian add something like .DisableSecurityCheck(true) ?

[xtofian]

@rchunping - the canonical way to suppress the default sanitization/validation for a particular value is to wrap it in an appropriate type that indicates to html/template that the value should be considered safe for a given context; in this case, template.URL. See the example that @stjj89 gave above, https://play.golang.org/p/fVJv1ORih2

[rchunping]

@xtofian thanks ,but the link was broken.

<h1>Unavailable For Legal Reasons</h1><p>If you believe this is in error, please <a href="https://golang.org/issue">file an issue</a>.</p>

[rsc]

Thanks @mikesamuel, this is a good explanation.

@stjj89, can you send a CL expanding the internal comment on func urlFilter to explain a bit more about this, and maybe also think about whether the set there needs to be expanded? We don't have to do that until next cycle, but if you want to send the CL now to get it off your plate, that's fine. Thanks.

[gopherbot]

Change https://golang.org/cl/52853 mentions this issue: html/template: explain URL filtering