Figure out what to do about attribute filtering in Twig

Adding some related issues.

This is not a new problem; all Twig authors have to deal with this, either by ... or dealing with it in the prepping code. It is, however, new to Drupal 8.

Note that it is also not entirely new to Drupal 8 relative to Drupal 7. In Drupal 7, preprocess functions are expected to make all .tpl.php variables safe. But, what's new to Drupal 8 is that preprocess functions no longer need to make variables used within HTML text or fragments safe, as that gets autoescaped, so there is a bit of an inconsistency currently that preprocess functions in HEAD explicitly sanitize URLs, but not anything else, and a corollary inconsistency in which variables Twig templates print via the attributes object and which ones it prints directly into attribute values, such as <a href="{{ item.url }}">.

Restored <code> tags in the IS.

Just a note this isn't just about URLs, it's about any HTML attribute in a template - given that Twig won't auto-escape variables marked as safe that were filtered as opposed to escaped.

Right. So for example, suppose you were to change bartik's node.html.twig to print the node title into an attribute value, like this:

<a href="{{ url }}" rel="bookmark" title="{{ label }}">{{ label }}</a>

Because {{ label }} contains a <span> element (for QuickEdit), you would get this:

Even if this example isn't an XSS vulnerability, it still leads to invalid HTML and renders to the browser in a way that isn't desired.

Even if we tried hard to avoid using '|escape' in templates, I think this is one where it is beneficial for themers to learn it.

So we would teach to use:

<a href="{{ url | e('html_attr') }}" rel="bookmark" title="{{ label | e('html_attr') }}">{{ label }}</a>

with docs that the Attribute object takes care of that automatically.

In case url is a SafeStringAttribute() or a SafeString with strategy = 'html_attr' then twig would not do double-escaping.

Overall that kind of support would be pretty simple to add to drupalEscapeFilter() in TwigExtension.

One way to also do this is to use a link, though I would myself have to lookup how to add the label in (will edit later)

{{ link('text', url) }}

Of course writing direct HTML is nicer, but maybe we should allow access to create attribute objects on the fly in twig. Then you could choose:

a)

{% set attributes = safe_attributes({'label': label, 'href': url}); %}
<a{{- attributes }}>{{ label }}</a>

b)

<a href="{{ url | e('html_attr') }}" rel="bookmark" title="{{ label | e('html_attr') }}">{{ label }}</a>

In one case the Attribute object would take over the escaping, in the other you are yourself responsible.

In any case extending SafeString to either take a strategy parameter or have a class in descending order of safeness for each escaping strategy sounds a good idea to me and makes both of this possible.

Actually a better example should be:

<a href="{{ url | e('url') }}" rel="bookmark" title="{{ label | e('html_attr') }}" style="{{ some_css|e('css') }}">{{ label }}</a>

and shows that maybe having magic attributes as a 90% API is better - if I get that wrong on first try ;).

Overall the following escaping strategies are supported in Twig:

• - html
• - js
• - css
• - url
• - html_attr

(or a PHP callback that takes the template "filename" and must return the escaping strategy to use)

I would say that Urls itself should have the SafeUrlStringInterface implemented or something like that ...

Thanks @Fabianx. I agree, I do think we need to let Twig's security rule in the twig file.

It could prove tricky to change how those escape strategies work without going upstream pull requests, or a request to make them easier to change. So crossing my fingers a bit on them being acceptable as-is.

#9 this is a good example to work from and add to our documentation:
<a href="{{ url|e('url') }}" rel="bookmark" title="{{ label|e('html_attr') }}" style="{{ some_css|e('css') }}">{{ label }}</a>

And agree with creating a create_attribute({'id': var}) would be a great addition because right now we can only create new Attribute objects through PHP.

Based on the Twig docs the 'url' strategy is not workable for full URLs. html_attr is probably fine for non-URL attributes.

Unless this part changes I think we should instead add our own strategies.

Built-in escapers cannot be overridden mainly they should be considered as the final implementation and also for better performance.

create_attribute() is interesting, probably not a bad idea.

Since it's been 7 years without movement

1. Wonder if this is still a valid task
2. If so what needs to be done for D10

Worth noting #2544110: XSS attribute filtering is inconsistent and strips valid attributes is being worked on and not sure how it plays here.