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: If class contains foo" 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&amp;quot; onclick=&amp;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 |raw or Markup::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

  1. Install views_bootstrap 5.5.x and enable audit + audit_twig (audit version: [VERSION]).
  2. Run the Twig audit (Admin > Reports > Site Audit, or drush audit:run).
  3. Observe the finding for views-bootstrap-table.html.twig.
  4. Optionally, render a Twig string in Drupal with <div class="base {{ class }}"> and class = 'foo" onclick="alert(1)', and observe that the output is escaped.

Proposed resolution

  1. 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_class so classes are alterable and valid."
    • Remove the incorrect attack example and the statement that auto-escaping does not protect against attribute injection.
  2. Keep or add security findings only for patterns that are actually risky:
    • unquoted attribute values, e.g. class={{ foo }}
    • |raw or {% autoescape false %}
    • variables in on* event handler or style attributes
    • optionally, a lower-severity review hint for full values of URL attributes (href, src, action, formaction), since HTML escaping does not prevent javascript: URLs
  3. 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

anybody created an issue. See original summary.

anybody’s picture

Issue summary: View changes
anybody’s picture

Issue summary: View changes
anybody’s picture

Issue summary: View changes
anybody’s picture

Issue summary: View changes
anybody’s picture

Issue summary: View changes

Sorry I was struggling with the markup :D

anybody’s picture

Please check if my assumption is correct and this is indeed a false positive reported.

anybody’s picture

Issue summary: View changes