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: incorrectly escapes urls with fragments #36096

Open
takeda opened this issue Dec 12, 2019 · 2 comments
Open

html/template: incorrectly escapes urls with fragments #36096

takeda opened this issue Dec 12, 2019 · 2 comments

Comments

@takeda
Copy link

@takeda takeda commented Dec 12, 2019

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

1.13.5

Does this issue reproduce with the latest release?

yes (can reproduce it on go playground)

What did you do?

The code contains URL used by Kibana (eleastic search plugin which uses RISON for URLs)

https://play.golang.org/p/7-WnsX9FgKk

package main

import (
	"fmt"
	"html/template"
	"net/url"
	"os"
)

func main() {
	testUrl := "https://host.com/_plugin/kibana/app/kibana#/discover?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-15m,mode:quick,to:now))&_a=(columns:!(_source),index:'745526c0-a366-11e9-ada4-f767130ae0b4',interval:auto,query:(language:lucene,query:'app:\"app\" AND proc:\"web\" AND environment:\"staging\"'),sort:!('@timestamp',desc))"
	parsedUrl, err := url.Parse(testUrl)
	if err != nil {
		panic(err)
	}
	tmpl, err := template.New("name").Parse(`<a href="{{.}}">link</a>`)
	if err != nil {
		panic(err)
	}
	fmt.Println(testUrl)
	fmt.Println(parsedUrl)
	tmpl.Execute(os.Stdout, parsedUrl)
}

What did you expect to see?

I expected to see the original URL since the fragments should be escaped so:
https://host.com/_plugin/kibana/app/kibana#/discover?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-15m,mode:quick,to:now))&_a=(columns:!(_source),index:'745526c0-a366-11e9-ada4-f767130ae0b4',interval:auto,query:(language:lucene,query:'app:"app" AND proc:"web" AND environment:"staging"'),sort:!('@timestamp',desc))

URL as escaped by net.url package is also acceptable (Kibana seems to be able to handle it):

https://host.com/_plugin/kibana/app/kibana#/discover?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-15m,mode:quick,to:now))&_a=(columns:!(_source),index:%27745526c0-a366-11e9-ada4-f767130ae0b4%27,interval:auto,query:(language:lucene,query:%27app:%22app%22%20AND%20proc:%22web%22%20AND%20environment:%22staging%22%27),sort:!(%27@timestamp%27,desc))

What did you see instead?

The URL that is invalid and has wrong parts escaped:

<a href="https://host.com/_plugin/kibana/app/kibana#/discover?_g=%28refreshInterval:%28pause:!t,value:0%29,time:%28from:now-15m,mode:quick,to:now%29%29&amp;_a=%28columns:!%28_source%29,index:%27745526c0-a366-11e9-ada4-f767130ae0b4%27,interval:auto,query:%28language:lucene,query:%27app:%22app%22%20AND%20proc:%22web%22%20AND%20environment:%22staging%22%27%29,sort:!%28%27@timestamp%27,desc%29%29">link</a>

If this is indeed a bug any workaround I can apply right now to make the template do the right thing?

@ALTree ALTree changed the title html/template incorrectly escapes urls with fragments html/template: incorrectly escapes urls with fragments Dec 12, 2019
@empijei
Copy link
Contributor

@empijei empijei commented Apr 9, 2020

As a workaround you could disable contextual autoescaping by casting it to the template.URL type. This will expose you to XSS so only do this if you trust the source of that URL and you are sure no one except you can control its content. You could for example escape it with the net/url package and promote it to template.URL

That said I will need to investigate more on this one to determine if it is a bug.

@empijei
Copy link
Contributor

@empijei empijei commented Apr 9, 2020

According to RFC3986 section 3.5 a fragment is defined as

      fragment    = *( pchar / "/" / "?" )

And pchar is defined as

      pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"

Note the pct-encoded.

This means that any receiver of a fragment should percent-unescape it to read its content.

This is, for example, implemented in the net/url package. I took your three URLs and tried to parse them (there is a round of html unescape because you were interpolating in an HTML context) and as you can see the three URLs are equivalent once parsed.

I invite you to open a bug on Kibana and apply my suggested patch above in the meantime.

/cc @kele

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants