Contrib.social

Prevent XSS in data attributes

Open on Drupal.org →
Created on 16 May 2025, about 1 month ago

Problem/Motivation

It is a common mistake in contrib JS to use markup from a data attribute without sanitizing it.
Modules will store some description or caption in a data attribute then use it in JS without realizing an attacker could provide a data attribute containing malicious JS.

(There has been at least 5 vulnerabilities related to this fixed in contrib in the last 6 months.)

Other attributes could be vulnerable (most often alt and title) but I'm not sure we could cover every scenario, just make some exploits a bit hard.

Steps to reproduce

Typically a module will generate this kind of markup:

<a data-title="Foo" href="https://example.com">Click me</a>

Then on click will use it like this:

$('.description').html($(this).data('title'));

If an attacker is able to insert links with data attributes in the page, it can be used to inject malicious JS:

<a data-title="&lt;img src=x onerror=alert()&gt;" href="https://example.com">Click me</a>

This is often mitigated by the fact the default CKE config does not allow data attributes.
But Xss::filter() does and it is used by several contrib modules to filter user input.

This is of course a vulnerability in the JS itself, but we could make it harder to exploit.

Proposed resolution

Standard::filterXssDataAttributes() already does this but for some reason it is not on the parent Xss class.
So it could be as simple as moving this method to the parent class.

The only caveat I see is that it could remove some legitimate strings that look like HTML from data attributes.
Data attributes can contain arbitrary strings so it is hard to sanitize them without breaking some legitimate strings (see

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Active

Version

11.0 🔥

Component

render system

Created by

🇫🇷France prudloff Lille

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @prudloff
  • Merge request !12149Move filterXssDataAttributes() to parent class → (Open) created by prudloff
  • Pipeline finished with Failed
    about 1 month ago
    Total: 106s
    #499295
  • Comment about 1 month ago →
    🇫🇷France prudloff Lille
  • Pipeline finished with Failed
    about 1 month ago
    Total: 537s
    #499304
  • Comment about 1 month ago →
    🇫🇷France prudloff Lille

    Failing tests reveal there is a problem with this approach.
    Xss::filter() is sometimes called on partial HTML (for example #prefix and #suffix keys) and because filterXssDataAttributes() uses the DOM API, it tries to correct the HTML and closes unclosed tags.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 175s
    #499327
  • Pipeline finished with Failed
    about 1 month ago
    Total: 149s
    #499330
  • Comment about 1 month ago →
    🇫🇷France prudloff Lille

    Instead of parsing the HTML separately, I moved this to Xss::attributes() which already parses attributes.
    I reused the $skip_protocol_filtering condition in order to keep the change simple, I think we can consider that a non-URI attribute is an attribute that could contain HTML. This has the added benefit of sanitizing the alt and title attributes which are other common attack vectors.

    Some tests are still failing.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 712s
    #499334
  • Pipeline finished with Failed
    about 1 month ago
    Total: 179s
    #502166
  • Pipeline finished with Failed
    about 1 month ago
    Total: 145s
    #502203
  • Pipeline finished with Failed
    about 1 month ago
    Total: 584s
    #502236
  • Pipeline finished with Failed
    about 1 month ago
    Total: 199s
    #502261
  • Pipeline finished with Failed
    about 1 month ago
    Total: 531s
    #502266
  • Comment about 1 month ago →
    🇫🇷France prudloff Lille

    Xss::filter() encodes some lone characters like &, <, >. This means that we are going to double-encode these characters. For example data-title="Me &amp; my friends" would become data-title="Me &amp;amp; my friends".
    Note that this is already the case when using data attributes in CKE.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1114s
    #502300
  • Pipeline finished with Success
    28 days ago
    Total: 604s
    #510853
  • Comment 28 days ago →
    🇫🇷France prudloff Lille
  • Comment 17 days ago →
    🇫🇷France prudloff Lille

    This means that we are going to double-encode these characters. For example data-title="Me & my friends" would become data-title="Me &amp; my friends".

    Note that this problem already exists in other places in core. For example when using (string) new Attribute(['data-title' => 'Me &amp; my friends']);.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024