diff --git a/core/lib/Drupal/Component/Utility/SafeMarkup.php b/core/lib/Drupal/Component/Utility/SafeMarkup.php index 172a323..69d12fe 100644 --- a/core/lib/Drupal/Component/Utility/SafeMarkup.php +++ b/core/lib/Drupal/Component/Utility/SafeMarkup.php @@ -282,4 +282,51 @@ public static function placeholder($text) { return $string; } + /** + * Replaces all occurrences of the search string with the replacement string. + * + * Functions identically to str_replace(), but marks the returned output as + * safe if all the inputs and the subject have also been marked as safe. + * + * @param string|array $search + * The value being searched for. An array may be used to designate multiple + * values to search for. + * @param string|array $replace + * The replacement value that replaces found search values. An array may be + * used to designate multiple replacements. + * @param string $subject + * The string or array being searched and replaced on. + * + * @return string + * The passed subject with replaced values. + */ + public static function replace($search, $replace, $subject) { + $output = str_replace($search, $replace, $subject); + + // If any replacement is unsafe, then the output is also unsafe, so just + // return the output. + if (!is_array($replace)) { + if (!SafeMarkup::isSafe($replace)) { + return $output; + } + } + else { + foreach ($replace as $replacement) { + if (!SafeMarkup::isSafe($replacement)) { + return $output; + } + } + } + + // If the subject is unsafe, then the output is as well, so return it. + if (!SafeMarkup::isSafe($subject)) { + return $output; + } + else { + // If we have reached this point, then all replacements were safe. If the + // subject was also safe, then mark the entire output as safe. + return SafeMarkup::set($output); + } + } + } diff --git a/core/lib/Drupal/Core/Render/Element/HtmlTag.php b/core/lib/Drupal/Core/Render/Element/HtmlTag.php index 553767f..9b2ab57 100644 --- a/core/lib/Drupal/Core/Render/Element/HtmlTag.php +++ b/core/lib/Drupal/Core/Render/Element/HtmlTag.php @@ -46,7 +46,12 @@ public function getInfo() { * Pre-render callback: Renders a generic HTML tag with attributes into #markup. * * Note: It is the caller's responsibility to sanitize any input parameters. - * This callback does not perform sanitization. + * This callback does not perform sanitization. Despite the result of this + * pre-render callback being a #markup element, it is not passed through + * \Drupal\Component\Utility\Xss::filterAdmin(). This is because it is marked + * safe here, which causes + * \Drupal\Component\Utility\SafeMarkup::checkAdminXss() to regard it as safe + * and bypass the call to \Drupal\Component\Utility\Xss::filterAdmin(). * * @param array $element * An associative array containing: @@ -94,7 +99,7 @@ public static function preRenderHtmlTag($element) { $markup = SafeMarkup::set($markup); } if (!empty($element['#noscript'])) { - $element['#markup'] = ''; + $element['#markup'] = SafeMarkup::format('', ['@markup' => $markup]); } else { $element['#markup'] = $markup; diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index 8bf3db6..9847855 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -350,9 +350,8 @@ protected function doRender(&$elements, $is_root_call = FALSE) { $elements['#children'] = ''; } - // @todo Simplify after https://www.drupal.org/node/2273925. if (isset($elements['#markup'])) { - $elements['#markup'] = SafeMarkup::set($elements['#markup']); + $elements['#markup'] = SafeMarkup::checkAdminXss($elements['#markup']); } // Assume that if #theme is set it represents an implemented hook. @@ -613,7 +612,10 @@ protected function createPlaceholder(array $element) { // Build the placeholder element to return. $placeholder_element = []; - $placeholder_element['#markup'] = $placeholder_markup; + // A placeholder should not be removed by SafeMarkup::checkAdminXss(), so + // mark it as safe markup. We have complete control over its generation so + // know it is safe. + $placeholder_element['#markup'] = SafeMarkup::set($placeholder_markup); $placeholder_element['#attached']['placeholders'][$placeholder_markup] = $placeholder_render_array; return $placeholder_element; } diff --git a/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php b/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php index d10078b..6e0b7f4 100644 --- a/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php +++ b/core/modules/contextual/src/Element/ContextualLinksPlaceholder.php @@ -9,6 +9,7 @@ use Drupal\Core\Template\Attribute; use Drupal\Core\Render\Element\RenderElement; +use Drupal\Component\Utility\SafeMarkup; /** * Provides a contextual_links_placeholder element. @@ -47,7 +48,10 @@ public function getInfo() { * @see _contextual_links_to_id() */ public static function preRenderPlaceholder(array $element) { - $element['#markup'] = ' $element['#id'])) . '>'; + // This markup is safe because the arguments will always be instances of + // \Drupal\Core\Template\AttributeString, which is passed through + // \Drupal\Component\Utility\SafeMarkup::checkPlain() before being output. + $element['#markup'] = SafeMarkup::set(' $element['#id']]) . '>'); return $element; } diff --git a/core/modules/filter/src/Element/ProcessedText.php b/core/modules/filter/src/Element/ProcessedText.php index d007b5f..2f92b22 100644 --- a/core/modules/filter/src/Element/ProcessedText.php +++ b/core/modules/filter/src/Element/ProcessedText.php @@ -8,6 +8,7 @@ namespace Drupal\filter\Element; use Drupal\Component\Utility\NestedArray; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Cache\Cache; use Drupal\Core\Render\BubbleableMetadata; use Drupal\Core\Render\Element\RenderElement; @@ -118,9 +119,15 @@ public static function preRenderText($element) { } } - // Filtering done, store in #markup, set the updated bubbleable rendering - // metadata, and set the text format's cache tag. - $element['#markup'] = $text; + // Filtering and sanitizing have been done in + // \Drupal\filter\Plugin\FilterInterface. $text is not guaranteed to be + // safe, but it has been passed through the filter system and checked with + // a text format, so it must be printed as is. (See the note about security + // in the method documentation above.) + $element['#markup'] = SafeMarkup::set($text); + + // Set the updated bubbleable rendering metadata and the text format's + // cache tag. $metadata->applyTo($element); $element['#cache']['tags'] = Cache::mergeTags($element['#cache']['tags'], $format->getCacheTags()); diff --git a/core/modules/rest/src/Plugin/views/display/RestExport.php b/core/modules/rest/src/Plugin/views/display/RestExport.php index 32c0110..d365678 100644 --- a/core/modules/rest/src/Plugin/views/display/RestExport.php +++ b/core/modules/rest/src/Plugin/views/display/RestExport.php @@ -284,6 +284,7 @@ public function execute() { $header = []; $header['Content-type'] = $this->getMimeType(); + $output['#markup'] = SafeMarkup::set($output['#markup']); $response = new CacheableResponse($this->renderer->renderRoot($output), 200); $cache_metadata = CacheableMetadata::createFromRenderArray($output); diff --git a/core/modules/rest/src/Tests/Views/StyleSerializerTest.php b/core/modules/rest/src/Tests/Views/StyleSerializerTest.php index 0d16fcb..fd890f7 100644 --- a/core/modules/rest/src/Tests/Views/StyleSerializerTest.php +++ b/core/modules/rest/src/Tests/Views/StyleSerializerTest.php @@ -390,6 +390,16 @@ public function testFieldapiField() { $result = $this->drupalGetJSON('test/serialize/node-field'); $this->assertEqual($result[0]['nid'], $node->id()); $this->assertEqual($result[0]['body'], $node->body->processed); - } + // Make sure that serialized fields are not exposed to XSS. + $node = $this->drupalCreateNode(); + $node->body = [ + 'value' => '' . $this->randomMachineName(32), + 'format' => filter_default_format(), + ]; + $node->save(); + $result = $this->drupalGetJSON('test/serialize/node-field'); + $this->assertEqual($result[1]['nid'], $node->id()); + $this->assertTrue(strpos($this->getRawContent(), " and