Status messages rendered in managed file form elements filter HTML too aggressively

Created on 25 June 2024, 2 months ago
Updated 16 July 2024, about 2 months ago

Problem/Motivation

The ManagedFile form element plugin makes use of a #prefix to render out status messages in Drupal\file\Element\ManagedFile::uploadAjaxCallback. This means that any output from the status message template will be ran through Drupal's XSS filter -- which is known to cause problems.

Steps to reproduce

  1. Set up an off-the-shelf standard install.
  2. Add the webform module (for convenience; a native custom code FAPI form would work just as well...)
  3. Add a required managed file element to a form.
  4. Submit the form without uploading a file.
  5. Note that the status message has an <svg> sprite.
  6. Now things get interesting; upload a file that's too large for the field to accept...
  7. Note that the status message that's rendered by the managed file element does not have an <svg> sprite! The design has been broken!

Given this does break the Olivero design, I feel this is a legitimate bug that should be resolved.

Proposed resolution

By the time the status message template has finished rendering, this output should be trusted enough not to need this degree of XSS filtering . Can this be done safely?

Remaining tasks

Ask the security team to chime in.

User interface changes

TBD

API changes

TBD

Data model changes

TBD

Release notes snippet

TBD

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
File module 

Last updated 5 days ago

Created by

🇺🇸United States Luke.Leber Pennsylvania

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Luke.Leber
  • 🇺🇸United States cilefen

    This seems similar in nature to 🐛 Support displaying a message that correctly validates file uploads for the Gin theme. Active . Do they have a common cause?

  • 🇺🇸United States Luke.Leber Pennsylvania

    I believe that the root cause is that Drupal's XSS utility is somewhat dated. There are a pile of "safe" HTML5 tags that are stripped out -- a subset of SVG's elements among them.

    This is a somewhat heavy-handed approach that immediately comes to mind to resolve this in the mean-time:

        $status_messages = ['#type' => 'status_messages'];
        
        // Rather than concatenating things in `#prefix`, extract the
        // existing prefix and only run XSS filter on that.
        $prefix = $form['#prefix'] ?? '';
        if (!($prefix instanceof MarkupInterface)) {
          $prefix = Xss::filterAdmin($prefix);
        }
        
        // Then render a build array noting that...
        //  1. $prefix has been sanitized.
        //  2. $status_messages has been sanitized (via Twig)
        $build = [
          ['#markup' => Markup::create($prefix)],
          $status_messages,
          array_filter($form, static fn($key) => $key !== '#prefix', ARRAY_FILTER_USE_KEY),
        ];
        $output = $renderer->renderRoot($build);
    
  • 🇮🇳India naveenvalecha New Delhi

    This looks related

  • 🇳🇿New Zealand quietone New Zealand
Production build 0.71.5 2024