Issue #2495179 by dawehner, Gábor Hojtsy, lauriii, Fabianx, chx, effulgentsia:...

Issue #2495179 by dawehner, Gábor Hojtsy, lauriii, Fabianx, chx, effulgentsia: Twig placeholder filter should not map to raw filter

core/lib/Drupal/Core/Template/TwigExtension.php

@@ -132,7 +132,7 @@ public function getFilters() {
       // be used in "trans" tags.
       // @see TwigNodeTrans::compileString()
       new \Twig_SimpleFilter('passthrough', 'twig_raw_filter', array('is_safe' => array('html'))),
-      new \Twig_SimpleFilter('placeholder', 'twig_raw_filter', array('is_safe' => array('html'))),
+      new \Twig_SimpleFilter('placeholder', [$this, 'escapePlaceholder'], array('is_safe' => array('html'), 'needs_environment' => TRUE)),
 
       // Replace twig's escape filter with our own.
       new \Twig_SimpleFilter('drupal_escape', [$this, 'escapeFilter'], array('needs_environment' => true, 'is_safe_callback' => 'twig_escape_filter_is_safe')),
@@ -350,6 +350,21 @@ public function attachLibrary($library) {
     $this->renderer->render($template_attached);
   }
 
+  /**
+   * Provides a placeholder wrapper around ::escapeFilter.
+   *
+   * @param \Twig_Environment $env
+   *   A Twig_Environment instance.
+   * @param mixed $string
+   *   The value to be escaped.
+   *
+   * @return string|null
+   *   The escaped, rendered output, or NULL if there is no valid output.
+   */
+  public function escapePlaceholder($env, $string) {
+    return '<em class="placeholder">' . $this->escapeFilter($env, $string) . '</em>';
+  }
+
   /**
    * Overrides twig_escape_filter().
    *

core/modules/system/src/Tests/Theme/TwigTransTest.php

@@ -7,7 +7,9 @@
 
 namespace Drupal\system\Tests\Theme;
 
+use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Core\Language\LanguageInterface;
+use Drupal\Core\Url;
 use Drupal\language\Entity\ConfigurableLanguage;
 use Drupal\simpletest\WebTestBase;
 
@@ -175,6 +177,20 @@ public function testTwigTransDebug() {
     $this->checkForDebugMarkup(TRUE);
   }
 
+  /**
+   * Tests rendering a placeholder outside of translate.
+   *
+   * This test ensures that the security problem described in
+   * https://www.drupal.org/node/2495179 doesn't exist.
+   */
+  public function testPlaceholderOutsideOfTrans() {
+    $this->drupalGet(Url::fromRoute('twig_theme_test.placeholder_outside_trans'));
+
+    $script = '<script>alert(123);</script>';
+    $this->assertNoRaw($script);
+    $this->assertEqual(2, substr_count($this->getRawContent(), '<em class="placeholder">' . SafeMarkup::checkPlain($script) . '</em>'));
+  }
+
   /**
    * Helper function: test twig debug translation markup.
    *

core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php

@@ -30,6 +30,16 @@ public function transBlockRender() {
     );
   }
 
+  /**
+   * Controller for testing the twig placeholder filter outside of {% trans %}
+   */
+  public function placeholderOutsideTransRender() {
+    return [
+      '#theme' => 'twig_theme_test_placeholder_outside_trans',
+      '#var' => '<script>alert(123);</script>',
+    ];
+  }
+
   /**
    * Renders for testing url_generator functions in a Twig template.
    */

core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.placeholder_outside_trans.html.twig

+Placeholder outside trans: {{ var | placeholder }}
+
+{% trans %}
+  Placeholder inside trans: {{ var | placeholder }}
+{% endtrans %}

core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module

@@ -15,6 +15,10 @@ function twig_theme_test_theme($existing, $type, $theme, $path) {
     'variables' => array(),
     'template' => 'twig_theme_test.trans',
   );
+  $items['twig_theme_test_placeholder_outside_trans'] = array(
+    'variables' => array('var' => ''),
+    'template' => 'twig_theme_test.placeholder_outside_trans',
+  );
   $items['twig_namespace_test'] = array(
     'variables' => array(),
     'template' => 'twig_namespace_test',

core/modules/system/tests/modules/twig_theme_test/twig_theme_test.routing.yml

@@ -12,6 +12,13 @@ twig_theme_test.trans:
   requirements:
     _access: 'TRUE'
 
+twig_theme_test.placeholder_outside_trans:
+  path: '/twig-theme-test/placeholder_outside_trans'
+  defaults:
+    _controller: '\Drupal\twig_theme_test\TwigThemeTestController::placeholderOutsideTransRender'
+  requirements:
+    _access: 'TRUE'
+
 twig_theme_test_url_generator:
   path: '/twig-theme-test/url-generator'
   defaults: