Problem/Motivation
audit_twig reports variables printed inside a quoted class attribute as a security vulnerability:
Dynamic class in HTML attribute string - potential XSS vulnerability (tags: security, cache, best_practice)
Security Issue: Building attributes as strings like
<div class="base {{ class }}">is a security vulnerability. [...] because Twig's auto-escaping does not protect against attribute injection.
Example of attack: Ifclasscontainsfoo" onclick="alert(1), the resulting HTML would be:<div class="base foo" onclick="alert(1)">
Example finding in views_bootstrap 5.5.x, templates/views-bootstrap-table.html.twig:
https://git.drupalcode.org/project/views_bootstrap/-/blob/5.5.x/template...
<div class="table-responsive{{ responsive_size }}">The security claim and the attack example are incorrect for Drupal. Drupal runs Twig with autoescaping enabled. Every {{ variable }} that is not a MarkupInterface object goes through Drupal's escape filter (Html::escape(), i.e. htmlspecialchars() with ENT_QUOTES). The value foo" onclick="alert(1) is actually rendered as:
<div class="base foo&quot; onclick=&quot;alert(1)">The value cannot leave a quoted attribute, so no attribute injection is possible. Attribute injection only applies to:
- unquoted attribute values (
class={{ foo }}) - output marked safe via
|raworMarkup::create() {% autoescape false %}blocks
Drupal core uses the same pattern in many templates. For example, core's views-view-table.html.twig prints <a href="{{ column.url }}" title="{{ column.title }}">. Reporting this as a vulnerability produces false positives across core and contrib. It may also lead users to file invalid security reports against contributed projects.
The recommendation itself has merit, but for other reasons. Using the Attribute object with |clean_class lets other modules and themes alter the classes and guarantees valid class names. That is a best-practice or maintainability concern, not a security one. It is also unclear why the finding is tagged "cache".
Steps to reproduce
- Install views_bootstrap 5.5.x and enable audit + audit_twig (audit version: [VERSION]).
- Run the Twig audit (Admin > Reports > Site Audit, or
drush audit:run). - Observe the finding for
views-bootstrap-table.html.twig. - Optionally, render a Twig string in Drupal with
<div class="base {{ class }}">andclass = 'foo" onclick="alert(1)', and observe that the output is escaped.
Proposed resolution
- For variables inside quoted attribute values:
- Remove the "security" tag and the XSS wording.
- Keep it as a best_practice hint, e.g. "Prefer the Attribute object and
|clean_classso classes are alterable and valid." - Remove the incorrect attack example and the statement that auto-escaping does not protect against attribute injection.
- Keep or add security findings only for patterns that are actually risky:
- unquoted attribute values, e.g.
class={{ foo }} |rawor{% autoescape false %}- variables in
on*event handler orstyleattributes - optionally, a lower-severity review hint for full values of URL attributes (
href,src,action,formaction), since HTML escaping does not preventjavascript:URLs
- unquoted attribute values, e.g.
- Remove the "cache" tag unless there is a concrete caching implication.
Remaining tasks
- Adjust the rule and its message.
- Add test cases for quoted vs. unquoted attributes.
User interface changes
Changed severity, tags and text of the finding.
API changes
None.
Data model changes
None.
AI disclosure
This issue summary was drafted with the help of an AI assistant (Claude). I reviewed the content and checked it against the audit_twig finding and the views_bootstrap template before posting.
Comments
Comment #2
anybodyComment #3
anybodyComment #4
anybodyComment #5
anybodyComment #6
anybodySorry I was struggling with the markup :D
Comment #7
anybodyPlease check if my assumption is correct and this is indeed a false positive reported.
Comment #8
anybody