Commit cc17945f authored by alexpott's avatar alexpott

Issue #2466931 by mikeker, Fabianx, dawehner, joelpittet: Valid Twig syntax is...

Issue #2466931 by mikeker, Fabianx, dawehner, joelpittet: Valid Twig syntax is incorrectly escaped in Views rewrites
parent b2e29ebb
......@@ -9,6 +9,7 @@
use Drupal\Component\Plugin\DependentPluginInterface;
use Drupal\Component\Utility\SafeMarkup;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Language\LanguageInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
......@@ -333,23 +334,27 @@ public function globalTokenReplace($string = '', array $options = array()) {
}
/**
* Replaces Views' tokens in a given string. It is the responsibility of the
* calling function to ensure $text and $token replacements are sanitized.
* Replaces Views' tokens in a given string. The resulting string will be
* sanitized with Xss::filterAdmin.
*
* This used to be a simple strtr() scattered throughout the code. Some Views
* tokens, such as arguments (e.g.: %1 or !1), still use the old format so we
* handle those as well as the new Twig-based tokens (e.g.: {{ field_name }})
*
* @param $text
* String with possible tokens.
* Unsanitized string with possible tokens.
* @param $tokens
* Array of token => replacement_value items.
*
* @return String
*/
protected function viewsTokenReplace($text, $tokens) {
if (!strlen($text)) {
// No need to run filterAdmin on an empty string.
return '';
}
if (empty($tokens)) {
return $text;
return Xss::filterAdmin($text);
}
// Separate Twig tokens from other tokens (e.g.: contextual filter tokens in
......@@ -370,11 +375,19 @@ protected function viewsTokenReplace($text, $tokens) {
// Non-Twig tokens are a straight string replacement, Twig tokens get run
// through an inline template for rendering and replacement.
$text = strtr($text, $other_tokens);
if ($twig_tokens && !empty($text)) {
if ($twig_tokens) {
// Use the unfiltered text for the Twig template, then filter the output.
// Otherwise, Xss::filterAdmin could remove valid Twig syntax before the
// template is parsed.
$build = array(
'#type' => 'inline_template',
'#template' => $text,
'#context' => $twig_tokens,
'#post_render' => [
function ($children, $elements) {
return Xss::filterAdmin($children);
}
],
);
return $this->getRenderer()->render($build);
......
......@@ -1290,9 +1290,7 @@ public function renderText($alter) {
* Render this field as user-defined altered text.
*/
protected function renderAltered($alter, $tokens) {
// Filter this right away as our substitutions are already sanitized.
$template = Xss::filterAdmin($alter['text']);
return $this->viewsTokenReplace($template, $tokens);
return $this->viewsTokenReplace($alter['text'], $tokens);
}
/**
......
......@@ -9,6 +9,7 @@
use Drupal\Component\Utility\Html;
use Drupal\Component\Utility\SafeMarkup;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Render\Element;
use Drupal\views\Plugin\views\display\DisplayPluginBase;
......@@ -239,6 +240,11 @@ public function tokenizeValue($value, $row_index) {
$value = $this->viewsTokenReplace($value, $tokens);
}
else {
// ::viewsTokenReplace() will run Xss::filterAdmin on the
// resulting string. We do the same here for consistency.
$value = Xss::filterAdmin($value);
}
return $value;
}
......
......@@ -192,7 +192,7 @@ public function testFieldTokens() {
$name_field_1->options['alter']['text'] = '{{ name_1 }} {{ name }}';
$name_field_2->options['alter']['alter_text'] = TRUE;
$name_field_2->options['alter']['text'] = '{{ name_2 }} {{ name_1 }}';
$name_field_2->options['alter']['text'] = '{% if name_2|length > 3 %}{{ name_2 }} {{ name_1 }}{% endif %}';
foreach ($view->result as $row) {
$expected_output_0 = $row->views_test_data_name;
......@@ -247,11 +247,43 @@ public function testFieldTokens() {
$output = $renderer->executeInRenderContext(new RenderContext(), function () use ($job_field, $row) {
return $job_field->advancedRender($row);
});
$this->assertSubString($output, $old_token, format_string('Make sure the old token style (!token => !value) is not changed in the output (!output)', [
$this->assertEqual($output, $old_token, format_string('Make sure the old token style (!token => !value) is not changed in the output (!output)', [
'!value' => $random_text,
'!output' => $output,
'!token' => $job_field->options['alter']['text'],
]));
// Verify HTML tags are allowed in rewrite templates while token
// replacements are escaped.
$job_field->options['alter']['text'] = '<h1>{{ job }}</h1>';
$random_text = $this->randomMachineName();
$job_field->setTestValue('<span>' . $random_text . '</span>');
$output = $renderer->executeInRenderContext(new RenderContext(), function () use ($job_field, $row) {
return $job_field->advancedRender($row);
});
$this->assertEqual($output, '<h1>&lt;span&gt;' . $random_text . '&lt;/span&gt;</h1>', 'Valid tags are allowed in rewrite templates and token replacements.');
// Verify <script> tags are correctly removed from rewritten text.
$rewrite_template = '<script>alert("malicious");</script>';
$job_field->options['alter']['text'] = $rewrite_template;
$random_text = $this->randomMachineName();
$job_field->setTestValue($random_text);
$output = $renderer->executeInRenderContext(new RenderContext(), function () use ($job_field, $row) {
return $job_field->advancedRender($row);
});
$this->assertNotSubString($output, '<script>', 'Ensure a script tag in the rewrite template is removed.');
$rewrite_template = '<script>{{ job }}</script>';
$job_field->options['alter']['text'] = $rewrite_template;
$random_text = $this->randomMachineName();
$job_field->setTestValue($random_text);
$output = $renderer->executeInRenderContext(new RenderContext(), function () use ($job_field, $row) {
return $job_field->advancedRender($row);
});
$this->assertEqual($output, $random_text, format_string('Make sure a script tag in the template (!template) is removed, leaving only the replaced token in the output (!output)', [
'!output' => $output,
'!template' => $rewrite_template,
]));
}
/**
......
......@@ -8,6 +8,7 @@
namespace Drupal\views\Tests;
use Drupal\comment\Tests\CommentTestTrait;
use Drupal\Component\Utility\Xss;
use Drupal\views\Entity\View;
use Drupal\views\Views;
use Drupal\views\ViewExecutable;
......@@ -326,7 +327,7 @@ public function testPropertyMethods() {
// Test the title methods.
$title = $this->randomString();
$view->setTitle($title);
$this->assertEqual($view->getTitle(), $title);
$this->assertEqual($view->getTitle(), Xss::filterAdmin($title));
}
/**
......
......@@ -453,6 +453,7 @@ public function testRenderAsLinkWithPathAndTokens($path, $tokens, $link_html) {
'#type' => 'inline_template',
'#template' => 'base:test-path/' . explode('/', $path)[1],
'#context' => ['foo' => 123],
'#post_render' => [function() {}],
];
$this->renderer->expects($this->once())
......
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