golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.58k stars 17.61k forks source link

html/template: dynamic substrings in HTML tags or attributes can result in unsafe HTML output #19669

Open stjj89 opened 7 years ago

stjj89 commented 7 years ago

The following template:

<s{{.X}}>alert('pwned')</script>

produces the following HTML output when executed with X = "cript":

<script>alert('pwned')</script>

This happens because:

In general, allowing dynamic substrings in HTML tags or attributes may confuse the parser and escaper, since the static and dynamic parts of the name are handled in different phases.

Suggested solution: disallow dynamic substrings in HTML tags or attributes completely.

stjj89 commented 7 years ago

@mikesamuel

mikesamuel commented 7 years ago

The idea initially was to allow switching between a few equivalence sets

so it might be worth not including the feature in strict mode or limiting it somehow.

I think

<s{{.}}

is high impact but low frequency if we're correct about our design assumption that template authors are acting in good-faith, so this is definitely bug, but not super high priority.

stjj89 commented 7 years ago

so it might be worth not including the feature in strict mode or limiting it somehow.

I definitely plan to disallow this feature in my wrapper package.

is high impact but low frequency if we're correct about our design assumption that template authors are acting in good-faith, so this is definitely bug, but not super high priority.

I agree that this bug is unlikely to occur if we assuming benign developers. Do you have a sense of how we should go about fixing this? The only two solutions I see are either disallowing this feature completely, or adding another escaping pass.

ianlancetaylor commented 6 years ago

ping @stjj89 @mikesamuel What is the status of this? Thanks.

stjj89 commented 6 years ago

As per Mike's comment, the probability of this being an issue might be low enough that it is not worth fixing preemptively. We can probably shelve this and revisit it in the future if it proves to be an issue.

ianlancetaylor commented 6 years ago

Thanks.