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_attr: do not escape colons #3614

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

gharlan
Copy link

@gharlan gharlan commented Dec 30, 2021

Is it necessary to escape the : in html_attr context?

For example in this code:

{% for key, value in attributes %}
    {{ key|escape('html_attr') }}="{{ value }}"
{% endfor %}

I think it should be allowed to use attribute keys like v-on:submit.prevent.

@stof
Copy link
Member

stof commented Apr 7, 2022

The goal is to escape things like javascript:foo are() allowing to inject JS execution in href

@gharlan
Copy link
Author

gharlan commented Apr 7, 2022

Good point!
In #2817 (comment) I've learned that it is ok to use html escaping in quoted attribute values:

example 1 is quite fine here, because your attribute value is quoted, and the html escaping strategy already escapes quotes.

The wrong cases are these one:

  • using a dynamic attribute name
  • using an unquoted attribute value

So for some attributes (like href), html_attr escaping should be used for the value, even if it is quoted.

@stof
Copy link
Member

stof commented Apr 8, 2022

The href attribute is indeed a bit special due to links allowing to execute JS

@gharlan
Copy link
Author

gharlan commented Mar 15, 2023

Even with html_attr the attribute name should never come from user input, because of onclick attributes etc.

The goal is to escape things like javascript:foo are() allowing to inject JS execution in href

{% set attr = "javascript:alert(1)" %}
<a href="{{ attr|e('html_attr') }}">Click</a>

will result in:

<a href="javascript&#x3A;alert&#x28;1&#x29;">Click</a>

(https://twigfiddle.com/n1rbba)

This is still executable javascript: https://jsfiddle.net/9ekxLy6u/


So I'm still not sure whether the : should be escaped in html_attr context.

Could also be relevant for #3760.
Which escaping strategies should the new html_attributes function/filter use for keys and values? Should it support attribute names like v-on:submit.prevent?

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

Successfully merging this pull request may close these issues.

2 participants