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
Comments
Comment #3
larowlanComment #4
larowlanComment #5
wim leersThere 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
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 👍Comment #6
alexpottI 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.
Comment #9
alexpottDiscussed with @xjm and @catch and we agreed to backport this to 9.4.x