diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index bc725c7..9925ae1 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -141,7 +141,7 @@ public static function escape($string) { /** * Applies a very permissive XSS/HTML filter for admin-only use. * - * Note: this method only filters if $string is not marked safe already. + * Note: This method only filters if $string is not marked safe already. * * @deprecated as of Drupal 8.0.x, will be removed before Drupal 8.0.0. If the * string used as part of a @link theme_render render array @endlink use @@ -167,7 +167,7 @@ public static function checkAdminXss($string) { * Filters HTML for XSS vulnerabilities and marks the result as safe. * * Calling this method unnecessarily will result in bloating the safe string - * list and increases the chance of unintended side-effects. + * list and increases the chance of unintended side effects. * * If Twig receives a value that is not marked as safe then it will * automatically encode special characters in a plain-text string for display @@ -193,10 +193,11 @@ public static function checkAdminXss($string) { * can cause an XSS attack. The string provided will always be escaped * regardless of whether the string is already marked as safe. * @param array $html_tags - * An array of HTML tags. + * (optional) An array of HTML tags. If omitted, it uses the default tag + * list defined by \Drupal\Component\Utility\Xss::filter(). * * @return string - * An XSS safe version of $string, or an empty string if $string is not + * An XSS-safe version of $string, or an empty string if $string is not * valid UTF-8. The string is marked as safe. * * @ingroup sanitization @@ -206,8 +207,13 @@ public static function checkAdminXss($string) { * @see \Drupal\Component\Utility\Xss::getAdminTagList() * @see \Drupal\Component\Utility\SafeMarkup::isSafe() */ - public static function xssFilter($string, $html_tags = array('a', 'em', 'strong', 'cite', 'blockquote', 'code', 'ul', 'ol', 'li', 'dl', 'dt', 'dd')) { - $string = Xss::filter($string, $html_tags); + public static function xssFilter($string, $html_tags = NULL) { + if (is_null($html_tags)) { + $string = Xss::filter($string); + } + else { + $string = Xss::filter($string, $html_tags); + } return static::set($string); } diff --git a/core/lib/Drupal/Core/Render/Element/HtmlTag.php b/core/lib/Drupal/Core/Render/Element/HtmlTag.php index 477f847..f0c78bb 100644 --- a/core/lib/Drupal/Core/Render/Element/HtmlTag.php +++ b/core/lib/Drupal/Core/Render/Element/HtmlTag.php @@ -174,8 +174,8 @@ public static function preRenderConditionalComments($element) { // Ensure what we are dealing with is safe. // This would be done later anyway in drupal_render(). - $prefix = isset($elements['#prefix']) ? Xss::FilterAdmin($elements['#prefix']) : ''; - $suffix = isset($elements['#suffix']) ? Xss::FilterAdmin($elements['#suffix']) : ''; + $prefix = isset($elements['#prefix']) ? Xss::filterAdmin($elements['#prefix']) : ''; + $suffix = isset($elements['#suffix']) ? Xss::filterAdmin($elements['#suffix']) : ''; // Now calling SafeMarkup::set is safe, because we ensured the // data coming in was at least admin escaped. diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index abd0a9b..503a26d 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -654,7 +654,7 @@ public function addCacheableDependency(array &$elements, $dependency) { /** * Applies a very permissive XSS/HTML filter for admin-only use. * - * Note: this method only filters if $string is not marked safe already. This + * Note: This method only filters if $string is not marked safe already. This * ensures that HTML intended for display is not filtered. * * @param string $string diff --git a/core/modules/block_content/block_content.pages.inc b/core/modules/block_content/block_content.pages.inc index 84b1a81..dbb614a 100644 --- a/core/modules/block_content/block_content.pages.inc +++ b/core/modules/block_content/block_content.pages.inc @@ -29,7 +29,7 @@ function template_preprocess_block_content_add_list(&$variables) { 'link' => \Drupal::l($type->label(), new Url('block_content.add_form', array('block_content_type' => $type->id()), array('query' => $query))), 'description' => array( // #markup is filtered for admin XSS automatically. - '#markup' => $type->getDescription() + '#markup' => $type->getDescription(), ), 'title' => $type->label(), 'localized_options' => array( diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index 9c543c0..25acc43 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -356,7 +356,7 @@ function _filter_tips($format_id, $long = FALSE) { $tips[$format->label()][$name] = array( // #markup is filtered for admin XSS automatically. 'tip' => array('#markup' => $tip), - 'id' => $name + 'id' => $name, ); } } diff --git a/core/modules/views/src/Plugin/Block/ViewsBlock.php b/core/modules/views/src/Plugin/Block/ViewsBlock.php index 18c47c7..97db7c9 100644 --- a/core/modules/views/src/Plugin/Block/ViewsBlock.php +++ b/core/modules/views/src/Plugin/Block/ViewsBlock.php @@ -8,6 +8,7 @@ namespace Drupal\views\Plugin\Block; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\Xss; use Drupal\Core\Config\Entity\Query\Query; use Drupal\Core\Form\FormStateInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -32,9 +33,8 @@ public function build() { if ($output = $this->view->buildRenderable($this->displayID, [], FALSE)) { // Override the label to the dynamic title configured in the view. if (empty($this->configuration['views_label']) && $this->view->getTitle()) { - // The title will be autoescaped by Twig so it is not necessary to - // XSS filter it. - $output['#title'] = $this->view->getTitle(); + // @todo https://www.drupal.org/node/2527360 remove call to SafeMarkup. + $output['#title'] = SafeMarkup::xssFilter($this->view->getTitle(), Xss::getAdminTagList()); } // Before returning the block output, convert it to a renderable array diff --git a/core/modules/views/src/Plugin/views/display/Page.php b/core/modules/views/src/Plugin/views/display/Page.php index e4e0e1b..7ec3560 100644 --- a/core/modules/views/src/Plugin/views/display/Page.php +++ b/core/modules/views/src/Plugin/views/display/Page.php @@ -8,6 +8,7 @@ namespace Drupal\views\Plugin\views\display; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Component\Utility\Xss; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\State\StateInterface; @@ -181,8 +182,8 @@ public function execute() { // it should be dropped. if (is_array($render)) { $render += array( - // #title will be auto escaped by Twig. - '#title' => $this->view->getTitle(), + // @todo https://www.drupal.org/node/2527360 remove call to SafeMarkup. + '#title' => SafeMarkup::xssFilter($this->view->getTitle(), Xss::getAdminTagList()), ); } return $render; diff --git a/core/modules/views_ui/views_ui.module b/core/modules/views_ui/views_ui.module index 5b43ad1..48d70f6 100644 --- a/core/modules/views_ui/views_ui.module +++ b/core/modules/views_ui/views_ui.module @@ -129,9 +129,8 @@ function views_ui_preprocess_views_view(&$variables) { // Render title for the admin preview. if (!empty($view->live_preview)) { - // The title will be autoescaped by Twig so it is not necessary to XSS - // filter it. - $variables['title'] = $view->getTitle(); + // #markup is filtered for admin XSS automatically. + $variables['title']['#markup'] = $view->getTitle(); } if (!empty($view->live_preview) && \Drupal::moduleHandler()->moduleExists('contextual')) { diff --git a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php index b5b609a..90a475f 100644 --- a/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php +++ b/core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php @@ -222,19 +222,30 @@ public function testReplace($search, $replace, $subject, $expected, $is_safe) { public function testAdminXss() { // Use the predefined XSS admin tag list. This strips the tags. $this->assertEquals('text', SafeMarkup::xssFilter('text', Xss::getAdminTagList())); + $this->assertTrue(SafeMarkup::isSafe('text'), 'The string \'text\' is marked as safe.'); - // This won't strip the tags and the string with html will be + // This won't strip the tags and the string with HTML will be // marked as safe. $filtered = SafeMarkup::xssFilter('text', array('marquee')); $this->assertEquals('text', $filtered); $this->assertTrue(SafeMarkup::isSafe('text'), 'The string \'text\' is marked as safe.'); - // The default tag list strips the tags even though the string is - // marked as safe. + // SafeMarkup::xssFilter() with the default tag list will strip the + // tag even though the string was marked safe above. $this->assertEquals('text', SafeMarkup::xssFilter('text')); - // This won't escape the tags since it is marked as safe. + // SafeMarkup::escape() will not escape the markup tag since the string was + // marked safe above. $this->assertEquals('text', SafeMarkup::escape($filtered)); + + // SafeMarkup::checkPlain() will escape the markup tag even though the + // string was marked safe above. + $this->assertEquals('<marquee>text</marquee>', SafeMarkup::checkPlain($filtered)); + + // Ensure that SafeMarkup::xssFilter strips all tags when passed an empty + // array and uses the default tag list when not passed a tag list. + $this->assertEquals('text', SafeMarkup::xssFilter('text', [])); + $this->assertEquals('text', SafeMarkup::xssFilter('text')); } /**