Skip to content

GitLab

Commit 00d1bdbe authored by catch's avatar catch
Browse files

Issue #2555931 by alexpott, pwolanin, joelpittet: Add #plain_text to escape text in render arrays

parent 9bee4588
Hide whitespace changes
Inline Side-by-side
core/lib/Drupal/Component/Utility/Html.php
View file @ 00d1bdbe
......@@ -374,6 +374,10 @@ public static function decodeEntities($text) {
* Html::decodeEntities() will convert all HTML entities to UTF-8 bytes,
* including "&eacute;" and "&lt;" to "é" and "<".
*
* When constructing @link theme_render render arrays @endlink passing the output of Html::escape() to
* '#markup' is not recommended. Use the '#plain_text' key instead and the
* renderer will autoescape the text.
*
* @param string $text
* The input text.
*
......
core/lib/Drupal/Core/Render/Element/Markup.php deleted 100644 → 0
View file @ 9bee4588
<?php
/**
* @file
* Contains \Drupal\Core\Render\Element\Markup.
*/
namespace Drupal\Core\Render\Element;
use Drupal\Component\Utility\Html as HtmlUtility;
use Drupal\Component\Utility\SafeMarkup;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Render\SafeString;
/**
* Provides a render element for HTML as a string, with sanitization.
*
* Properties:
* - #markup: Specifies that the array provides HTML markup directly. Unless
* the markup is very simple, such as an explanation in a paragraph tag, it
* is normally preferable to use #theme or #type instead, so that the theme
* can customize the markup. Note that the value is passed through
* \Drupal\Component\Utility\Xss::filterAdmin(), which strips known XSS
* vectors while allowing a permissive list of HTML tags that are not XSS
* vectors. (I.e, <script> and <style> are not allowed.) See
* \Drupal\Component\Utility\Xss::$adminTags for the list of tags that will
* be allowed. If your markup needs any of the tags that are not in this
* whitelist, then you can implement a theme hook and template file and/or
* an asset library. Alternatively, you can use the render array keys
* #safe_strategy and #allowed_tags to alter how #markup is made safe.
* - #safe_strategy: If #markup is supplied this can be used to change
* how the string is made safe for render. By default, all #markup is filtered
* using Xss::adminFilter(). However, if the string should be escaped using
* Html::escape() then this should be set to Markup::SAFE_STRATEGY_ESCAPE.
* - #allowed_tags: If #markup is supplied this can be used to change which tags
* are using to filter the markup. The value should be an array of tags that
* Xss::filter() would accept. If #safe_strategy is set to
* Markup::SAFE_STRATEGY_ESCAPE this value is ignored.
*
* Usage example:
* @code
* $output['admin_filtered_string'] = array(
* '#type' => 'markup',
* '#markup' => '<em>This is filtered using the admin tag list</em>',
* );
* $output['filtered_string'] = array(
* '#type' => 'markup',
* '#markup' => '<em>This is filtered</em>',
* '#allowed_tags' => ['strong'],
* );
* $output['escaped_string'] = array(
* '#type' => 'markup',
* '#markup' => '<em>This is escaped</em>',
* '#safe_strategy' => Markup::SAFE_STRATEGY_ESCAPE,
* );
* @endcode
*
* @see theme_render
*
* @ingroup sanitization
*
* @RenderElement("markup")
*/
class Markup extends RenderElement {
/**
* #safe_strategy indicating #markup should be filtered.
*
* @see ::ensureMarkupIsSafe()
* @see \Drupal\Component\Utility\Xss::filter()
*/
const SAFE_STRATEGY_FILTER = 'xss';
/**
* #safe_strategy indicating #markup should be escaped.
*
* @see ::ensureMarkupIsSafe()
* @see \Drupal\Component\Utility\Html::escape()
*/
const SAFE_STRATEGY_ESCAPE = 'escape';
/**
* {@inheritdoc}
*/
public function getInfo() {
return [
'#pre_render' => [
[static::class, 'ensureMarkupIsSafe'],
],
];
}
/**
* Escapes or filters #markup as required.
*
* Drupal uses Twig's auto-escape feature to improve security. This feature
* automatically escapes any HTML that is not known to be safe. Due to this
* the render system needs to ensure that all markup it generates is marked
* safe so that Twig does not do any additional escaping.
*
* By default all #markup is filtered to protect against XSS using the admin
* tag list. Render arrays can alter the list of tags allowed by the filter
* using the #allowed_tags property. This value should be an array of tags
* that Xss::filter() would accept. Render arrays can escape #markup instead
* of XSS filtering by setting the #safe_strategy property to
* Markup:SAFE_STRATEGY_ESCAPE. If the escaping strategy is used #allowed_tags
* is ignored.
*
* @param array $elements
* A render array with #markup set.
*
* @return \Drupal\Component\Utility\SafeStringInterface|string
* The escaped markup wrapped in a SafeString object. If
* SafeMarkup::isSafe($elements['#markup']) returns TRUE, it won't be
* escaped or filtered again.
*
* @see \Drupal\Component\Utility\Html::escape()
* @see \Drupal\Component\Utility\Xss::filter()
* @see \Drupal\Component\Utility\Xss::adminFilter()
* @see \Drupal\Core\Render\Element\Markup::SAFE_STRATEGY_FILTER
* @see \Drupal\Core\Render\Element\Markup::SAFE_STRATEGY_ESCAPE
*/
public static function ensureMarkupIsSafe(array $elements) {
if (empty($elements['#markup'])) {
return $elements;
}
$strategy = isset($elements['#safe_strategy']) ? $elements['#safe_strategy'] : static::SAFE_STRATEGY_FILTER;
if (SafeMarkup::isSafe($elements['#markup'])) {
// Nothing to do as #markup is already marked as safe.
return $elements;
}
elseif ($strategy == static::SAFE_STRATEGY_ESCAPE) {
$markup = HtmlUtility::escape($elements['#markup']);
}
else {
// The default behaviour is to XSS filter using the admin tag list.
$tags = isset($elements['#allowed_tags']) ? $elements['#allowed_tags'] : Xss::getAdminTagList();
$markup = Xss::filter($elements['#markup'], $tags);
}
$elements['#markup'] = SafeString::create($markup);
return $elements;
}
}
core/lib/Drupal/Core/Render/Renderer.php
View file @ 00d1bdbe
......@@ -7,6 +7,7 @@
namespace Drupal\Core\Render;
use Drupal\Component\Utility\Html;
use Drupal\Component\Utility\SafeMarkup;
use Drupal\Component\Utility\UrlHelper;
use Drupal\Component\Utility\Xss;
......@@ -14,6 +15,7 @@
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Controller\ControllerResolverInterface;
use Drupal\Core\Render\Element\Markup;
use Drupal\Core\Template\Attribute;
use Drupal\Core\Theme\ThemeManagerInterface;
use Symfony\Component\HttpFoundation\RequestStack;
......@@ -301,12 +303,6 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
$pre_bubbling_elements = [];
$pre_bubbling_elements['#cache'] = isset($elements['#cache']) ? $elements['#cache'] : [];
// If #markup is set, ensure #type is set. This allows to specify just
// #markup on an element without setting #type.
if (isset($elements['#markup']) && !isset($elements['#type'])) {
$elements['#type'] = 'markup';
}
// If the default values for this element have not been loaded yet, populate
// them.
if (isset($elements['#type']) && empty($elements['#defaults_loaded'])) {
......@@ -377,6 +373,12 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
$elements = $new_elements;
$elements['#lazy_builder_built'] = TRUE;
}
// All render elements support #markup and #plain_text.
if (!empty($elements['#markup']) || !empty($elements['#plain_text'])) {
$elements = $this->ensureMarkupIsSafe($elements);
}
// Make any final changes to the element before it is rendered. This means
// that the $element or the children can be altered or corrected before the
// element is rendered into the final text.
......@@ -756,4 +758,48 @@ protected function xssFilterAdminIfUnsafe($string) {
return SafeString::create($string);
}
/**
* Escapes #plain_text or filters #markup as required.
*
* Drupal uses Twig's auto-escape feature to improve security. This feature
* automatically escapes any HTML that is not known to be safe. Due to this
* the render system needs to ensure that all markup it generates is marked
* safe so that Twig does not do any additional escaping.
*
* By default all #markup is filtered to protect against XSS using the admin
* tag list. Render arrays can alter the list of tags allowed by the filter
* using the #allowed_tags property. This value should be an array of tags
* that Xss::filter() would accept. Render arrays can escape text instead
* of XSS filtering by setting the #plain_text property instead of #markup. If
* #plain_text is used #allowed_tags is ignored.
*
* @param array $elements
* A render array with #markup set.
*
* @return \Drupal\Component\Utility\SafeStringInterface|string
* The escaped markup wrapped in a SafeString object. If
* SafeMarkup::isSafe($elements['#markup']) returns TRUE, it won't be
* escaped or filtered again.
*
* @see \Drupal\Component\Utility\Html::escape()
* @see \Drupal\Component\Utility\Xss::filter()
* @see \Drupal\Component\Utility\Xss::adminFilter()
*/
protected function ensureMarkupIsSafe(array $elements) {
if (empty($elements['#markup']) && empty($elements['#plain_text'])) {
return $elements;
}
if (!empty($elements['#plain_text'])) {
$elements['#markup'] = SafeString::create(Html::escape($elements['#plain_text']));
}
elseif (!SafeMarkup::isSafe($elements['#markup'])) {
// The default behaviour is to XSS filter using the admin tag list.
$tags = isset($elements['#allowed_tags']) ? $elements['#allowed_tags'] : Xss::getAdminTagList();
$elements['#markup'] = SafeString::create(Xss::filter($elements['#markup'], $tags));
}
return $elements;
}
}
core/lib/Drupal/Core/Render/theme.api.php
View file @ 00d1bdbe
......@@ -267,11 +267,37 @@
* - #markup: Specifies that the array provides HTML markup directly. Unless
* the markup is very simple, such as an explanation in a paragraph tag, it
* is normally preferable to use #theme or #type instead, so that the theme
* can customize the markup. For additional options related to #markup see
* \Drupal\Core\Render\Element\Markup.
* can customize the markup. Note that the value is passed through
* \Drupal\Component\Utility\Xss::filterAdmin(), which strips known XSS
* vectors while allowing a permissive list of HTML tags that are not XSS
* vectors. (I.e, <script> and <style> are not allowed.) See
* \Drupal\Component\Utility\Xss::$adminTags for the list of tags that will
* be allowed. If your markup needs any of the tags that are not in this
* whitelist, then you can implement a theme hook and template file and/or
* an asset library. Aternatively, you can use the render array key
* #allowed_tags to alter which tags are filtered.
* - #plain_text: Specifies that the array provides text that needs to be
* escaped. This value takes precedence over #markup if present.
* - #allowed_tags: If #markup is supplied this can be used to change which tags
* are using to filter the markup. The value should be an array of tags that
* Xss::filter() would accept. If #plain_text is set this value is ignored.
*
* Usage example:
* @code
* $output['admin_filtered_string'] = array(
* '#markup' => '<em>This is filtered using the admin tag list</em>',
* );
* $output['filtered_string'] = array(
* '#markup' => '<em>This is filtered</em>',
* '#allowed_tags' => ['strong'],
* );
* $output['escaped_string'] = array(
* '#plain_text' => '<em>This is escaped</em>',
* );
* @endcode
*
* @see core.libraries.yml
* @see hook_theme()
* @see \Drupal\Core\Render\Element\Markup
*
* JavaScript and CSS assets are specified in the render array using the
* #attached property (see @ref sec_attached).
......
core/modules/contact/src/MessageForm.php
View file @ 00d1bdbe
......@@ -130,12 +130,12 @@ public function form(array $form, FormStateInterface $form_state) {
$form['name']['#type'] = 'item';
$form['name']['#value'] = $user->getUsername();
$form['name']['#required'] = FALSE;
$form['name']['#markup'] = SafeMarkup::checkPlain($user->getUsername());
$form['name']['#plain_text'] = $user->getUsername();
$form['mail']['#type'] = 'item';
$form['mail']['#value'] = $user->getEmail();
$form['mail']['#required'] = FALSE;
$form['mail']['#markup'] = SafeMarkup::checkPlain($user->getEmail());
$form['mail']['#plain_text'] = $user->getEmail();
}
// The user contact form has a preset recipient.
......
core/modules/contact/src/Tests/ContactPersonalTest.php
View file @ 00d1bdbe
......@@ -63,8 +63,13 @@ protected function setUp() {
* Tests that mails for contact messages are correctly sent.
*/
function testSendPersonalContactMessage() {
// Ensure that the web user's email needs escaping.
$mail = $this->webUser->getUsername() . '&escaped@example.com';
$this->webUser->setEmail($mail)->save();
$this->drupalLogin($this->webUser);
$this->drupalGet('user/' . $this->contactUser->id() . '/contact');
$this->assertEscaped($mail);
$message = $this->submitPersonalContact($this->contactUser);
$mails = $this->drupalGetMails();
$this->assertEqual(1, count($mails));
......@@ -93,7 +98,9 @@ function testSendPersonalContactMessage() {
'@sender_email' => $this->webUser->getEmail(),
'@recipient_name' => $this->contactUser->getUsername()
);
$this->assertText(SafeMarkup::format('@sender_name (@sender_email) sent @recipient_name an email.', $placeholders));
$this->assertRaw(SafeMarkup::format('@sender_name (@sender_email) sent @recipient_name an email.', $placeholders));
// Ensure an unescaped version of the email does not exist anywhere.
$this->assertNoRaw($this->webUser->getEmail());
}
/**
......
core/modules/dblog/src/Controller/DbLogController.php
View file @ 00d1bdbe
......@@ -184,13 +184,16 @@ public function overview() {
foreach ($result as $dblog) {
$message = $this->formatMessage($dblog);
if ($message && isset($dblog->wid)) {
// Truncate link_text to 56 chars of message. The l() call will escape
// any unsafe HTML entities in the final text.
$log_text = Unicode::truncate(Html::decodeEntities(strip_tags($message)), 56, TRUE, TRUE);
$title = Unicode::truncate(Html::decodeEntities(strip_tags($message)), 256, TRUE, TRUE);
$log_text = Unicode::truncate($title, 56, TRUE, TRUE);
// The link generator will escape any unsafe HTML entities in the final
// text.
$message = $this->l($log_text, new Url('dblog.event', array('event_id' => $dblog->wid), array(
'attributes' => array(
// Provide a title for the link for useful hover hints.
'title' => Unicode::truncate(strip_tags($message), 256, TRUE, TRUE),
// Provide a title for the link for useful hover hints. The
// Attribute object will escape any unsafe HTML entities in the
// final text.
'title' => $title,
),
)));
}
......
core/modules/dblog/src/Tests/DbLogTest.php
View file @ 00d1bdbe
......@@ -733,11 +733,12 @@ public function testTemporaryUser() {
*/
public function testOverviewLinks() {
$this->drupalLogin($this->adminUser);
$this->generateLogEntries(1, ['message' => "&lt;script&gt;alert('foo');&lt;/script&gt;<strong>Lorem</strong> ipsum dolor sit amet, consectetur adipiscing elit."]);
$this->generateLogEntries(1, ['message' => "&lt;script&gt;alert('foo');&lt;/script&gt;<strong>Lorem</strong> ipsum dolor sit amet, consectetur adipiscing & elit."]);
$this->drupalGet('admin/reports/dblog');
$this->assertResponse(200);
// Make sure HTML tags are filtered out.
$this->assertRaw('title="&amp;lt;script&amp;gt;alert(&#039;foo&#039;);&amp;lt;/script&amp;gt;Lorem ipsum dolor sit amet, consectetur adipiscing elit. Entry #0">&lt;script&gt;alert(&#039;foo&#039;);&lt;/script&gt;Lorem ipsum dolor sit…</a>');
$this->assertRaw('title="alert(&#039;foo&#039;);Lorem ipsum dolor sit amet, consectetur adipiscing &amp; elit. Entry #0">&lt;script&gt;alert(&#039;foo&#039;);&lt;/script&gt;Lorem ipsum dolor sit…</a>');
$this->assertNoRaw("<script>alert('foo');</script>");
}
}
core/modules/search/search.module
View file @ 00d1bdbe
......@@ -9,7 +9,6 @@
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Render\Element\Markup;
use Drupal\Core\Routing\RouteMatchInterface;
/**
......@@ -723,8 +722,7 @@ function search_excerpt($keys, $text, $langcode = NULL) {
// text. We also need to re-encode any HTML special characters that we
// entity-decoded above.
return [
'#markup' => Unicode::truncate($text, 256, TRUE, TRUE),
'#safe_strategy' => Markup::SAFE_STRATEGY_ESCAPE,
'#plain_text' => Unicode::truncate($text, 256, TRUE, TRUE),
];
}
......
core/modules/views/src/Tests/ViewsHooksTest.php
View file @ 00d1bdbe
......@@ -7,7 +7,6 @@
namespace Drupal\views\Tests;
use Drupal\Core\Render\Element\Markup;
use Drupal\Core\Render\RenderContext;
use Drupal\views\Views;
......@@ -108,8 +107,7 @@ public function testHooks() {
public function testViewsPreRenderViewsFormViewsForm() {
$element = [
'output' => [
'#markup' => '<!--will-be-escaped--><!--will-be-not-escaped-->',
'#safe_strategy' => Markup::SAFE_STRATEGY_ESCAPE,
'#plain_text' => '<!--will-be-escaped--><!--will-be-not-escaped-->',
],
'#substitutions' => ['#value' => []],
];
......
core/tests/Drupal/Tests/Core/Render/RendererTest.php
View file @ 00d1bdbe
......@@ -101,6 +101,19 @@ public function providerTestRenderBasic() {
$data[] = [[
'#markup' => 'foo',
], 'foo'];
// Basic #plain_text based renderable array.
$data[] = [[
'#plain_text' => 'foo',
], 'foo'];
// Mixing #plain_text and #markup based renderable array.
$data[] = [[
'#plain_text' => '<em>foo</em>',
'#markup' => 'bar',
], '&lt;em&gt;foo&lt;/em&gt;'];
// Safe strings in #plain_text are are still escaped.
$data[] = [[
'#plain_text' => SafeString::create('<em>foo</em>'),
], '&lt;em&gt;foo&lt;/em&gt;'];
// Renderable child element.
$data[] = [[
'child' => ['#markup' => 'bar'],
......@@ -119,11 +132,11 @@ public function providerTestRenderBasic() {
], "This is <em>alert('XSS')</em> <strong>test</strong>"];
// Html escaping test.
$data[] = [[
'child' => ['#markup' => "This is <script><em>alert('XSS')</em></script> <strong>test</strong>", '#safe_strategy' => Markup::SAFE_STRATEGY_ESCAPE],
'child' => ['#plain_text' => "This is <script><em>alert('XSS')</em></script> <strong>test</strong>"],
], "This is &lt;script&gt;&lt;em&gt;alert(&#039;XSS&#039;)&lt;/em&gt;&lt;/script&gt; &lt;strong&gt;test&lt;/strong&gt;"];
// XSS filtering by default test.
$data[] = [[
'child' => ['#markup' => "This is <script><em>alert('XSS')</em></script> <strong>test</strong>", '#safe_strategy' => 'nonsense'],
'child' => ['#markup' => "This is <script><em>alert('XSS')</em></script> <strong>test</strong>"],
], "This is <em>alert('XSS')</em> <strong>test</strong>"];
// Ensure non-XSS tags are not filtered out.
$data[] = [[
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment