Hi module's maintainers,

in the module there is a part where the output is not sanitized, allows cross site scripting to be executed.
As the module seems actively used by over 50k sites but has no coverage by Drupal Security Team, i open this issue here as a public one in order to let users of it know about the issue and act on their own safe before the new version of it gets released.

Here are the steps how it can be reproduced:
- Enable the module.
- Create a new format filter and set its name to this:

alert('XSS')

- Then add a long text (or use an already existing one) to a content type. When this page is loaded: admin/structure/types/manage/article/fields/body (where article is the bundle, body is the used field) you get the result of XSS exploit.

I'm attaching the patch that fixes this issue.

As for the exploit the following permissions are needed administer fields, administer filters if your site doesn't allow these to untrusted users the vulnerability's risk is much lower.

Thank you in advance,
Balazs.

CommentFileSizeAuthor
#6 better_formats-2896131-6-missing-check_plain-when-showing-filter-name.patch705 bytesAlan D.
xss-issue-in-the-codebase-1.patch710 bytestatarbj
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tatarbj created an issue. See original summary.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs review » Needs work
Issue tags: -Security

This is not a notifiable security issue as the permission "administer filters" is required. You should never ever add administer fields, administer filters to untrusted users on any site!!!

https://www.drupal.org/drupal-security-team/security-advisory-process-an...

i.e. if you have this permission, you can simple remove all text formatting on a filter format (aka free html) and just add/edit ANY field in content (i.e. new post body field) that uses that format to implement a XSS attack.

Or if you had PHP filter enabled, you could simply enable that on any format, allowing anyone to to create posts with PHP code, etc

So yes, this should be fixed, but security issue no :)

You should use check_plain() here to show the exact characters typed. i.e. convert > to $amp;gt; so that it is outputted as >

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at MachbarMacher commented
Priority: Normal » Major

Yah, this does not need an official security advisorsy, but nevertheless constitues a major repercussion so setting major.
I agree, this patch should be rerolled using check_plain.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at MachbarMacher commented
Parent issue: » #2030199: Better Formats stable release (7.x-1.0)
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented

Nar, totally not major, I actually say minor. ;)

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Priority: Major » Normal
Status: Needs work » Needs review
FileSize
better_formats-2896131-6-missing-check_plain-when-showing-filter-name.patch705 bytes

And a re-roll :)

Just to enforce the point about check_plain(), it's how Drupal handles it everywhere, doing anything else would be inconsistent with core

  $output .= '<h3>' . check_plain($format->name) . '</h3>';
  t('Added text format %format.', array('%format' => $format->name))
  // etc etc...
tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
CreditAttribution: tatarbj commented
Status: Needs review » Reviewed & tested by the community

I've tested and it works as expected :) Thanks guys for the work!
Bests,
Balazs.

Delphine Lepers’s picture

Delphine Lepers CreditAttribution: Delphine Lepers at Trasys for European Commission and European Union Institutions, Agencies and Bodies commented

Hi all,

Any chance to get a new release with this included?
Cheers

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
CreditAttribution: tatarbj commented

I think it depends on the module maintainers - until that time you can use the patch attached here.

tatarbj’s picture

tatarbj
Primary language Hungarian
Location Brussels
CreditAttribution: tatarbj at Petend for European Commission and European Union Institutions, Agencies and Bodies commented
joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Related issues: +#3336311: Offering to maintain Better Formats

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Status: Reviewed & tested by the community » Fixed

  • joseph.olstad committed f3f186c4 on 7.x-1.x 
    Revert "Issue #2896131 by Alan D.: XSS issue in the codebase"

This...

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

Released this:

https://www.drupal.org/project/better_formats/releases/7.x-1.0-beta3

And for those using PHP 8.0, PHP 8.1, PHP 8.2, use the latest 2.0 release (currently 7.x-2.0-beta1)

https://www.drupal.org/project/better_formats/releases/7.x-2.0-beta1

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.