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

Array arguments aren't escaped #10

Open
aqueenan opened this issue Mar 30, 2020 · 2 comments
Open

Array arguments aren't escaped #10

aqueenan opened this issue Mar 30, 2020 · 2 comments

Comments

@aqueenan
Copy link

If you pass in an array argument, the elements of the array are joined, but the result isn't escaped, so there is an opportunity for script injection.

@AntonioVdlC
Copy link
Owner

Hi @aqueenan,

Yes, that's by design as it allows for nested interpolations. Ideally, you would pass every element of the array through the html template tag (or any other function that performs escaping at some level).

// ❌ Danger
var names = ["Some", "Name", "/><script>alert('xss')</script>"];
var string = html`
	<ul>
		${names}
	</ul>
`;
// "<ul>SomeName/><script>alert('xss')</script></ul>"

// ❌ Danger
var names = ["Some", "Name", "/><script>alert('xss')</script>"];
var string = html`
	<ul>
		${names.map((name) => `
			<li>Hello, ${name}!</li>
		`)}
	</ul>
`;
// "<ul><li>Hello, Some!</li><li>Hello, Name!</li><li>Hello, /><script>alert('xss')</script>!</li></ul>"

// ✅ Safe
var names = ["Some", "Name", "/><script>alert('xss')</script>"];
var string = html`
	<ul>
		${names.map((name) => html`
			<li>Hello, ${name}!</li>
		`)}
	</ul>
`;
// "<ul><li>Hello, Some!</li><li>Hello, Name!</li><li>Hello, /&gt;&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;!</li></ul>"

If we were to escape array elements, then you wouldn't be able to have nested interpolations.

// ❌ This wouldn't work if we escaped array elements
var names = ["Some", "Name", "/><script>alert('xss')</script>"];
var string = html`
	<ul>
		${names.map((name) => `
			<li>Hello, ${name}!</li>
		`)}
	</ul>
`;
// "<ul>&lt;li&gt;Hello, Some!&lt;/li&gt;&lt;li&gt;Hello, Name!&lt;/li&gt;&lt;li&gt;Hello, /&gt;&lt;script&gt;alert(&#39;xss&#39;)&lt;/script&gt;!&lt;/li&gt;</ul>"

Ultimately, what the html template tag does is allow handwritten HTML to go through while escaping all the interpolations within that string literal.

@leafac
Copy link

leafac commented Jan 28, 2021

Proposal: ${[...]} should escape the contents of the array, and $${[...]} should not escape.

Advantages:

  1. It’s safe by default; you have to opt into the behavior of not escaping by introducing the extra $.
  2. It’s more consistent with the non-array case.

Disadvantage: It’s a breaking change.

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

No branches or pull requests

3 participants