Problem/Motivation

Drupal Core has a Server Side Template Injection vulnerability in the rendering of the Language button in the CKEditor.

Steps to reproduce

  • Make sure the ckeditor module is enabled (shipped within core)
  • A user with the translate interface permission can translate the source "Language": /admin/config/regional/translate
  • For example, the user fills in {{ 6 * 6 }} as translation for the source "Language"
  • The user saves the translations
  • Visit the page for adjusting ckeditor profiles:/admin/config/content/formats/manage/basic_html
  • Inspect the language button
  • The HTML of the button contains the translation as executed Twig:
    <a href="#" class="cke-icon-only" role="button" title="36" aria-label="36"><span class="cke_button_icon cke_button__language_icon">36</span></a>

This problem is caused by the usage of 'type' => 'inline_template' in the Language-class on line 68 (core/modules/ckeditor/src/Plugin/CKEditorPlugin/Language.php).
The value of the Language translation is not sanitised before usage in the Twig inline template.

This issue was reported privately to the Drupal security team who decided this could be solved in public because the permission required to translate the interface is a protected permission

Proposed resolution

use PlainTextOutput::renderFromHtml on the result and use the context argument instead of direct usage in the string

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 3331205.patch974 byteslarowlan

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Issue summary: View changes
larowlan’s picture

Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new974 bytes
wim leers’s picture

Status: Needs review » Reviewed & tested by the community

There won't be a Drupal 9.6, so … shall we move this to https://www.drupal.org/project/ckeditor?

This has zero impact for end users other than a extremely obscure security risk. It requires

A user with the translate interface permission can translate the source "Language": /admin/config/regional/translate

and interaction with the CKEditor 4 admin UI.

I'm fine with this landing though. But it should then be ported to the contrib module. It already is: #3331206: SSTI possible via translation of "Language" in CKEditor Language plugin 👍

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I checked CkEditor 5 and as far as I can see it is not affected.

Committed dcf22a4 and pushed to 9.5.x. Thanks!

I wonder if we should backport this security improvement to 9.4.

  • alexpott committed dcf22a42 on 9.5.x
    Issue #3331205 by larowlan, sanderwind: SSTI possible via translation of...

  • alexpott committed d7c15b6d on 9.4.x
    Issue #3331205 by larowlan, sanderwind: SSTI possible via translation of...
alexpott’s picture

Version: 9.5.x-dev » 9.4.x-dev

Discussed with @xjm and @catch and we agreed to backport this to 9.4.x

Status: Fixed » Closed (fixed)

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