From 878777a842d8c12545ce5152c8e117798a7ab4dd Mon Sep 17 00:00:00 2001 From: webchick <webchick@24967.no-reply.drupal.org> Date: Sat, 8 Feb 2014 12:32:05 -0800 Subject: [PATCH] Issue #2099741 by Wim Leers, wwalc, mr.baileys, eaton, dstol, nod_, effulgentsia: Protect WYSIWYG Editors from XSS Without Destroying User Data. --- core/lib/Drupal/Component/Utility/Xss.php | 51 +- .../Plugin/CKEditorPlugin/Internal.php | 3 +- .../ckeditor/Plugin/Editor/CKEditor.php | 3 +- .../ckeditor/Tests/CKEditorLoadingTest.php | 2 + core/modules/editor/editor.api.php | 27 + core/modules/editor/editor.module | 94 +++ core/modules/editor/editor.routing.yml | 7 + core/modules/editor/js/editor.js | 72 ++- .../lib/Drupal/editor/Annotation/Editor.php | 9 +- .../lib/Drupal/editor/EditorController.php | 43 +- .../editor/EditorXssFilter/Standard.php | 136 +++++ .../editor/EditorXssFilterInterface.php | 47 ++ .../lib/Drupal/editor/Plugin/EditorBase.php | 4 +- .../Drupal/editor/Plugin/EditorManager.php | 1 + .../editor/Plugin/InPlaceEditor/Editor.php | 3 +- .../Drupal/editor/Tests/EditorLoadingTest.php | 2 + .../Drupal/editor/Tests/EditorManagerTest.php | 1 + .../editor/Tests/EditorSecurityTest.php | 415 +++++++++++++ .../Tests/EditorXssFilter/StandardTest.php | 553 ++++++++++++++++++ .../editor/tests/modules/editor_test.module | 17 + .../editor_test/EditorXssFilter/Insecure.php | 26 + .../Plugin/Editor/UnicornEditor.php | 5 +- core/modules/filter/filter.module | 37 +- .../lib/Drupal/filter/Entity/FilterFormat.php | 3 +- .../filter/Plugin/Filter/FilterAutoP.php | 2 +- .../filter/Plugin/Filter/FilterCaption.php | 2 +- .../filter/Plugin/Filter/FilterHtml.php | 2 +- .../Plugin/Filter/FilterHtmlCorrector.php | 2 +- .../filter/Plugin/Filter/FilterHtmlEscape.php | 2 +- .../Plugin/Filter/FilterHtmlImageSecure.php | 2 +- .../filter/Plugin/Filter/FilterNull.php | 2 +- .../Drupal/filter/Plugin/Filter/FilterUrl.php | 2 +- .../Drupal/filter/Plugin/FilterInterface.php | 48 +- .../lib/Drupal/filter/Tests/FilterAPITest.php | 21 +- .../filter/Tests/FilterSecurityTest.php | 7 +- .../Plugin/Filter/FilterTestReplace.php | 2 +- .../FilterTestRestrictTagsAndAttributes.php | 2 +- .../Plugin/Filter/FilterTestUncacheable.php | 2 +- .../Tests/Component/Utility/XssTest.php | 77 ++- 39 files changed, 1631 insertions(+), 105 deletions(-) create mode 100644 core/modules/editor/lib/Drupal/editor/EditorXssFilter/Standard.php create mode 100644 core/modules/editor/lib/Drupal/editor/EditorXssFilterInterface.php create mode 100644 core/modules/editor/lib/Drupal/editor/Tests/EditorSecurityTest.php create mode 100644 core/modules/editor/tests/Drupal/editor/Tests/EditorXssFilter/StandardTest.php create mode 100644 core/modules/editor/tests/modules/lib/Drupal/editor_test/EditorXssFilter/Insecure.php diff --git a/core/lib/Drupal/Component/Utility/Xss.php b/core/lib/Drupal/Component/Utility/Xss.php index 0daab37e5807..ef35e2ade8cd 100644 --- a/core/lib/Drupal/Component/Utility/Xss.php +++ b/core/lib/Drupal/Component/Utility/Xss.php @@ -12,6 +12,18 @@ */ class Xss { + /** + * Indicates that XSS filtering must be applied in whitelist mode: only + * specified HTML tags are allowed. + */ + const FILTER_MODE_WHITELIST = TRUE; + + /** + * Indicates that XSS filtering must be applied in blacklist mode: only + * specified HTML tags are disallowed. + */ + const FILTER_MODE_BLACKLIST = FALSE; + /** * The list of html tags allowed by filterAdmin(). * @@ -35,10 +47,14 @@ class Xss { * javascript:). * * @param $string - * The string with raw HTML in it. It will be stripped of everything that can - * cause an XSS attack. - * @param array $allowed_tags - * An array of allowed tags. + * The string with raw HTML in it. It will be stripped of everything that + * can cause an XSS attack. + * @param array $html_tags + * An array of HTML tags. + * @param bool $mode + * (optional) Defaults to FILTER_MODE_WHITELIST ($html_tags is used as a + * whitelist of allowed tags), but can also be set to FILTER_MODE_BLACKLIST + * ($html_tags is used as a blacklist of disallowed tags). * * @return string * An XSS safe version of $string, or an empty string if $string is not @@ -48,14 +64,14 @@ class Xss { * * @ingroup sanitization */ - public static function filter($string, $allowed_tags = array('a', 'em', 'strong', 'cite', 'blockquote', 'code', 'ul', 'ol', 'li', 'dl', 'dt', 'dd')) { + public static function filter($string, $html_tags = array('a', 'em', 'strong', 'cite', 'blockquote', 'code', 'ul', 'ol', 'li', 'dl', 'dt', 'dd'), $mode = Xss::FILTER_MODE_WHITELIST) { // Only operate on valid UTF-8 strings. This is necessary to prevent cross // site scripting issues on Internet Explorer 6. if (!Unicode::validateUtf8($string)) { return ''; } // Store the text format. - static::split($allowed_tags, TRUE); + static::split($html_tags, TRUE, $mode); // Remove NULL characters (ignored by some browsers). $string = str_replace(chr(0), '', $string); // Remove Netscape 4 JS entities. @@ -80,7 +96,7 @@ public static function filter($string, $allowed_tags = array('a', 'em', 'strong' <[^>]*(>|$) # a string that starts with a <, up until the > or the end of the string | # or > # just a > - )%x', '\Drupal\Component\Utility\Xss::split', $string); + )%x', 'static::split', $string); } /** @@ -112,17 +128,22 @@ public static function filterAdmin($string) { * If $store is TRUE then the array contains the allowed tags. * If $store is FALSE then the array has one element, the HTML tag to process. * @param bool $store - * Whether to store $m. + * Whether to store $matches. + * @param bool $mode + * (optional) Ignored when $store is FALSE, otherwise used to determine + * whether $matches is a list of allowed (if FILTER_MODE_WHITELIST) or + * disallowed (if FILTER_MODE_BLACKLIST) HTML tags. * * @return string * If the element isn't allowed, an empty string. Otherwise, the cleaned up * version of the HTML element. */ - protected static function split($matches, $store = FALSE) { - static $allowed_html; + protected static function split($matches, $store = FALSE, $mode = Xss::FILTER_MODE_WHITELIST) { + static $html_tags, $split_mode; if ($store) { - $allowed_html = array_flip($matches); + $html_tags = array_flip($matches); + $split_mode = $mode; return; } @@ -151,8 +172,12 @@ protected static function split($matches, $store = FALSE) { $elem = '!--'; } - if (!isset($allowed_html[strtolower($elem)])) { - // Disallowed HTML element. + // When in whitelist mode, an element is disallowed when not listed. + if ($split_mode === static::FILTER_MODE_WHITELIST && !isset($html_tags[strtolower($elem)])) { + return ''; + } + // When in blacklist mode, an element is disallowed when listed. + elseif ($split_mode === static::FILTER_MODE_BLACKLIST && isset($html_tags[strtolower($elem)])) { return ''; } diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php index e0e23aad2f69..c683e3e2fd34 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/CKEditorPlugin/Internal.php @@ -10,6 +10,7 @@ use Drupal\ckeditor\CKEditorPluginBase; use Drupal\Component\Utility\NestedArray; use Drupal\editor\Entity\Editor; +use Drupal\filter\Plugin\FilterInterface; /** * Defines the "internal" plugin (i.e. core plugins part of our CKEditor build). @@ -287,7 +288,7 @@ protected function generateAllowedContentSetting(Editor $editor) { // When nothing is disallowed, set allowedContent to true. $format = entity_load('filter_format', $editor->format); $filter_types = $format->getFilterTypes(); - if (!in_array(FILTER_TYPE_HTML_RESTRICTOR, $filter_types)) { + if (!in_array(FilterInterface::TYPE_HTML_RESTRICTOR, $filter_types)) { return TRUE; } // Generate setting that accurately reflects allowed tags and attributes. diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php index 6b880e3aacbb..21db7222f2e1 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Plugin/Editor/CKEditor.php @@ -24,7 +24,8 @@ * id = "ckeditor", * label = @Translation("CKEditor"), * supports_content_filtering = TRUE, - * supports_inline_editing = TRUE + * supports_inline_editing = TRUE, + * is_xss_safe = FALSE * ) */ class CKEditor extends EditorBase implements ContainerFactoryPluginInterface { diff --git a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorLoadingTest.php b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorLoadingTest.php index e97c7e77b7e4..fa3fc7acb037 100644 --- a/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorLoadingTest.php +++ b/core/modules/ckeditor/lib/Drupal/ckeditor/Tests/CKEditorLoadingTest.php @@ -104,6 +104,7 @@ function testLoading() { 'editor' => 'ckeditor', 'editorSettings' => $ckeditor_plugin->getJSSettings($editor), 'editorSupportsContentFiltering' => TRUE, + 'isXssSafe' => FALSE, ))); $this->assertTrue($editor_settings_present, "Text Editor module's JavaScript settings are on the page."); $this->assertIdentical($expected, $settings['editor'], "Text Editor module's JavaScript settings on the page are correct."); @@ -131,6 +132,7 @@ function testLoading() { 'editor' => 'ckeditor', 'editorSettings' => $ckeditor_plugin->getJSSettings($editor), 'editorSupportsContentFiltering' => TRUE, + 'isXssSafe' => FALSE, ))); $this->assertTrue($editor_settings_present, "Text Editor module's JavaScript settings are on the page."); $this->assertIdentical($expected, $settings['editor'], "Text Editor module's JavaScript settings on the page are correct."); diff --git a/core/modules/editor/editor.api.php b/core/modules/editor/editor.api.php index fe9548b12837..681399a9ddef 100644 --- a/core/modules/editor/editor.api.php +++ b/core/modules/editor/editor.api.php @@ -5,6 +5,8 @@ * Documentation for Text Editor API. */ +use Drupal\filter\FilterFormatInterface; + /** * @addtogroup hooks * @{ @@ -103,6 +105,31 @@ function hook_editor_js_settings_alter(array &$settings, array $formats) { } } +/** + * Modifies the text editor XSS filter that will used for the given text format. + * + * Is only called when an EditorXssFilter will effectively be used; this hook + * does not allow one to alter that decision. + * + * @param string &$editor_xss_filter_class + * The text editor XSS filter class that will be used. + * @param \Drupal\filter\FilterFormatInterface $format + * The text format configuration entity. Provides context based upon which + * one may want to adjust the filtering. + * @param \Drupal\filter\FilterFormatInterface $original_format|null + * (optional) The original text format configuration entity (when switching + * text formats/editors). Also provides context based upon which one may want + * to adjust the filtering. + * + * @see \Drupal\editor\EditorXssFilterInterface + */ +function hook_editor_xss_filter_alter(&$editor_xss_filter_class, FilterFormatInterface $format, FilterFormatInterface $original_format = NULL) { + $filters = $format->filters()->getAll(); + if (isset($filters['filter_wysiwyg']) && $filters['filter_wysiwyg']->status) { + $editor_xss_filter_class = '\Drupal\filter_wysiwyg\EditorXssFilter\WysiwygFilter'; + } +} + /** * @} End of "addtogroup hooks". */ diff --git a/core/modules/editor/editor.module b/core/modules/editor/editor.module index f638282d34f9..a0bf85ea1f95 100644 --- a/core/modules/editor/editor.module +++ b/core/modules/editor/editor.module @@ -10,6 +10,8 @@ use Drupal\Component\Utility\NestedArray; use Drupal\Core\Entity\EntityInterface; use Drupal\field\Field; +use Drupal\filter\FilterFormatInterface; +use Drupal\filter\Plugin\FilterInterface; /** * Implements hook_help(). @@ -377,9 +379,101 @@ function editor_pre_render_format($element) { $manager = \Drupal::service('plugin.manager.editor'); $element['#attached'] = NestedArray::mergeDeep($element['#attached'], $manager->getAttachments($format_ids)); + // Apply XSS filters when editing content if necessary. Some types of text + // editors cannot guarantee that the end user won't become a victim of XSS. + if (!empty($element['value']['#value'])) { + $original = $element['value']['#value']; + $format = entity_load('filter_format', $element['format']['format']['#value']); + + // Ensure XSS-safety for the current text format/editor. + $filtered = editor_filter_xss($original, $format); + if ($filtered !== FALSE) { + $element['value']['#value'] = $filtered; + } + + // Only when the user has access to multiple text formats, we must add data- + // attributes for the original value and change tracking, because they are + // only necessary when the end user can switch between text formats/editors. + if ($element['format']['format']['#access']) { + $element['value']['#attributes']['data-editor-value-is-changed'] = 'false'; + $element['value']['#attributes']['data-editor-value-original'] = $original; + } + } + return $element; } +/** + * Applies text editor XSS filtering. + * + * @param string $html + * The HTML string that will be passed to the text editor. + * @param \Drupal\filter\FilterFormatInterface $format + * The text format whose text editor will be used. + * @param \Drupal\filter\FilterFormatInterface $original_format|null + * (optional) The original text format (i.e. when switching text formats, + * $format is the text format that is going to be used, $original_format is + * the one that was being used initially, the one that is stored in the + * database when editing). + * + * @return string|false + * FALSE when no XSS filtering needs to be applied (either because no text + * editor is associated with the text format, or because the text editor is + * safe from XSS attacks, or because the text format does not use any XSS + * protection filters), otherwise the XSS filtered string. + * + * @see https://drupal.org/node/2099741 + */ +function editor_filter_xss($html, FilterFormatInterface $format, FilterFormatInterface $original_format = NULL) { + $editor = editor_load($format->id()); + + // If no text editor is associated with this text format, then we don't need + // text editor XSS filtering either. + if (!isset($editor)) { + return FALSE; + } + + // If the text editor associated with this text format guarantees security, + // then we also don't need text editor XSS filtering. + $definition = \Drupal::service('plugin.manager.editor')->getDefinition($editor->editor); + if ($definition['is_xss_safe'] === TRUE) { + return FALSE; + } + + // If there is no filter preventing XSS attacks in the text format being used, + // then no text editor XSS filtering is needed either. (Because then the + // editing user can already be attacked by merely viewing the content.) + // e.g.: an admin user creates content in Full HTML and then edits it, no text + // format switching happens; in this case, no text editor XSS filtering is + // desirable, because it would strip style attributes, amongst others. + $current_filter_types = $format->getFilterTypes(); + if (!in_array(FilterInterface::TYPE_HTML_RESTRICTOR, $current_filter_types, TRUE)) { + if ($original_format === NULL) { + return FALSE; + } + // Unless we are switching from another text format, in which case we must + // first check whether a filter preventing XSS attacks is used in that text + // format, and if so, we must still apply XSS filtering. + // e.g.: an anonymous user creates content in Restricted HTML, an admin user + // edits it (then no XSS filtering is applied because no text editor is + // used), and switches to Full HTML (for which a text editor is used). Then + // we must apply XSS filtering to protect the admin user. + else { + $original_filter_types = $original_format->getFilterTypes(); + if (!in_array(FilterInterface::TYPE_HTML_RESTRICTOR, $original_filter_types, TRUE)) { + return FALSE; + } + } + } + + // Otherwise, apply the text editor XSS filter. We use the default one unless + // a module tells us to use a different one. + $editor_xss_filter_class = '\Drupal\editor\EditorXssFilter\Standard'; + \Drupal::moduleHandler()->alter('editor_xss_filter', $editor_xss_filter_class, $format, $original_format); + + return call_user_func($editor_xss_filter_class . '::filterXss', $html, $format, $original_format); +} + /** * Implements hook_entity_insert(). */ diff --git a/core/modules/editor/editor.routing.yml b/core/modules/editor/editor.routing.yml index bf9d3607c52d..2a753f57ad4f 100644 --- a/core/modules/editor/editor.routing.yml +++ b/core/modules/editor/editor.routing.yml @@ -1,3 +1,10 @@ +editor.filter_xss: + path: '/editor/filter_xss/{filter_format}' + defaults: + _controller: '\Drupal\editor\EditorController::filterXss' + requirements: + _entity_access: 'filter_format.view' + editor.field_untransformed_text: path: '/editor/{entity_type}/{entity}/{field_name}/{langcode}/{view_mode_id}' defaults: diff --git a/core/modules/editor/js/editor.js b/core/modules/editor/js/editor.js index ebaf5e4bbeb5..d538b682a4f7 100644 --- a/core/modules/editor/js/editor.js +++ b/core/modules/editor/js/editor.js @@ -34,14 +34,22 @@ * attached. */ function changeTextEditor($formatSelector, activeFormatID, newFormatID) { + var originalFormatID = activeFormatID; var field = findFieldForFormatSelector($formatSelector); // Detach the current editor (if any) and attach a new editor. if (drupalSettings.editor.formats[activeFormatID]) { Drupal.editorDetach(field, drupalSettings.editor.formats[activeFormatID]); } + // When no text editor is currently active, stop tracking changes. + else if (!drupalSettings.editor.formats[activeFormatID]) { + $(field).off('.editor'); + } activeFormatID = newFormatID; + + // Attach the new text editor (if any). if (drupalSettings.editor.formats[activeFormatID]) { - Drupal.editorAttach(field, drupalSettings.editor.formats[activeFormatID]); + var format = drupalSettings.editor.formats[activeFormatID]; + filterXssWhenSwitching(field, format, originalFormatID, Drupal.editorAttach); } $formatSelector.attr('data-editor-active-text-format', newFormatID); } @@ -135,10 +143,22 @@ $this.attr('data-editor-active-text-format', activeFormatID); var field = findFieldForFormatSelector($this); - // Directly attach this editor, if the text format is enabled. + // Directly attach this text editor, if the text format is enabled. if (settings.editor.formats[activeFormatID]) { + // XSS protection for the current text format/editor is performed on the + // server side, so we don't need to do anything special here. Drupal.editorAttach(field, settings.editor.formats[activeFormatID]); } + // When there is no text editor for this text format, still track changes, + // because the user has the ability to switch to some text editor, other- + // wise this code would not be executed. + else { + $(field).on('change.editor keypress.editor', function () { + field.setAttribute('data-editor-value-is-changed', 'true'); + // Just knowing that the value was changed is enough, stop tracking. + $(field).off('.editor'); + }); + } // Attach onChange handler to text format selector element. if ($this.is('select')) { @@ -200,6 +220,10 @@ // happen within the text editor. Drupal.editors[format.editor].onChange(field, function () { $(field).trigger('formUpdated'); + + // Keep track of changes, so we know what to do when switching text + // formats and guaranteeing XSS protection. + field.setAttribute('data-editor-value-is-changed', 'true'); }); } }; @@ -214,7 +238,51 @@ } Drupal.editors[format.editor].detach(field, format, trigger); + + // Restore the original value if the user didn't make any changes yet. + if (field.getAttribute('data-editor-value-is-changed') === 'false') { + field.value = field.getAttribute('data-editor-value-original'); + } } }; + /** + * Filter away XSS attack vectors when switching text formats. + * + * @param DOM field + * The textarea DOM element. + * @param Object format + * The text format that's being activated, from drupalSettings.editor.formats. + * @param String originalFormatID + * The text format ID of the original text format. + * @param Function callback + * A callback to be called (with no parameters) after the field's value has + * been XSS filtered. + */ + function filterXssWhenSwitching (field, format, originalFormatID, callback) { + // A text editor that already is XSS-safe needs no additional measures. + if (format.editor.isXssSafe) { + callback(field, format); + } + // Otherwise, ensure XSS safety: let the server XSS filter this value. + else { + $.ajax({ + url: Drupal.url('editor/filter_xss/' + format.format), + type: 'POST', + data: { + 'value': field.value, + 'original_format_id': originalFormatID + }, + dataType: 'json', + success: function (xssFilteredValue) { + // If the server returns false, then no XSS filtering is needed. + if (xssFilteredValue !== false) { + field.value = xssFilteredValue; + } + callback(field, format); + } + }); + } + } + })(jQuery, Drupal, drupalSettings); diff --git a/core/modules/editor/lib/Drupal/editor/Annotation/Editor.php b/core/modules/editor/lib/Drupal/editor/Annotation/Editor.php index be126b44627c..32ac3dbe19c1 100644 --- a/core/modules/editor/lib/Drupal/editor/Annotation/Editor.php +++ b/core/modules/editor/lib/Drupal/editor/Annotation/Editor.php @@ -42,8 +42,15 @@ class Editor extends Plugin { /** * Whether the editor supports the inline editing provided by the Edit module. * - * @var boolean + * @var bool */ public $supports_inline_editing; + /** + * Whether this text editor is not vulnerable to XSS attacks. + * + * @var bool + */ + public $is_xss_safe; + } diff --git a/core/modules/editor/lib/Drupal/editor/EditorController.php b/core/modules/editor/lib/Drupal/editor/EditorController.php index 27c874d95e23..0e65571d5ce3 100644 --- a/core/modules/editor/lib/Drupal/editor/EditorController.php +++ b/core/modules/editor/lib/Drupal/editor/EditorController.php @@ -10,17 +10,21 @@ use Drupal\Core\Ajax\AjaxResponse; use Drupal\Core\Ajax\OpenModalDialogCommand; use Drupal\Core\Ajax\CloseModalDialogCommand; +use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Entity\EntityInterface; use Drupal\editor\Ajax\GetUntransformedTextCommand; use Drupal\editor\Form\EditorImageDialog; use Drupal\editor\Form\EditorLinkDialog; -use Drupal\filter\Entity\FilterFormat; -use Symfony\Component\DependencyInjection\ContainerAware; +use Drupal\filter\Plugin\FilterInterface; +use Drupal\filter\FilterFormatInterface; +use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** * Returns responses for Editor module routes. */ -class EditorController extends ContainerAware { +class EditorController extends ControllerBase { /** * Returns an Ajax response to render a text field without transformation filters. @@ -43,10 +47,41 @@ public function getUntransformedText(EntityInterface $entity, $field_name, $lang // Direct text editing is only supported for single-valued fields. $field = $entity->getTranslation($langcode)->$field_name; - $editable_text = check_markup($field->value, $field->format, $langcode, FALSE, array(FILTER_TYPE_TRANSFORM_REVERSIBLE, FILTER_TYPE_TRANSFORM_IRREVERSIBLE)); + $editable_text = check_markup($field->value, $field->format, $langcode, FALSE, array(FilterInterface::TYPE_TRANSFORM_REVERSIBLE, FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE)); $response->addCommand(new GetUntransformedTextCommand($editable_text)); return $response; } + /** + * Apply the necessary XSS filtering for using a certain text format's editor. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request object. + * @param \Drupal\filter\FilterFormatInterface $filter_format + * The text format whose text editor (if any) will be used. + * + * @return \Symfony\Component\HttpFoundation\JsonResponse + * A JSON response containing the XSS-filtered value. + * + * @see editor_filter_xss() + */ + public function filterXss(Request $request, FilterFormatInterface $filter_format) { + $value = $request->request->get('value'); + if (!isset($value)) { + throw new NotFoundHttpException(); + } + + // The original_format parameter will only exist when switching text format. + $original_format_id = $request->request->get('original_format_id'); + $original_format = NULL; + if (isset($original_format_id)) { + $original_format = $this->entityManager() + ->getStorageController('filter_format') + ->load($original_format_id); + } + + return new JsonResponse(editor_filter_xss($value, $filter_format, $original_format)); + } + } diff --git a/core/modules/editor/lib/Drupal/editor/EditorXssFilter/Standard.php b/core/modules/editor/lib/Drupal/editor/EditorXssFilter/Standard.php new file mode 100644 index 000000000000..3f9386342c9c --- /dev/null +++ b/core/modules/editor/lib/Drupal/editor/EditorXssFilter/Standard.php @@ -0,0 +1,136 @@ +<?php + +/** + * @file + * Contains \Drupal\editor\EditorXssFilter\Standard. + */ + +namespace Drupal\editor\EditorXssFilter; + +use Drupal\Component\Utility\Xss; +use Drupal\filter\FilterFormatInterface; +use Drupal\editor\EditorXssFilterInterface; + +/** + * Defines the standard text editor XSS filter. + */ +class Standard implements EditorXssFilterInterface { + + /** + * {@inheritdoc} + */ + public static function filterXss($html, FilterFormatInterface $format, FilterFormatInterface $original_format = NULL) { + // Apply XSS filtering, but blacklist the <script>, <style>, <link>, <embed> + // and <object> tags. + // The <script> and <style> tags are blacklisted because their contents + // can be malicious (and therefor they are inherently unsafe), whereas for + // all other tags, only their attributes can make them malicious. Since + // Xss::filter() protects against malicious attributes, we take no + // blacklisting action. + // The exceptions to the above rule are <link>, <embed> and <object>: + // - <link> because the href attribute allows the attacker to import CSS + // using the HTTP(S) protocols which Xss::filter() considers safe by + // default. The imported remote CSS is applied to the main document, thus + // allowing for the same XSS attacks as a regular <style> tag. + // - <embed> and <object> because these tags allow non-HTML applications or + // content to be embedded using the src or data attributes, respectively. + // This is safe in the case of HTML documents, but not in the case of + // Flash objects for example, that may access/modify the main document + // directly. + // <iframe> is considered safe because it only allows HTML content to be + // embedded, hence ensuring the same origin policy always applies. + $dangerous_tags = array('script', 'style', 'link', 'embed', 'object'); + + // Simply blacklisting these five dangerious tags would bring safety, but + // also user frustration: what if a text format is configured to allow + // <embed>, for example? Then we would strip that tag, even though it is + // allowed, thereby causing data loss! + // Therefor, we want to be smarter still. We want to take into account which + // HTML tags are allowed and forbidden by the text format we're filtering + // for, and if we're switching from another text format, we want to take + // that format's allowed and forbidden tags into account as well. + // In other words: we only expect markup allowed in both the original and + // the new format to continue to exist. + $format_restrictions = $format->getHtmlRestrictions(); + if ($original_format !== NULL) { + $original_format_restrictions = $original_format->getHtmlRestrictions(); + } + + // Any tags that are explicitly blacklisted by the text format must be + // appended to the list of default dangerous tags: if they're explicitly + // forbidden, then we must respect that configuration. + // When switching from another text format, we must use the union of + // forbidden tags: if either text format is more restrictive, then the + // safety expectations of *both* text formats apply. + $forbidden_tags = self::getForbiddenTags($format_restrictions); + if ($original_format !== NULL) { + $forbidden_tags = array_merge($forbidden_tags, self::getForbiddenTags($original_format_restrictions)); + } + + // Any tags that are explicitly whitelisted by the text format must be + // removed from the list of default dangerous tags: if they're explicitly + // allowed, then we must respect that configuration. + // When switching from another format, we must use the intersection of + // allowed tags: if either format is more restrictive, then the safety + // expectations of *both* formats apply. + $allowed_tags = self::getAllowedTags($format_restrictions); + if ($original_format !== NULL) { + $allowed_tags = array_intersect($allowed_tags, self::getAllowedTags($original_format_restrictions)); + } + + // Don't blacklist dangerous tags that are explicitly allowed in both text + // formats. + $blacklisted_tags = array_diff($dangerous_tags, $allowed_tags); + + // Also blacklist tags that are explicitly forbidden in either text format. + $blacklisted_tags = array_merge($blacklisted_tags, $forbidden_tags); + + return Xss::filter($html, $blacklisted_tags, Xss::FILTER_MODE_BLACKLIST); + } + + + /** + * Get all allowed tags from a restrictions data structure. + * + * @param array|FALSE $restrictions + * Restrictions as returned by FilterInterface::getHTMLRestrictions(). + * + * @return array + * An array of allowed HTML tags. + * + * @see \Drupal\filter\Plugin\Filter\FilterInterface::getHTMLRestrictions() + */ + protected static function getAllowedTags($restrictions) { + if ($restrictions === FALSE || !isset($restrictions['allowed'])) { + return array(); + } + + $allowed_tags = array_keys($restrictions['allowed']); + // Exclude the wildcard tag, which is used to set attribute restrictions on + // all tags simultaneously. + $allowed_tags = array_diff($allowed_tags, array('*')); + + return $allowed_tags; + } + + /** + * Get all forbidden tags from a restrictions data structure. + * + * @param array|FALSE $restrictions + * Restrictions as returned by FilterInterface::getHTMLRestrictions(). + * + * @return array + * An array of forbidden HTML tags. + * + * @see \Drupal\filter\Plugin\Filter\FilterInterface::getHTMLRestrictions() + */ + protected static function getForbiddenTags($restrictions) { + if ($restrictions === FALSE || !isset($restrictions['forbidden_tags'])) { + return array(); + } + else { + return $restrictions['forbidden_tags']; + } + } + +} diff --git a/core/modules/editor/lib/Drupal/editor/EditorXssFilterInterface.php b/core/modules/editor/lib/Drupal/editor/EditorXssFilterInterface.php new file mode 100644 index 000000000000..7575ab26e540 --- /dev/null +++ b/core/modules/editor/lib/Drupal/editor/EditorXssFilterInterface.php @@ -0,0 +1,47 @@ +<?php + +/** + * @file + * Contains \Drupal\editor\EditorXssFilterInterface. + */ + +namespace Drupal\editor; + +use Drupal\filter\FilterFormatInterface; + +/** + * Defines an interface for text editor XSS (Cross-site scripting) filters. + */ +interface EditorXssFilterInterface { + + /** + * Filters HTML to prevent XSS attacks when a user edits it in a text editor. + * + * Should filter as minimally as possible, only to remove XSS attack vectors. + * + * Is only called when: + * - loading a non-XSS-safe text editor for a $format that contains a filter + * preventing XSS attacks (a FilterInterface::TYPE_HTML_RESTRICTOR filter): + * if the output is safe, it should also be safe to edit. + * - loading a non-XSS-safe text editor for a $format that doesn't contain a + * filter preventing XSS attacks, but we're switching from a previous text + * format ($original_format is not NULL) that did prevent XSS attacks: if + * the output was previously safe, it should be safe to switch to another + * text format and edit. + * + * @param string $html + * The HTML to be filtered. + * @param \Drupal\filter\FilterFormatInterface $format + * The text format configuration entity. Provides context based upon which + * one may want to adjust the filtering. + * @param \Drupal\filter\FilterFormatInterface $original_format|null + * (optional) The original text format configuration entity (when switching + * text formats/editors). Also provides context based upon which one may + * want to adjust the filtering. + * + * @return string + * The filtered HTML that cannot cause any XSSes anymore. + */ + public static function filterXss($html, FilterFormatInterface $format, FilterFormatInterface $original_format = NULL); + +} diff --git a/core/modules/editor/lib/Drupal/editor/Plugin/EditorBase.php b/core/modules/editor/lib/Drupal/editor/Plugin/EditorBase.php index 20a4169da652..32e44089fb09 100644 --- a/core/modules/editor/lib/Drupal/editor/Plugin/EditorBase.php +++ b/core/modules/editor/lib/Drupal/editor/Plugin/EditorBase.php @@ -28,6 +28,7 @@ * only" filtering. * - supports_inline_editing: Whether the editor supports the inline editing * provided by the Edit module. + * - is_xss_safe: Whether this text editor is not vulnerable to XSS attacks. * * A complete sample plugin definition should be defined as in this example: * @@ -36,7 +37,8 @@ * id = "myeditor", * label = @Translation("My Editor"), * supports_content_filtering = FALSE, - * supports_inline_editing = FALSE + * supports_inline_editing = FALSE, + * is_xss_safe = FALSE * ) * @endcode */ diff --git a/core/modules/editor/lib/Drupal/editor/Plugin/EditorManager.php b/core/modules/editor/lib/Drupal/editor/Plugin/EditorManager.php index 558cfde1ce96..5e33b7bf8c7c 100644 --- a/core/modules/editor/lib/Drupal/editor/Plugin/EditorManager.php +++ b/core/modules/editor/lib/Drupal/editor/Plugin/EditorManager.php @@ -83,6 +83,7 @@ public function getAttachments(array $format_ids) { 'editor' => $editor->editor, 'editorSettings' => $plugin->getJSSettings($editor), 'editorSupportsContentFiltering' => $plugin_definition['supports_content_filtering'], + 'isXssSafe' => $plugin_definition['is_xss_safe'], ); } diff --git a/core/modules/editor/lib/Drupal/editor/Plugin/InPlaceEditor/Editor.php b/core/modules/editor/lib/Drupal/editor/Plugin/InPlaceEditor/Editor.php index 45db0e0b5feb..8752efbbc32f 100644 --- a/core/modules/editor/lib/Drupal/editor/Plugin/InPlaceEditor/Editor.php +++ b/core/modules/editor/lib/Drupal/editor/Plugin/InPlaceEditor/Editor.php @@ -10,6 +10,7 @@ use Drupal\Component\Plugin\PluginBase; use Drupal\Core\Field\FieldItemListInterface; use Drupal\edit\Plugin\InPlaceEditorInterface; +use Drupal\filter\Plugin\FilterInterface; /** * Defines the formatted text in-place editor. @@ -61,7 +62,7 @@ function getMetadata(FieldItemListInterface $items) { */ protected function textFormatHasTransformationFilters($format_id) { $format = entity_load('filter_format', $format_id); - return (bool) count(array_intersect(array(FILTER_TYPE_TRANSFORM_REVERSIBLE, FILTER_TYPE_TRANSFORM_IRREVERSIBLE), $format->getFiltertypes())); + return (bool) count(array_intersect(array(FilterInterface::TYPE_TRANSFORM_REVERSIBLE, FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE), $format->getFiltertypes())); } /** diff --git a/core/modules/editor/lib/Drupal/editor/Tests/EditorLoadingTest.php b/core/modules/editor/lib/Drupal/editor/Tests/EditorLoadingTest.php index ffbf9a82ae26..59dde52c07f1 100644 --- a/core/modules/editor/lib/Drupal/editor/Tests/EditorLoadingTest.php +++ b/core/modules/editor/lib/Drupal/editor/Tests/EditorLoadingTest.php @@ -97,6 +97,7 @@ public function testLoading() { 'editor' => 'unicorn', 'editorSettings' => array('ponyModeEnabled' => TRUE), 'editorSupportsContentFiltering' => TRUE, + 'isXssSafe' => FALSE, ))); $this->assertTrue($editor_settings_present, "Text Editor module's JavaScript settings are on the page."); $this->assertIdentical($expected, $settings['editor'], "Text Editor module's JavaScript settings on the page are correct."); @@ -125,6 +126,7 @@ public function testLoading() { 'editor' => 'unicorn', 'editorSettings' => array('ponyModeEnabled' => TRUE), 'editorSupportsContentFiltering' => TRUE, + 'isXssSafe' => FALSE, ))); $this->assertTrue($editor_settings_present, "Text Editor module's JavaScript settings are on the page."); $this->assertIdentical($expected, $settings['editor'], "Text Editor module's JavaScript settings on the page are correct."); diff --git a/core/modules/editor/lib/Drupal/editor/Tests/EditorManagerTest.php b/core/modules/editor/lib/Drupal/editor/Tests/EditorManagerTest.php index 78140c79b585..d97f98944e67 100644 --- a/core/modules/editor/lib/Drupal/editor/Tests/EditorManagerTest.php +++ b/core/modules/editor/lib/Drupal/editor/Tests/EditorManagerTest.php @@ -105,6 +105,7 @@ public function testManager() { 'editor' => 'unicorn', 'editorSettings' => $unicorn_plugin->getJSSettings($editor), 'editorSupportsContentFiltering' => TRUE, + 'isXssSafe' => FALSE, ) ))) ) diff --git a/core/modules/editor/lib/Drupal/editor/Tests/EditorSecurityTest.php b/core/modules/editor/lib/Drupal/editor/Tests/EditorSecurityTest.php new file mode 100644 index 000000000000..52a6de57a272 --- /dev/null +++ b/core/modules/editor/lib/Drupal/editor/Tests/EditorSecurityTest.php @@ -0,0 +1,415 @@ +<?php + +/** + * @file + * Definition of \Drupal\editor\Tests\EditorSecurityTest. + */ + +namespace Drupal\editor\Tests; + +use Drupal\simpletest\WebTestBase; +use Drupal\Component\Utility\String; + +/** + * Tests XSS protection for content creators when using text editors. + */ +class EditorSecurityTest extends WebTestBase { + + /** + * The sample content to use in all tests. + * + * @var string + */ + protected static $sampleContent = '<p style="color: red">Hello, Dumbo Octopus!</p><script>alert(0)</script><embed type="image/svg+xml" src="image.svg" />'; + + /** + * The secured sample content to use in most tests. + * + * @var string + */ + protected static $sampleContentSecured = '<p>Hello, Dumbo Octopus!</p>alert(0)'; + + /** + * The secured sample content to use in tests when the <embed> tag is allowed. + * + * @var string + */ + protected static $sampleContentSecuredEmbedAllowed = '<p>Hello, Dumbo Octopus!</p>alert(0)<embed type="image/svg+xml" src="image.svg" />'; + + /** + * Modules to enable. + * + * @var array + */ + public static $modules = array('filter', 'editor', 'editor_test', 'node'); + + public static function getInfo() { + return array( + 'name' => 'Text editor security', + 'description' => 'Tests XSS protection for content creators when using text editors.', + 'group' => 'Text Editor', + ); + } + + function setUp() { + parent::setUp(); + + // Create 5 text formats, to cover all potential use cases: + // 1. restricted_without_editor (untrusted: anonymous) + // 2. restricted_with_editor (normal: authenticated) + // 3. restricted_plus_dangerous_tag_with_editor (privileged: trusted) + // 4. unrestricted_without_editor (privileged: admin) + // 5. unrestricted_with_editor (privileged: admin) + // With text formats 2, 3 and 5, we also associate a text editor that does + // not guarantee XSS safety. "restricted" means the text format has XSS + // filters on output, "unrestricted" means the opposite. + $format = entity_create('filter_format', array( + 'format' => 'restricted_without_editor', + 'name' => 'Restricted HTML, without text editor', + 'weight' => 0, + 'filters' => array( + // A filter of the FilterInterface::TYPE_HTML_RESTRICTOR type. + 'filter_html' => array( + 'status' => 1, + 'settings' => array( + 'allowed_html' => '<h4> <h5> <h6> <p> <br> <strong> <a>', + ) + ), + ), + )); + $format->save(); + $format = entity_create('filter_format', array( + 'format' => 'restricted_with_editor', + 'name' => 'Restricted HTML, with text editor', + 'weight' => 1, + 'filters' => array( + // A filter of the FilterInterface::TYPE_HTML_RESTRICTOR type. + 'filter_html' => array( + 'status' => 1, + 'settings' => array( + 'allowed_html' => '<h4> <h5> <h6> <p> <br> <strong> <a>', + ) + ), + ), + )); + $format->save(); + $editor = entity_create('editor', array( + 'format' => 'restricted_with_editor', + 'editor' => 'unicorn', + )); + $editor->save(); + $format = entity_create('filter_format', array( + 'format' => 'restricted_plus_dangerous_tag_with_editor', + 'name' => 'Restricted HTML, dangerous tag allowed, with text editor', + 'weight' => 1, + 'filters' => array( + // A filter of the FilterInterface::TYPE_HTML_RESTRICTOR type. + 'filter_html' => array( + 'status' => 1, + 'settings' => array( + 'allowed_html' => '<h4> <h5> <h6> <p> <br> <strong> <a> <embed>', + ) + ), + ), + )); + $format->save(); + $editor = entity_create('editor', array( + 'format' => 'restricted_plus_dangerous_tag_with_editor', + 'editor' => 'unicorn', + )); + $editor->save(); + $format = entity_create('filter_format', array( + 'format' => 'unrestricted_without_editor', + 'name' => 'Unrestricted HTML, without text editor', + 'weight' => 0, + 'filters' => array(), + )); + $format->save(); + $format = entity_create('filter_format', array( + 'format' => 'unrestricted_with_editor', + 'name' => 'Unrestricted HTML, with text editor', + 'weight' => 1, + 'filters' => array(), + )); + $format->save(); + $editor = entity_create('editor', array( + 'format' => 'unrestricted_with_editor', + 'editor' => 'unicorn', + )); + $editor->save(); + + + // Create node type. + $this->drupalCreateContentType(array( + 'type' => 'article', + 'name' => 'Article', + )); + + // Create 3 users, each with access to different text formats/editors: + // - "untrusted": restricted_without_editor + // - "normal": restricted_with_editor, + // - "trusted": restricted_plus_dangerous_tag_with_editor + // - "privileged": restricted_without_editor, restricted_with_editor, + // restricted_plus_dangerous_tag_with_editor, + // unrestricted_without_editor and unrestricted_with_editor + $this->untrusted_user = $this->drupalCreateUser(array( + 'create article content', + 'edit any article content', + 'use text format restricted_without_editor', + )); + $this->normal_user = $this->drupalCreateUser(array( + 'create article content', + 'edit any article content', + 'use text format restricted_with_editor', + )); + $this->trusted_user = $this->drupalCreateUser(array( + 'create article content', + 'edit any article content', + 'use text format restricted_plus_dangerous_tag_with_editor', + )); + $this->privileged_user = $this->drupalCreateUser(array( + 'create article content', + 'edit any article content', + 'use text format restricted_without_editor', + 'use text format restricted_with_editor', + 'use text format restricted_plus_dangerous_tag_with_editor', + 'use text format unrestricted_without_editor', + 'use text format unrestricted_with_editor', + )); + + // Create an "article" node for each possible text format, with the same + // sample content, to do our tests on. + $samples = array( + array('author' => $this->untrusted_user->id(), 'format' => 'restricted_without_editor'), + array('author' => $this->normal_user->id(), 'format' => 'restricted_with_editor'), + array('author' => $this->trusted_user->id(), 'format' => 'restricted_plus_dangerous_tag_with_editor'), + array('author' => $this->privileged_user->id(), 'format' => 'unrestricted_without_editor'), + array('author' => $this->privileged_user->id(), 'format' => 'unrestricted_with_editor'), + ); + foreach ($samples as $sample) { + $this->drupalCreateNode(array( + 'type' => 'article', + 'body' => array( + array('value' => self::$sampleContent, 'format' => $sample['format']) + ), + 'uid' => $sample['author'] + )); + } + } + + /** + * Tests initial security: is the user safe without switching text formats? + * + * Tests 8 scenarios. Tests only with a text editor that is not XSS-safe. + */ + function testInitialSecurity() { + $expected = array( + array( + 'node_id' => 1, + 'format' => 'restricted_without_editor', + // No text editor => no XSS filtering. + 'value' => self::$sampleContent, + 'users' => array( + $this->untrusted_user, + $this->privileged_user, + ), + ), + array( + 'node_id' => 2, + 'format' => 'restricted_with_editor', + // Text editor => XSS filtering. + 'value' => self::$sampleContentSecured, + 'users' => array( + $this->normal_user, + $this->privileged_user, + ), + ), + array( + 'node_id' => 3, + 'format' => 'restricted_plus_dangerous_tag_with_editor', + // Text editor => XSS filtering. + 'value' => self::$sampleContentSecuredEmbedAllowed, + 'users' => array( + $this->trusted_user, + $this->privileged_user, + ), + ), + array( + 'node_id' => 4, + 'format' => 'unrestricted_without_editor', + // No text editor => no XSS filtering. + 'value' => self::$sampleContent, + 'users' => array( + $this->privileged_user, + ), + ), + array( + 'node_id' => 5, + 'format' => 'unrestricted_with_editor', + // Text editor, no security filter => no XSS filtering. + 'value' => self::$sampleContent, + 'users' => array( + $this->privileged_user, + ), + ), + ); + + // Log in as each user that may edit the content, and assert the value. + foreach ($expected as $case) { + foreach ($case['users'] as $account) { + $this->pass(format_string('Scenario: sample %sample_id, %format.', array( + '%sample_id' => $case['node_id'], + '%format' => $case['format'], + ))); + $this->drupalLogin($account); + $this->drupalGet('node/' . $case['node_id'] . '/edit'); + $dom_node = $this->xpath('//textarea[@id="edit-body-0-value"]'); + $this->assertIdentical($case['value'], (string) $dom_node[0], 'The value was correctly filtered for XSS attack vectors.'); + } + } + } + + /** + * Tests administrator security: is the user safe when switching text formats? + * + * Tests 24 scenarios. Tests only with a text editor that is not XSS-safe. + * + * When changing from a more restrictive text format with a text editor (or a + * text format without a text editor) to a less restrictive text format, it is + * possible that a malicious user could trigger an XSS. + * + * E.g. when switching a piece of text that uses the Restricted HTML text + * format and contains a <script> tag to the Full HTML text format, the + * <script> tag would be executed. Unless we apply appropriate filtering. + */ + function testSwitchingSecurity() { + $expected = array( + array( + 'node_id' => 1, + 'value' => self::$sampleContent, // No text editor => no XSS filtering. + 'format' => 'restricted_without_editor', + 'switch_to' => array( + 'restricted_with_editor' => self::$sampleContentSecured, + // Intersection of restrictions => most strict XSS filtering. + 'restricted_plus_dangerous_tag_with_editor' => self::$sampleContentSecured, + // No text editor => no XSS filtering. + 'unrestricted_without_editor' => FALSE, + 'unrestricted_with_editor' => self::$sampleContentSecured, + ), + ), + array( + 'node_id' => 2, + 'value' => self::$sampleContentSecured, // Text editor => XSS filtering. + 'format' => 'restricted_with_editor', + 'switch_to' => array( + // No text editor => no XSS filtering. + 'restricted_without_editor' => FALSE, + // Intersection of restrictions => most strict XSS filtering. + 'restricted_plus_dangerous_tag_with_editor' => self::$sampleContentSecured, + // No text editor => no XSS filtering. + 'unrestricted_without_editor' => FALSE, + 'unrestricted_with_editor' => self::$sampleContentSecured, + ), + ), + array( + 'node_id' => 3, + 'value' => self::$sampleContentSecuredEmbedAllowed, // Text editor => XSS filtering. + 'format' => 'restricted_plus_dangerous_tag_with_editor', + 'switch_to' => array( + // No text editor => no XSS filtering. + 'restricted_without_editor' => FALSE, + // Intersection of restrictions => most strict XSS filtering. + 'restricted_with_editor' => self::$sampleContentSecured, + // No text editor => no XSS filtering. + 'unrestricted_without_editor' => FALSE, + // Intersection of restrictions => most strict XSS filtering. + 'unrestricted_with_editor' => self::$sampleContentSecured, + ), + ), + array( + 'node_id' => 4, + 'value' => self::$sampleContent, // No text editor => no XSS filtering. + 'format' => 'unrestricted_without_editor', + 'switch_to' => array( + // No text editor => no XSS filtering. + 'restricted_without_editor' => FALSE, + 'restricted_with_editor' => self::$sampleContentSecured, + // Intersection of restrictions => most strict XSS filtering. + 'restricted_plus_dangerous_tag_with_editor' => self::$sampleContentSecured, + // From no editor, no security filters, to editor, still no security + // filters: resulting content when viewed was already vulnerable, so + // it must be intentional. + 'unrestricted_with_editor' => FALSE, + ), + ), + array( + 'node_id' => 5, + 'value' => self::$sampleContentSecured, // Text editor => XSS filtering. + 'format' => 'unrestricted_with_editor', + 'switch_to' => array( + // From editor, no security filters to security filters, no editor: no + // risk. + 'restricted_without_editor' => FALSE, + 'restricted_with_editor' => self::$sampleContentSecured, + // Intersection of restrictions => most strict XSS filtering. + 'restricted_plus_dangerous_tag_with_editor' => self::$sampleContentSecured, + // From no editor, no security filters, to editor, still no security + // filters: resulting content when viewed was already vulnerable, so + // it must be intentional. + 'unrestricted_without_editor' => FALSE, + ), + ), + ); + + // Log in as the privileged user, and for every sample, do the following: + // - switch to every other text format/editor + // - assert the XSS-filtered values that we get from the server + $value_original_attribute = String::checkPlain(self::$sampleContent); + $this->drupalLogin($this->privileged_user); + foreach ($expected as $case) { + $this->drupalGet('node/' . $case['node_id'] . '/edit'); + + // Verify data- attributes. + $dom_node = $this->xpath('//textarea[@id="edit-body-0-value"]'); + $this->assertIdentical(self::$sampleContent, (string) $dom_node[0]['data-editor-value-original'], 'The data-editor-value-original attribute is correctly set.'); + $this->assertIdentical('false', (string) $dom_node[0]['data-editor-value-is-changed'], 'The data-editor-value-is-changed attribute is correctly set.'); + + // Switch to every other text format/editor and verify the results. + foreach ($case['switch_to'] as $format => $expected_filtered_value) { + $this->pass(format_string('Scenario: sample %sample_id, switch from %original_format to %format.', array( + '%sample_id' => $case['node_id'], + '%original_format' => $case['format'], + '%format' => $format, + ))); + $post = array( + 'value' => self::$sampleContent, + 'original_format_id' => $case['format'], + ); + $response = $this->drupalPost('editor/filter_xss/' . $format, 'application/json', $post); + $this->assertResponse(200); + $json = drupal_json_decode($response); + $this->assertIdentical($json, $expected_filtered_value, 'The value was correctly filtered for XSS attack vectors.'); + } + } + } + + /** + * Tests the standard text editor XSS filter being overridden. + */ + function testEditorXssFilterOverride() { + // First: the Standard text editor XSS filter. + $this->drupalLogin($this->normal_user); + $this->drupalGet('node/2/edit'); + $dom_node = $this->xpath('//textarea[@id="edit-body-0-value"]'); + $this->assertIdentical(self::$sampleContentSecured, (string) $dom_node[0], 'The value was filtered by the Standard text editor XSS filter.'); + + // Enable editor_test.module's hook_editor_xss_filter_alter() implementation + // to ater the text editor XSS filter class being used. + \Drupal::state()->set('editor_test_editor_xss_filter_alter_enabled', TRUE); + + // First: the Insecure text editor XSS filter. + $this->drupalGet('node/2/edit'); + $dom_node = $this->xpath('//textarea[@id="edit-body-0-value"]'); + $this->assertIdentical(self::$sampleContent, (string) $dom_node[0], 'The value was filtered by the Insecure text editor XSS filter.'); + } +} diff --git a/core/modules/editor/tests/Drupal/editor/Tests/EditorXssFilter/StandardTest.php b/core/modules/editor/tests/Drupal/editor/Tests/EditorXssFilter/StandardTest.php new file mode 100644 index 000000000000..1557bbcbee83 --- /dev/null +++ b/core/modules/editor/tests/Drupal/editor/Tests/EditorXssFilter/StandardTest.php @@ -0,0 +1,553 @@ +<?php + +/** + * @file + * Contains \Drupal\editor\Tests\editor\EditorXssFilter\StandardTest. + */ + +namespace Drupal\editor\Tests\editor\EditorXssFilter; + +use Drupal\Tests\UnitTestCase; +use Drupal\filter\Plugin\FilterInterface; + +/** + * Tests the standard text editor XSS filter. + * + * @group Drupal + * @group Editor + * + * @see \Drupal\editor\EditorXssFilter\Standard + */ +class StandardTest extends UnitTestCase { + + /** + * The tested text editor XSS filter. + * + * @var \Drupal\editor\EditorXssFilter\Standard + */ + protected $editorXssFilterClass; + + /** + * The mocked text format configuration entity. + * + * @var \Drupal\filter\Entity\FilterFormat|\PHPUnit_Framework_MockObject_MockObject + */ + protected $format; + + public static function getInfo() { + return array( + 'name' => 'Standard text editor XSS filter test', + 'description' => 'Unit test of standard text editor XSS filter.', + 'group' => 'Text Editor' + ); + } + + protected function setUp() { + $this->editorXssFilterClass = '\Drupal\editor\EditorXssFilter\Standard'; + + // Mock text format configuration entity object. + $this->format = $this->getMockBuilder('\Drupal\filter\Entity\FilterFormat') + ->disableOriginalConstructor() + ->getMock(); + $this->format->expects($this->any()) + ->method('getFilterTypes') + ->will($this->returnValue(array(FilterInterface::TYPE_HTML_RESTRICTOR))); + $restrictions = array( + 'allowed' => array( + 'p' => TRUE, + 'a' => TRUE, + '*' => array( + 'style' => FALSE, + 'on*' => FALSE, + ), + ), + ); + $this->format->expects($this->any()) + ->method('getHtmlRestrictions') + ->will($this->returnValue($restrictions)); + } + + /** + * Provides test data for testFilterXss(). + * + * \Drupal\editor\EditorXssFilter\Standard uses + * Drupal\Component\Utility\Xss. See \Drupal\Tests\Component\Utility\XssTest::testBlacklistMode() + * for more detailed test coverage. + * + * @see \Drupal\editor\Tests\editor\EditorXssFilter\StandardTest::testFilterXss() + * @see \Drupal\Tests\Component\Utility\XssTest::testBlacklistMode() + */ + public function providerTestFilterXss() { + $data = array(); + $data[] = array('<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown>', '<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown>'); + $data[] = array('<p style="color:red">Hello, world!</p><unknown>Pink Fairy Armadillo</unknown>', '<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown>'); + $data[] = array('<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown><script>alert("evil");</script>', '<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown>alert("evil");'); + $data[] = array('<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown><a href="javascript:alert(1)">test</a>', '<p>Hello, world!</p><unknown>Pink Fairy Armadillo</unknown><a href="alert(1)">test</a>'); + + // All cases listed on https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet + + // No Filter Evasion. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#No_Filter_Evasion + $data[] = array('<SCRIPT SRC=http://ha.ckers.org/xss.js></SCRIPT>', ''); + + // Image XSS using the JavaScript directive. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Image_XSS_using_the_JavaScript_directive + $data[] = array('<IMG SRC="javascript:alert(\'XSS\');">', '<IMG src="alert('XSS');">'); + + // No quotes and no semicolon. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#No_quotes_and_no_semicolon + $data[] = array('<IMG SRC=javascript:alert(\'XSS\')>', '<IMG>'); + + // Case insensitive XSS attack vector. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Case_insensitive_XSS_attack_vector + $data[] = array('<IMG SRC=JaVaScRiPt:alert(\'XSS\')>', '<IMG>'); + + // HTML entities. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#HTML_entities + $data[] = array('<IMG SRC=javascript:alert("XSS")>', '<IMG>'); + + // Grave accent obfuscation. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Grave_accent_obfuscation + $data[] = array('<IMG SRC=`javascript:alert("RSnake says, \'XSS\'")`>', '<IMG>'); + + // Malformed A tags. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Malformed_A_tags + $data[] = array('<a onmouseover="alert(document.cookie)">xxs link</a>', '<a>xxs link</a>'); + $data[] = array('<a onmouseover=alert(document.cookie)>xxs link</a>', '<a>xxs link</a>'); + + // Malformed IMG tags. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Malformed_IMG_tags + $data[] = array('<IMG """><SCRIPT>alert("XSS")</SCRIPT>">', '<IMG>alert("XSS")">'); + + // fromCharCode. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#fromCharCode + $data[] = array('<IMG SRC=javascript:alert(String.fromCharCode(88,83,83))>', '<IMG src="alert(String.fromCharCode(88,83,83))">'); + + // Default SRC tag to get past filters that check SRC domain. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Default_SRC_tag_to_get_past_filters_that_check_SRC_domain + $data[] = array('<IMG SRC=# onmouseover="alert(\'xxs\')">', '<IMG src="#">'); + + // Default SRC tag by leaving it empty. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Default_SRC_tag_by_leaving_it_empty + $data[] = array('<IMG SRC= onmouseover="alert(\'xxs\')">', '<IMG nmouseover="alert('xxs')">'); + + // Default SRC tag by leaving it out entirely. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Default_SRC_tag_by_leaving_it_out_entirely + $data[] = array('<IMG onmouseover="alert(\'xxs\')">', '<IMG>'); + + // Decimal HTML character references. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Decimal_HTML_character_references + $data[] = array('<IMG SRC=javascript:alert('XSS')>', '<IMG src="alert('XSS')">'); + + // Decimal HTML character references without trailing semicolons. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Decimal_HTML_character_references_without_trailing_semicolons + $data[] = array('<IMG SRC=javascript:alert('XSS')>', '<IMG src="&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041">'); + + // Hexadecimal HTML character references without trailing semicolons. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Hexadecimal_HTML_character_references_without_trailing_semicolons + $data[] = array('<IMG SRC=javascript:alert('XSS')>', '<IMG src="&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29">'); + + // Embedded tab. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Embedded_tab + $data[] = array('<IMG SRC="jav ascript:alert(\'XSS\');">', '<IMG src="alert('XSS');">'); + + // Embedded Encoded tab. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Embedded_Encoded_tab + $data[] = array('<IMG SRC="jav	ascript:alert(\'XSS\');">', '<IMG src="alert('XSS');">'); + + // Embedded newline to break up XSS. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Embedded_newline_to_break_up_XSS + $data[] = array('<IMG SRC="jav
ascript:alert(\'XSS\');">', '<IMG src="alert('XSS');">'); + + // Embedded carriage return to break up XSS. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Embedded_carriage_return_to_break_up_XSS + $data[] = array('<IMG SRC="jav
ascript:alert(\'XSS\');">', '<IMG src="alert('XSS');">'); + + // Null breaks up JavaScript directive. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Null_breaks_up_JavaScript_directive + $data[] = array("<IMG SRC=java\0script:alert(\"XSS\")>", '<IMG>'); + + // Spaces and meta chars before the JavaScript in images for XSS. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Spaces_and_meta_chars_before_the_JavaScript_in_images_for_XSS + $data[] = array('<IMG SRC="  javascript:alert(\'XSS\');">', '<IMG src="alert('XSS');">'); + + // Non-alpha-non-digit XSS. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Non-alpha-non-digit_XSS + $data[] = array('<SCRIPT/XSS SRC="http://ha.ckers.org/xss.js"></SCRIPT>', ''); + $data[] = array('<BODY onload!#$%&()*~+-_.,:;?@[/|\]^`=alert("XSS")>', '<BODY>'); + $data[] = array('<SCRIPT/SRC="http://ha.ckers.org/xss.js"></SCRIPT>', ''); + + // Extraneous open brackets. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Extraneous_open_brackets + $data[] = array('<<SCRIPT>alert("XSS");//<</SCRIPT>', '<alert("XSS");//<'); + + // No closing script tags. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#No_closing_script_tags + $data[] = array('<SCRIPT SRC=http://ha.ckers.org/xss.js?< B >', ''); + + // Protocol resolution in script tags. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Protocol_resolution_in_script_tags + $data[] = array('<SCRIPT SRC=//ha.ckers.org/.j>', ''); + + // Half open HTML/JavaScript XSS vector. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Half_open_HTML.2FJavaScript_XSS_vector + $data[] = array('<IMG SRC="javascript:alert(\'XSS\')"', '<IMG src="alert('XSS')">'); + + // Double open angle brackets. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Double_open_angle_brackets + // @see http://ha.ckers.org/blog/20060611/hotbot-xss-vulnerability/ to + // understand why this is a vulnerability. + $data[] = array('<iframe src=http://ha.ckers.org/scriptlet.html <', '<iframe src="http://ha.ckers.org/scriptlet.html">'); + + // Escaping JavaScript escapes. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Escaping_JavaScript_escapes + // This one is irrelevent for Drupal; we *never* output any JavaScript code + // that depends on the URL's query string. + + // End title tag. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#End_title_tag + $data[] = array('</TITLE><SCRIPT>alert("XSS");</SCRIPT>', '</TITLE>alert("XSS");'); + + // INPUT image. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#INPUT_image + $data[] = array('<INPUT TYPE="IMAGE" SRC="javascript:alert(\'XSS\');">', '<INPUT type="IMAGE" src="alert('XSS');">'); + + // BODY image. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#BODY_image + $data[] = array('<BODY BACKGROUND="javascript:alert(\'XSS\')">', '<BODY background="alert('XSS')">'); + + // IMG Dynsrc. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#IMG_Dynsrc + $data[] = array('<IMG DYNSRC="javascript:alert(\'XSS\')">', '<IMG dynsrc="alert('XSS')">'); + + // IMG lowrsc. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#IMG_lowsrc + $data[] = array('<IMG LOWSRC="javascript:alert(\'XSS\')">', '<IMG lowsrc="alert('XSS')">'); + + // List-style-image. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#List-style-image + $data[] = array('<STYLE>li {list-style-image: url("javascript:alert(\'XSS\')");}</STYLE><UL><LI>XSS</br>', 'li {list-style-image: url("javascript:alert(\'XSS\')");}<UL><LI>XSS</br>'); + + // VBscript in an image. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#VBscript_in_an_image + $data[] = array('<IMG SRC=\'vbscript:msgbox("XSS")\'>', '<IMG src=\'msgbox("XSS")\'>'); + + // Livescript (older versions of Netscape only). + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Livescript_.28older_versions_of_Netscape_only.29 + $data[] = array('<IMG SRC="livescript:[code]">', '<IMG src="[code]">'); + + // BODY tag. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#BODY_tag + $data[] = array('<BODY ONLOAD=alert(\'XSS\')>', '<BODY>'); + + // Event handlers. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Event_Handlers + $events = array( + 'onAbort', + 'onActivate', + 'onAfterPrint', + 'onAfterUpdate', + 'onBeforeActivate', + 'onBeforeCopy', + 'onBeforeCut', + 'onBeforeDeactivate', + 'onBeforeEditFocus', + 'onBeforePaste', + 'onBeforePrint', + 'onBeforeUnload', + 'onBeforeUpdate', + 'onBegin', + 'onBlur', + 'onBounce', + 'onCellChange', + 'onChange', + 'onClick', + 'onContextMenu', + 'onControlSelect', + 'onCopy', + 'onCut', + 'onDataAvailable', + 'onDataSetChanged', + 'onDataSetComplete', + 'onDblClick', + 'onDeactivate', + 'onDrag', + 'onDragEnd', + 'onDragLeave', + 'onDragEnter', + 'onDragOver', + 'onDragDrop', + 'onDragStart', + 'onDrop', + 'onEnd', + 'onError', + 'onErrorUpdate', + 'onFilterChange', + 'onFinish', + 'onFocus', + 'onFocusIn', + 'onFocusOut', + 'onHashChange', + 'onHelp', + 'onInput', + 'onKeyDown', + 'onKeyPress', + 'onKeyUp', + 'onLayoutComplete', + 'onLoad', + 'onLoseCapture', + 'onMediaComplete', + 'onMediaError', + 'onMessage', + 'onMousedown', + 'onMouseEnter', + 'onMouseLeave', + 'onMouseMove', + 'onMouseOut', + 'onMouseOver', + 'onMouseUp', + 'onMouseWheel', + 'onMove', + 'onMoveEnd', + 'onMoveStart', + 'onOffline', + 'onOnline', + 'onOutOfSync', + 'onPaste', + 'onPause', + 'onPopState', + 'onProgress', + 'onPropertyChange', + 'onReadyStateChange', + 'onRedo', + 'onRepeat', + 'onReset', + 'onResize', + 'onResizeEnd', + 'onResizeStart', + 'onResume', + 'onReverse', + 'onRowsEnter', + 'onRowExit', + 'onRowDelete', + 'onRowInserted', + 'onScroll', + 'onSeek', + 'onSelect', + 'onSelectionChange', + 'onSelectStart', + 'onStart', + 'onStop', + 'onStorage', + 'onSyncRestored', + 'onSubmit', + 'onTimeError', + 'onTrackChange', + 'onUndo', + 'onUnload', + 'onURLFlip', + ); + foreach ($events as $event) { + $data[] = array('<p ' . $event . '="javascript:alert(\'XSS\');">Dangerous llama!</p>', '<p>Dangerous llama!</p>'); + } + + // BGSOUND. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#BGSOUND + $data[] = array('<BGSOUND SRC="javascript:alert(\'XSS\');">', '<BGSOUND src="alert('XSS');">'); + + // & JavaScript includes. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#.26_JavaScript_includes + $data[] = array('<BR SIZE="&{alert(\'XSS\')}">', '<BR size="">'); + + // STYLE sheet. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#STYLE_sheet + $data[] = array('<LINK REL="stylesheet" HREF="javascript:alert(\'XSS\');">', ''); + + // Remote style sheet. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Remote_style_sheet + $data[] = array('<LINK REL="stylesheet" HREF="http://ha.ckers.org/xss.css">', ''); + + // Remote style sheet part 2. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Remote_style_sheet_part_2 + $data[] = array('<STYLE>@import\'http://ha.ckers.org/xss.css\';</STYLE>', '@import\'http://ha.ckers.org/xss.css\';'); + + // Remote style sheet part 3. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Remote_style_sheet_part_3 + $data[] = array('<META HTTP-EQUIV="Link" Content="<http://ha.ckers.org/xss.css>; REL=stylesheet">', '<META http-equiv="Link">; REL=stylesheet">'); + + // Remote style sheet part 4. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Remote_style_sheet_part_4 + $data[] = array('<STYLE>BODY{-moz-binding:url("http://ha.ckers.org/xssmoz.xml#xss")}</STYLE>', 'BODY{-moz-binding:url("http://ha.ckers.org/xssmoz.xml#xss")}'); + + // STYLE tags with broken up JavaScript for XSS. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#STYLE_tags_with_broken_up_JavaScript_for_XSS + $data[] = array('<STYLE>@im\port\'\ja\vasc\ript:alert("XSS")\';</STYLE>', '@im\port\'\ja\vasc\ript:alert("XSS")\';'); + + // STYLE attribute using a comment to break up expression. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#STYLE_attribute_using_a_comment_to_break_up_expression + $data[] = array('<IMG STYLE="xss:expr/*XSS*/ession(alert(\'XSS\'))">', '<IMG>'); + + // IMG STYLE with expression. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#IMG_STYLE_with_expression + $data[] = array('exp/*<A STYLE=\'no\xss:noxss("*//*"); +xss:ex/*XSS*//*/*/pression(alert("XSS"))\'>', 'exp/*<A>'); + + // STYLE tag (Older versions of Netscape only). + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#STYLE_tag_.28Older_versions_of_Netscape_only.29 + $data[] = array('<STYLE TYPE="text/javascript">alert(\'XSS\');</STYLE>', 'alert(\'XSS\');'); + + // STYLE tag using background-image. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#STYLE_tag_using_background-image + $data[] = array('<STYLE>.XSS{background-image:url("javascript:alert(\'XSS\')");}</STYLE><A CLASS=XSS></A>', '.XSS{background-image:url("javascript:alert(\'XSS\')");}<A class="XSS"></A>'); + + // STYLE tag using background. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#STYLE_tag_using_background + $data[] = array('<STYLE type="text/css">BODY{background:url("javascript:alert(\'XSS\')")}</STYLE>', 'BODY{background:url("javascript:alert(\'XSS\')")}'); + + // Anonymous HTML with STYLE attribute. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Anonymous_HTML_with_STYLE_attribute + $data[] = array('<XSS STYLE="xss:expression(alert(\'XSS\'))">', '<XSS>'); + + // Local htc file. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Local_htc_file + $data[] = array('<XSS STYLE="behavior: url(xss.htc);">', '<XSS>'); + + // US-ASCII encoding. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#US-ASCII_encoding + // This one is irrelevant for Drupal; Drupal *always* outputs UTF-8. + + // META. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#META + $data[] = array('<META HTTP-EQUIV="refresh" CONTENT="0;url=javascript:alert(\'XSS\');">', '<META http-equiv="refresh" content="alert('XSS');">'); + + // META using data. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#META_using_data + $data[] = array('<META HTTP-EQUIV="refresh" CONTENT="0;url=data:text/html base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K">', '<META http-equiv="refresh" content="text/html base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K">'); + + // META with additional URL parameter + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#META + $data[] = array('<META HTTP-EQUIV="refresh" CONTENT="0; URL=http://;URL=javascript:alert(\'XSS\');">', '<META http-equiv="refresh" content="//;URL=javascript:alert('XSS');">'); + + // IFRAME. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#IFRAME + $data[] = array('<IFRAME SRC="javascript:alert(\'XSS\');"></IFRAME>', '<IFRAME src="alert('XSS');"></IFRAME>'); + + // IFRAME Event based. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#IFRAME_Event_based + $data[] = array('<IFRAME SRC=# onmouseover="alert(document.cookie)"></IFRAME>', '<IFRAME src="#"></IFRAME>'); + + // FRAME. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#FRAME + $data[] = array('<FRAMESET><FRAME SRC="javascript:alert(\'XSS\');"></FRAMESET>', '<FRAMESET><FRAME src="alert('XSS');"></FRAMESET>'); + + // TABLE. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#TABLE + $data[] = array('<TABLE BACKGROUND="javascript:alert(\'XSS\')">', '<TABLE background="alert('XSS')">'); + + // TD. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#TD + $data[] = array('<TABLE><TD BACKGROUND="javascript:alert(\'XSS\')">', '<TABLE><TD background="alert('XSS')">'); + + // DIV background-image. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#DIV_background-image + $data[] = array('<DIV STYLE="background-image: url(javascript:alert(\'XSS\'))">', '<DIV>'); + + // DIV background-image with unicoded XSS exploit. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#DIV_background-image_with_unicoded_XSS_exploit + $data[] = array('<DIV STYLE="background-image:\0075\0072\006C\0028\'\006a\0061\0076\0061\0073\0063\0072\0069\0070\0074\003a\0061\006c\0065\0072\0074\0028.1027\0058.1053\0053\0027\0029\'\0029">', '<DIV>'); + + // DIV background-image plus extra characters. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#DIV_background-image_plus_extra_characters + $data[] = array('<DIV STYLE="background-image: url(javascript:alert(\'XSS\'))">', '<DIV>'); + + // DIV expression. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#DIV_expression + $data[] = array('<DIV STYLE="width: expression(alert(\'XSS\'));">', '<DIV>'); + + // Downlevel-Hidden block. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Downlevel-Hidden_block + $data[] = array('<!--[if gte IE 4]> + <SCRIPT>alert(\'XSS\');</SCRIPT> + <![endif]-->', "\n alert('XSS');\n "); + + // BASE tag. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#BASE_tag + $data[] = array('<BASE HREF="javascript:alert(\'XSS\');//">', '<BASE href="alert('XSS');//">'); + + // OBJECT tag. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#OBJECT_tag + $data[] = array('<OBJECT TYPE="text/x-scriptlet" DATA="http://ha.ckers.org/scriptlet.html"></OBJECT>', ''); + + // Using an EMBED tag you can embed a Flash movie that contains XSS. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Using_an_EMBED_tag_you_can_embed_a_Flash_movie_that_contains_XSS + $data[] = array('<EMBED SRC="http://ha.ckers.org/xss.swf" AllowScriptAccess="always"></EMBED>', ''); + + // You can EMBED SVG which can contain your XSS vector. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#You_can_EMBED_SVG_which_can_contain_your_XSS_vector + $data[] = array('<EMBED SRC="data:image/svg+xml;base64,PHN2ZyB4bWxuczpzdmc9Imh0dH A6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcv MjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hs aW5rIiB2ZXJzaW9uPSIxLjAiIHg9IjAiIHk9IjAiIHdpZHRoPSIxOTQiIGhlaWdodD0iMjAw IiBpZD0ieHNzIj48c2NyaXB0IHR5cGU9InRleHQvZWNtYXNjcmlwdCI+YWxlcnQoIlh TUyIpOzwvc2NyaXB0Pjwvc3ZnPg==" type="image/svg+xml" AllowScriptAccess="always"></EMBED>', ''); + + // XML data island with CDATA obfuscation. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#XML_data_island_with_CDATA_obfuscation + $data[] = array('<XML ID="xss"><I><B><IMG SRC="javas<!-- -->cript:alert(\'XSS\')"></B></I></XML><SPAN DATASRC="#xss" DATAFLD="B" DATAFORMATAS="HTML"></SPAN>', '<XML id="xss"><I><B><IMG>cript:alert(\'XSS\')"></B></I></XML><SPAN datasrc="#xss" datafld="B" dataformatas="HTML"></SPAN>'); + + // Locally hosted XML with embedded JavaScript that is generated using an XML data island. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Locally_hosted_XML_with_embedded_JavaScript_that_is_generated_using_an_XML_data_island + // This one is irrelevant for Drupal; Drupal disallows XML uploads by + // default. + + // HTML+TIME in XML. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#HTML.2BTIME_in_XML + $data[] = array('<?xml:namespace prefix="t" ns="urn:schemas-microsoft-com:time"><?import namespace="t" implementation="#default#time2"><t:set attributeName="innerHTML" to="XSS<SCRIPT DEFER>alert("XSS")</SCRIPT>">', '<?xml:namespace prefix="t" ns="urn:schemas-microsoft-com:time"><?import namespace="t" implementation="#default#time2"><t set attributename="innerHTML">alert("XSS")">'); + + // Assuming you can only fit in a few characters and it filters against ".js". + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Assuming_you_can_only_fit_in_a_few_characters_and_it_filters_against_.22.js.22 + $data[] = array('<SCRIPT SRC="http://ha.ckers.org/xss.jpg"></SCRIPT>', ''); + + // IMG Embedded commands. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#IMG_Embedded_commands + // This one is irrelevant for Drupal; this is actually a CSRF, for which + // Drupal has CSRF protection, see https://drupal.org/node/178896. + + // Cookie manipulation. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Cookie_manipulation + $data[] = array('<META HTTP-EQUIV="Set-Cookie" Content="USERID=<SCRIPT>alert(\'XSS\')</SCRIPT>">', '<META http-equiv="Set-Cookie">alert(\'XSS\')">'); + + // UTF-7 encoding. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#UTF-7_encoding + // This one is irrelevant for Drupal; Drupal *always* outputs UTF-8. + + // XSS using HTML quote encapsulation. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#XSS_using_HTML_quote_encapsulation + $data[] = array('<SCRIPT a=">" SRC="http://ha.ckers.org/xss.js"></SCRIPT>', '" SRC="http://ha.ckers.org/xss.js">'); + $data[] = array('<SCRIPT =">" SRC="http://ha.ckers.org/xss.js"></SCRIPT>', '" SRC="http://ha.ckers.org/xss.js">'); + $data[] = array('<SCRIPT a=">" \'\' SRC="http://ha.ckers.org/xss.js"></SCRIPT>', '" \'\' SRC="http://ha.ckers.org/xss.js">'); + $data[] = array('<SCRIPT "a=\'>\'" SRC="http://ha.ckers.org/xss.js"></SCRIPT>', '\'" SRC="http://ha.ckers.org/xss.js">'); + $data[] = array('<SCRIPT a=`>` SRC="http://ha.ckers.org/xss.js"></SCRIPT>', '` SRC="http://ha.ckers.org/xss.js">'); + $data[] = array('<SCRIPT a=">\'>" SRC="http://ha.ckers.org/xss.js"></SCRIPT>', '\'>" SRC="http://ha.ckers.org/xss.js">'); + $data[] = array('<SCRIPT>document.write("<SCRI");</SCRIPT>PT SRC="http://ha.ckers.org/xss.js"></SCRIPT>', 'document.write("<SCRI>PT SRC="http://ha.ckers.org/xss.js">'); + + // URL string evasion. + // @see https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#URL_string_evasion + // This one is irrelevant for Drupal; Drupal doesn't forbid linking to some + // sites, it only forbids linking to any protocols other than those that are + // whitelisted. + + return $data; + } + + /** + * Tests the method for checking access to routes. + * + * @param string $input + * The input. + * @param string $expected_output + * The expected output. + * + * @dataProvider providerTestFilterXss + */ + public function testFilterXss($input, $expected_output) { + $output = call_user_func($this->editorXssFilterClass . '::filterXss', $input, $this->format); + $this->assertSame($expected_output, $output); + } + +} diff --git a/core/modules/editor/tests/modules/editor_test.module b/core/modules/editor/tests/modules/editor_test.module index fc2d748c665e..4feffa1ca774 100644 --- a/core/modules/editor/tests/modules/editor_test.module +++ b/core/modules/editor/tests/modules/editor_test.module @@ -5,6 +5,8 @@ * Helper module for the Text Editor tests. */ +use Drupal\filter\FilterFormatInterface; + /** * Implements hook_editor_default_settings(). */ @@ -39,3 +41,18 @@ function editor_test_editor_js_settings_alter(&$settings) { $settings['full_html']['editorSettings']['ponyModeEnabled'] = FALSE; } } + +/** + * Implements hook_editor_xss_filter_alter(). + */ +function editor_test_editor_xss_filter_alter(&$editor_xss_filter_class, FilterFormatInterface $format, FilterFormatInterface $original_format = NULL) { + // Allow tests to enable or disable this alter hook. + if (!\Drupal::state()->get('editor_test_editor_xss_filter_alter_enabled', FALSE)) { + return; + } + + $filters = $format->filters()->getAll(); + if (isset($filters['filter_html']) && $filters['filter_html']->status) { + $editor_xss_filter_class = '\Drupal\editor_test\EditorXssFilter\Insecure'; + } +} diff --git a/core/modules/editor/tests/modules/lib/Drupal/editor_test/EditorXssFilter/Insecure.php b/core/modules/editor/tests/modules/lib/Drupal/editor_test/EditorXssFilter/Insecure.php new file mode 100644 index 000000000000..8f97d27c087c --- /dev/null +++ b/core/modules/editor/tests/modules/lib/Drupal/editor_test/EditorXssFilter/Insecure.php @@ -0,0 +1,26 @@ +<?php + +/** + * @file + * Contains \Drupal\editor_test\EditorXssFilter\Insecure. + */ + +namespace Drupal\editor_test\EditorXssFilter; + +use Drupal\filter\FilterFormatInterface; +use Drupal\editor\EditorXssFilterInterface; + +/** + * Defines an insecure text editor XSS filter (for testing purposes). + */ +class Insecure implements EditorXssFilterInterface { + + /** + * {@inheritdoc} + */ + public static function filterXss($html, FilterFormatInterface $format, FilterFormatInterface $original_format = NULL) { + // Don't apply any XSS filtering, just return the string we received. + return $html; + } + +} diff --git a/core/modules/editor/tests/modules/lib/Drupal/editor_test/Plugin/Editor/UnicornEditor.php b/core/modules/editor/tests/modules/lib/Drupal/editor_test/Plugin/Editor/UnicornEditor.php index e6b34cd91318..53d565c5ff3a 100644 --- a/core/modules/editor/tests/modules/lib/Drupal/editor_test/Plugin/Editor/UnicornEditor.php +++ b/core/modules/editor/tests/modules/lib/Drupal/editor_test/Plugin/Editor/UnicornEditor.php @@ -11,13 +11,14 @@ use Drupal\editor\Entity\Editor as EditorEntity; /** - * Defines a Unicorn-powered text editor for Drupal. + * Defines a Unicorn-powered text editor for Drupal (for testing purposes). * * @Editor( * id = "unicorn", * label = @Translation("Unicorn Editor"), * supports_content_filtering = TRUE, - * supports_inline_editing = TRUE + * supports_inline_editing = TRUE, + * is_xss_safe = FALSE * ) */ class UnicornEditor extends EditorBase { diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index 710c59fdb711..a18c820cfd75 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -11,34 +11,7 @@ use Drupal\Core\Session\AccountInterface; use Drupal\Core\Template\Attribute; use Drupal\filter\FilterFormatInterface; - -/** - * Non-HTML markup language filters that generate HTML. - * - * @todo Move into \Drupal\filter\Plugin\Filter\FilterInterface - */ -const FILTER_TYPE_MARKUP_LANGUAGE = 0; - -/** - * HTML tag and attribute restricting filters. - * - * @todo Move into \Drupal\filter\Plugin\Filter\FilterInterface - */ -const FILTER_TYPE_HTML_RESTRICTOR = 1; - -/** - * Reversible transformation filters. - * - * @todo Move into \Drupal\filter\Plugin\Filter\FilterInterface - */ -const FILTER_TYPE_TRANSFORM_REVERSIBLE = 2; - -/** - * Irreversible transformation filters. - * - * @todo Move into \Drupal\filter\Plugin\Filter\FilterInterface - */ -const FILTER_TYPE_TRANSFORM_IRREVERSIBLE = 3; +use Drupal\filter\Plugin\FilterInterface; /** * Implements hook_help(). @@ -412,7 +385,7 @@ function filter_format_allowcache($format_id) { * (optional) An array of filter types to skip, or an empty array (default) * to skip no filter types. All of the format's filters will be applied, * except for filters of the types that are marked to be skipped. - * FILTER_TYPE_HTML_RESTRICTOR is the only type that cannot be skipped. + * FilterInterface::TYPE_HTML_RESTRICTOR is the only type that cannot be skipped. * * @return * The filtered text. @@ -429,9 +402,9 @@ function check_markup($text, $format_id = NULL, $langcode = '', $cache = FALSE, return ''; } - // Prevent FILTER_TYPE_HTML_RESTRICTOR from being skipped. - if (in_array(FILTER_TYPE_HTML_RESTRICTOR, $filter_types_to_skip)) { - $filter_types_to_skip = array_diff($filter_types_to_skip, array(FILTER_TYPE_HTML_RESTRICTOR)); + // Prevent FilterInterface::TYPE_HTML_RESTRICTOR from being skipped. + if (in_array(FilterInterface::TYPE_HTML_RESTRICTOR, $filter_types_to_skip)) { + $filter_types_to_skip = array_diff($filter_types_to_skip, array(FilterInterface::TYPE_HTML_RESTRICTOR)); } // When certain filters should be skipped, don't perform caching. diff --git a/core/modules/filter/lib/Drupal/filter/Entity/FilterFormat.php b/core/modules/filter/lib/Drupal/filter/Entity/FilterFormat.php index fb6292b67f92..15537acc3dbf 100644 --- a/core/modules/filter/lib/Drupal/filter/Entity/FilterFormat.php +++ b/core/modules/filter/lib/Drupal/filter/Entity/FilterFormat.php @@ -12,6 +12,7 @@ use Drupal\Core\Entity\EntityStorageControllerInterface; use Drupal\filter\FilterFormatInterface; use Drupal\filter\FilterBag; +use Drupal\filter\Plugin\FilterInterface; /** * Represents a text format. @@ -287,7 +288,7 @@ public function getHtmlRestrictions() { if (!$filter->status) { return FALSE; } - if ($filter->getType() === FILTER_TYPE_HTML_RESTRICTOR && $filter->getHTMLRestrictions() !== FALSE) { + if ($filter->getType() === FilterInterface::TYPE_HTML_RESTRICTOR && $filter->getHTMLRestrictions() !== FALSE) { return TRUE; } return FALSE; diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterAutoP.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterAutoP.php index 3795430b10c1..c9eca0a206cb 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterAutoP.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterAutoP.php @@ -15,7 +15,7 @@ * @Filter( * id = "filter_autop", * title = @Translation("Convert line breaks into HTML (i.e. <code><br></code> and <code><p></code>)"), - * type = FILTER_TYPE_MARKUP_LANGUAGE + * type = Drupal\filter\Plugin\FilterInterface::TYPE_MARKUP_LANGUAGE * ) */ class FilterAutoP extends FilterBase { diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterCaption.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterCaption.php index 004cd658e2ba..5b5ce632edcc 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterCaption.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterCaption.php @@ -19,7 +19,7 @@ * id = "filter_caption", * title = @Translation("Display image captions and align images"), * description = @Translation("Uses data-caption and data-align attributes on <img> tags to caption and align images."), - * type = FILTER_TYPE_TRANSFORM_REVERSIBLE + * type = Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_REVERSIBLE * ) */ class FilterCaption extends FilterBase { diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php index b5b6bfea0005..a49f5f41f877 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtml.php @@ -15,7 +15,7 @@ * @Filter( * id = "filter_html", * title = @Translation("Limit allowed HTML tags"), - * type = FILTER_TYPE_HTML_RESTRICTOR, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_HTML_RESTRICTOR, * settings = { * "allowed_html" = "<a> <em> <strong> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd> <h4> <h5> <h6>", * "filter_html_help" = 1, diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlCorrector.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlCorrector.php index e19071b4b756..8d9b8e0abbbb 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlCorrector.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlCorrector.php @@ -15,7 +15,7 @@ * @Filter( * id = "filter_htmlcorrector", * title = @Translation("Correct faulty and chopped off HTML"), - * type = FILTER_TYPE_HTML_RESTRICTOR, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE, * weight = 10 * ) */ diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php index 6d195b712905..5780735476b3 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlEscape.php @@ -15,7 +15,7 @@ * @Filter( * id = "filter_html_escape", * title = @Translation("Display any HTML as plain text"), - * type = FILTER_TYPE_HTML_RESTRICTOR, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_HTML_RESTRICTOR, * weight = -10 * ) */ diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlImageSecure.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlImageSecure.php index 7a0e998512b5..9e0a0e0efce9 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlImageSecure.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterHtmlImageSecure.php @@ -16,7 +16,7 @@ * id = "filter_html_image_secure", * title = @Translation("Restrict images to this site"), * description = @Translation("Disallows usage of <img> tag sources that are not hosted on this site by replacing them with a placeholder image."), - * type = FILTER_TYPE_HTML_RESTRICTOR, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE, * weight = 9 * ) */ diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterNull.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterNull.php index 90e57eb4162b..73465d6d8396 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterNull.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterNull.php @@ -19,7 +19,7 @@ * @Filter( * id = "filter_null", * title = @Translation("Provides a fallback for missing filters. Do not use."), - * type = FILTER_TYPE_HTML_RESTRICTOR, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_HTML_RESTRICTOR, * weight = -10 * ) */ diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterUrl.php b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterUrl.php index 2398840bc873..498b32c1c61e 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterUrl.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/Filter/FilterUrl.php @@ -15,7 +15,7 @@ * @Filter( * id = "filter_url", * title = @Translation("Convert URLs into links"), - * type = FILTER_TYPE_MARKUP_LANGUAGE, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_MARKUP_LANGUAGE, * settings = { * "filter_url_length" = 72 * } diff --git a/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php b/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php index 16cdfd44d18b..f6f7dd294c00 100644 --- a/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php +++ b/core/modules/filter/lib/Drupal/filter/Plugin/FilterInterface.php @@ -2,7 +2,7 @@ /** * @file - * Contains \Drupal\filter\Plugin\Filter\FilterInterface. + * Contains \Drupal\filter\Plugin\FilterInterface. */ namespace Drupal\filter\Plugin; @@ -54,13 +54,14 @@ * - title: (required) An administrative summary of what the filter does. * - type: (required) A classification of the filter's purpose. This is one * of the following: - * - FILTER_TYPE_HTML_RESTRICTOR: HTML tag and attribute restricting + * - FilterInterface::TYPE_HTML_RESTRICTOR: HTML tag and attribute + * restricting filters. + * - FilterInterface::TYPE_MARKUP_LANGUAGE: Non-HTML markup language filters + * that generate HTML. + * - FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE: Irreversible + * transformation filters. + * - FilterInterface::TYPE_TRANSFORM_REVERSIBLE: Reversible transformation * filters. - * - FILTER_TYPE_MARKUP_LANGUAGE: Non-HTML markup language filters that - * generate HTML. - * - FILTER_TYPE_TRANSFORM_IRREVERSIBLE: Irreversible transformation - * filters. - * - FILTER_TYPE_TRANSFORM_REVERSIBLE: Reversible transformation filters. * - description: Additional administrative information about the filter's * behavior, if needed for clarification. * - status: The default status for new instances of the filter. Defaults to @@ -79,15 +80,35 @@ */ interface FilterInterface extends ConfigurablePluginInterface, PluginInspectionInterface { + /** + * Non-HTML markup language filters that generate HTML. + */ + const TYPE_MARKUP_LANGUAGE = 0; + + /** + * HTML tag and attribute restricting filters to prevent XSS attacks. + */ + const TYPE_HTML_RESTRICTOR = 1; + + /** + * Reversible transformation filters. + */ + const TYPE_TRANSFORM_REVERSIBLE = 2; + + /** + * Irreversible transformation filters. + */ + const TYPE_TRANSFORM_IRREVERSIBLE = 3; + /** * Returns the processing type of this filter plugin. * * @return int * One of: - * - FILTER_TYPE_MARKUP_LANGUAGE - * - FILTER_TYPE_HTML_RESTRICTOR - * - FILTER_TYPE_TRANSFORM_REVERSIBLE - * - FILTER_TYPE_TRANSFORM_IRREVERSIBLE + * - FilterInterface::TYPE_MARKUP_LANGUAGE + * - FilterInterface::TYPE_HTML_RESTRICTOR + * - FilterInterface::TYPE_TRANSFORM_REVERSIBLE + * - FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE */ public function getType(); @@ -162,8 +183,9 @@ public function process($text, $langcode, $cache, $cache_id); /** * Returns HTML allowed by this filter's configuration. * - * May be implemented by filters of the type FILTER_TYPE_HTML_RESTRICTOR, this - * won't be used for filters of other types; they should just return FALSE. + * May be implemented by filters of the FilterInterface::TYPE_HTML_RESTRICTOR + * type, this won't be used for filters of other types; they should just + * return FALSE. * * This callback function is only necessary for filters that strip away HTML * tags (and possibly attributes) and allows other modules to gain insight in diff --git a/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php b/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php index f9b4822d2872..c36b009247fa 100644 --- a/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php +++ b/core/modules/filter/lib/Drupal/filter/Tests/FilterAPITest.php @@ -10,6 +10,7 @@ use Drupal\Core\TypedData\AllowedValuesInterface; use Drupal\Core\TypedData\DataDefinition; use Drupal\filter\Plugin\DataType\FilterFormat; +use Drupal\filter\Plugin\FilterInterface; use Drupal\system\Tests\Entity\EntityUnitTestBase; use Symfony\Component\Validator\ConstraintViolationListInterface; @@ -38,12 +39,12 @@ function setUp() { 'format' => 'filtered_html', 'name' => 'Filtered HTML', 'filters' => array( - // Note that the filter_html filter is of the type FILTER_TYPE_MARKUP_LANGUAGE. + // Note that the filter_html filter is of the type FilterInterface::TYPE_MARKUP_LANGUAGE. 'filter_url' => array( 'weight' => -1, 'status' => 1, ), - // Note that the filter_html filter is of the type FILTER_TYPE_HTML_RESTRICTOR. + // Note that the filter_html filter is of the type FilterInterface::TYPE_HTML_RESTRICTOR. 'filter_html' => array( 'status' => 1, 'settings' => array( @@ -78,18 +79,18 @@ function testCheckMarkup() { 'Expected filter result.' ); $this->assertIdentical( - check_markup($text, 'filtered_html', '', FALSE, array(FILTER_TYPE_MARKUP_LANGUAGE)), + check_markup($text, 'filtered_html', '', FALSE, array(FilterInterface::TYPE_MARKUP_LANGUAGE)), $expected_filter_text_without_html_generators, - 'Expected filter result when skipping FILTER_TYPE_MARKUP_LANGUAGE filters.' + 'Expected filter result when skipping FilterInterface::TYPE_MARKUP_LANGUAGE filters.' ); // Related to @see FilterSecurityTest.php/testSkipSecurityFilters(), but // this check focuses on the ability to filter multiple filter types at once. // Drupal core only ships with these two types of filters, so this is the // most extensive test possible. $this->assertIdentical( - check_markup($text, 'filtered_html', '', FALSE, array(FILTER_TYPE_HTML_RESTRICTOR, FILTER_TYPE_MARKUP_LANGUAGE)), + check_markup($text, 'filtered_html', '', FALSE, array(FilterInterface::TYPE_HTML_RESTRICTOR, FilterInterface::TYPE_MARKUP_LANGUAGE)), $expected_filter_text_without_html_generators, - 'Expected filter result when skipping FILTER_TYPE_MARKUP_LANGUAGE filters, even when trying to disable filters of the FILTER_TYPE_HTML_RESTRICTOR type.' + 'Expected filter result when skipping FilterInterface::TYPE_MARKUP_LANGUAGE filters, even when trying to disable filters of the FilterInterface::TYPE_HTML_RESTRICTOR type.' ); } @@ -108,7 +109,7 @@ function testFilterFormatAPI() { ); $this->assertIdentical( $filtered_html_format->getFilterTypes(), - array(FILTER_TYPE_MARKUP_LANGUAGE, FILTER_TYPE_HTML_RESTRICTOR), + array(FilterInterface::TYPE_MARKUP_LANGUAGE, FilterInterface::TYPE_HTML_RESTRICTOR), 'FilterFormatInterface::getFilterTypes() works as expected for the filtered_html format.' ); @@ -146,12 +147,12 @@ function testFilterFormatAPI() { ); $this->assertIdentical( $stupid_filtered_html_format->getFilterTypes(), - array(FILTER_TYPE_HTML_RESTRICTOR), + array(FilterInterface::TYPE_HTML_RESTRICTOR), 'FilterFormatInterface::getFilterTypes() works as expected for the stupid_filtered_html format.' ); // Test on very_restricted_html, where there's two different filters of the - // FILTER_TYPE_HTML_RESTRICTOR type, each restricting in different ways. + // FilterInterface::TYPE_HTML_RESTRICTOR type, each restricting in different ways. $very_restricted_html_format = entity_create('filter_format', array( 'format' => 'very_restricted_html', 'name' => 'Very Restricted HTML', @@ -185,7 +186,7 @@ function testFilterFormatAPI() { ); $this->assertIdentical( $very_restricted_html_format->getFilterTypes(), - array(FILTER_TYPE_HTML_RESTRICTOR), + array(FilterInterface::TYPE_HTML_RESTRICTOR), 'FilterFormatInterface::getFilterTypes() works as expected for the very_restricted_html format.' ); } diff --git a/core/modules/filter/lib/Drupal/filter/Tests/FilterSecurityTest.php b/core/modules/filter/lib/Drupal/filter/Tests/FilterSecurityTest.php index d24042a367e9..6321c1b33b0d 100644 --- a/core/modules/filter/lib/Drupal/filter/Tests/FilterSecurityTest.php +++ b/core/modules/filter/lib/Drupal/filter/Tests/FilterSecurityTest.php @@ -9,6 +9,7 @@ use Drupal\Core\Language\Language; use Drupal\simpletest\WebTestBase; +use Drupal\filter\Plugin\FilterInterface; /** * Security tests for missing/vanished text formats or filters. @@ -32,7 +33,7 @@ class FilterSecurityTest extends WebTestBase { public static function getInfo() { return array( 'name' => 'Security', - 'description' => 'Test the behavior of check_markup() when a filter or text format vanishes, or when check_markup() is called in such a way that it is instructed to skip all filters of the "FILTER_TYPE_HTML_RESTRICTOR" type.', + 'description' => 'Test the behavior of check_markup() when a filter or text format vanishes, or when check_markup() is called in such a way that it is instructed to skip all filters of the "FilterInterface::TYPE_HTML_RESTRICTOR" type.', 'group' => 'Filter', ); } @@ -48,7 +49,7 @@ function setUp() { 'format' => 'filtered_html', 'name' => 'Filtered HTML', 'filters' => array( - // Note that the filter_html filter is of the type FILTER_TYPE_HTML_RESTRICTOR. + // Note that the filter_html filter is of the type FilterInterface::TYPE_HTML_RESTRICTOR. 'filter_html' => array( 'status' => 1, ), @@ -103,6 +104,6 @@ function testSkipSecurityFilters() { $text = "Text with some disallowed tags: <script />, <em><object>unicorn</object></em>, <i><table></i>."; $expected_filtered_text = "Text with some disallowed tags: , <em>unicorn</em>, ."; $this->assertEqual(check_markup($text, 'filtered_html', '', FALSE, array()), $expected_filtered_text, 'Expected filter result.'); - $this->assertEqual(check_markup($text, 'filtered_html', '', FALSE, array(FILTER_TYPE_HTML_RESTRICTOR)), $expected_filtered_text, 'Expected filter result, even when trying to disable filters of the FILTER_TYPE_HTML_RESTRICTOR type.'); + $this->assertEqual(check_markup($text, 'filtered_html', '', FALSE, array(FilterInterface::TYPE_HTML_RESTRICTOR)), $expected_filtered_text, 'Expected filter result, even when trying to disable filters of the FilterInterface::TYPE_HTML_RESTRICTOR type.'); } } diff --git a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestReplace.php b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestReplace.php index 8f5b31b91427..f48c5b157f21 100644 --- a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestReplace.php +++ b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestReplace.php @@ -16,7 +16,7 @@ * id = "filter_test_replace", * title = @Translation("Testing filter"), * description = @Translation("Replaces all content with filter and text format information."), - * type = FILTER_TYPE_TRANSFORM_IRREVERSIBLE + * type = Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE * ) */ class FilterTestReplace extends FilterBase { diff --git a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php index 1ff852496ac3..6e95d2eb232b 100644 --- a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php +++ b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestRestrictTagsAndAttributes.php @@ -17,7 +17,7 @@ * id = "filter_test_restrict_tags_and_attributes", * title = @Translation("Tag and attribute restricting filter"), * description = @Translation("Used for testing \Drupal\filter\Entity\FilterFormatInterface::getHtmlRestrictions()."), - * type = FILTER_TYPE_HTML_RESTRICTOR + * type = Drupal\filter\Plugin\FilterInterface::TYPE_HTML_RESTRICTOR * ) */ class FilterTestRestrictTagsAndAttributes extends FilterBase { diff --git a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestUncacheable.php b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestUncacheable.php index 4a03e7c05dce..fd8318f93e25 100644 --- a/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestUncacheable.php +++ b/core/modules/filter/tests/filter_test/lib/Drupal/filter_test/Plugin/Filter/FilterTestUncacheable.php @@ -16,7 +16,7 @@ * id = "filter_test_uncacheable", * title = @Translation("Uncacheable filter"), * description = @Translation("Does nothing, but makes a text format uncacheable"), - * type = FILTER_TYPE_TRANSFORM_IRREVERSIBLE, + * type = Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE, * cache = false * ) */ diff --git a/core/tests/Drupal/Tests/Component/Utility/XssTest.php b/core/tests/Drupal/Tests/Component/Utility/XssTest.php index 2614c5d03b07..f14a06c34965 100644 --- a/core/tests/Drupal/Tests/Component/Utility/XssTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/XssTest.php @@ -107,7 +107,7 @@ public function providerTestFilterXssNormalized() { } /** - * Tests limiting allowed tags and XSS prevention. + * Tests limiting to allowed tags and XSS prevention. * * XSS tests assume that script is disallowed by default and src is allowed * by default, but on* and style attributes are disallowed. @@ -115,11 +115,11 @@ public function providerTestFilterXssNormalized() { * @param string $value * The value to filter. * @param string $expected - * The expected result. + * The string that is expected to be missing. * @param string $message * The assertion message to display upon failure. * @param array $allowed_tags - * (Optional) The allowed tags to be passed on Xss::filter(). + * (optional) The allowed HTML tags to be passed to Xss::filter(). * * @dataProvider providerTestFilterXssNotNormalized */ @@ -140,11 +140,11 @@ public function testFilterXssNotNormalized($value, $expected, $message, array $a * * @return array * An array of arrays containing the following elements: - * - The value to filter string. - * - The value to expect after filtering string. - * - The assertion message string. - * - (optional) The allowed html tags array that should be passed to - * Xss::filter(). + * - The value to filter. + * - The value to expect that's missing after filtering. + * - The assertion message. + * - (optional) The allowed HTML HTML tags array that should be passed to + * Xss::filter(). */ public function providerTestFilterXssNotNormalized() { $cases = array( @@ -434,6 +434,65 @@ public function providerTestFilterXssNotNormalized() { return $cases; } + /** + * Tests removing disallowed tags and XSS prevention. + * + * Xss::filter() has the ability to run in blacklist mode, in which it still + * applies the exact same filtering, with one exception: it no longer works + * with a list of allowed tags, but with a list of disallowed tags. + * + * @param string $value + * The value to filter. + * @param string $expected + * The string that is expected to be missing. + * @param string $message + * The assertion message to display upon failure. + * @param array $disallowed_tags + * (optional) The disallowed HTML tags to be passed to Xss::filter(). + * + * @dataProvider providerTestBlackListMode + */ + public function testBlacklistMode($value, $expected, $message, array $disallowed_tags) { + $value = Xss::filter($value, $disallowed_tags, Xss::FILTER_MODE_BLACKLIST); + $this->assertSame($expected, $value, $message); + } + + /** + * Data provider for testBlacklistMode(). + * + * @see testBlacklistMode() + * + * @return array + * An array of arrays containing the following elements: + * - The value to filter. + * - The value to expect after filtering. + * - The assertion message. + * - (optional) The disallowed HTML tags to be passed to Xss::filter(). + */ + public function providerTestBlackListMode() { + return array( + array( + '<unknown style="visibility:hidden">Pink Fairy Armadillo</unknown><video src="gerenuk.mp4"><script>alert(0)</script>', + '<unknown>Pink Fairy Armadillo</unknown><video src="gerenuk.mp4">alert(0)', + 'Disallow only the script tag', + array('script') + ), + array( + '<unknown style="visibility:hidden">Pink Fairy Armadillo</unknown><video src="gerenuk.mp4"><script>alert(0)</script>', + '<unknown>Pink Fairy Armadillo</unknown>alert(0)', + 'Disallow both the script and video tags', + array('script', 'video') + ), + // No real use case for this, but it is an edge case we must ensure works. + array( + '<unknown style="visibility:hidden">Pink Fairy Armadillo</unknown><video src="gerenuk.mp4"><script>alert(0)</script>', + '<unknown>Pink Fairy Armadillo</unknown><video src="gerenuk.mp4"><script>alert(0)</script>', + 'Disallow no tags', + array() + ), + ); + } + /** * Checks that invalid multi-byte sequences are rejected. * @@ -521,7 +580,7 @@ public function providerTestFilterXssAdminNotNormalized() { } /** - * Asserts that a text transformed to lowercase with HTML entities decoded does contains a given string. + * Asserts that a text transformed to lowercase with HTML entities decoded does contain a given string. * * Otherwise fails the test with a given message, similar to all the * SimpleTest assert* functions. -- GitLab