From 3b75e8ef878fe54d1991a376df4e8b4c00c7f45f Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Thu, 29 Sep 2022 08:49:39 +0100
Subject: [PATCH] SA-CORE-2022-016 by fabpot, nicolas.grekas, xjm, lauriii,
alexpott, Berdir, larowlan, catch, longwave, cilefen, james.williams,
benjifisher
---
composer.lock | 16 +-
.../Metapackage/CoreRecommended/composer.json | 2 +-
.../scaffold/files/default.services.yml | 15 ++
core/composer.json | 2 +-
core/core.services.yml | 8 +-
.../Core/Template/Loader/FilesystemLoader.php | 55 +++++-
.../help_topics/src/HelpTopicTwigLoader.php | 14 ++
.../src/Kernel/Theme/TwigIncludeTest.php | 157 ++++++++++++++++++
sites/default/default.services.yml | 15 ++
9 files changed, 272 insertions(+), 12 deletions(-)
create mode 100644 core/modules/system/tests/src/Kernel/Theme/TwigIncludeTest.php
diff --git a/composer.lock b/composer.lock
index 9f3bec19764a..a0d6ebf9a95b 100644
--- a/composer.lock
+++ b/composer.lock
@@ -527,7 +527,7 @@
"dist": {
"type": "path",
"url": "core",
- "reference": "90de8ecf3eaee50ae38ec7c7b67a7a660dde50f4"
+ "reference": "153be65a5b89f9bf2826ac2ff6339a7528a37d96"
},
"require": {
"asm89/stack-cors": "^1.3",
@@ -572,7 +572,7 @@
"symfony/translation": "^4.4",
"symfony/validator": "^4.4",
"symfony/yaml": "^4.4.19",
- "twig/twig": "^2.15",
+ "twig/twig": "^2.15.3",
"typo3/phar-stream-wrapper": "^3.1.3"
},
"conflict": {
@@ -4377,16 +4377,16 @@
},
{
"name": "twig/twig",
- "version": "v2.15.2",
+ "version": "v2.15.3",
"source": {
"type": "git",
"url": "https://github.com/twigphp/Twig.git",
- "reference": "3e43405a9a8b578809426339cc3780e16fba0c52"
+ "reference": "ab402673db8746cb3a4c46f3869d6253699f614a"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/twigphp/Twig/zipball/3e43405a9a8b578809426339cc3780e16fba0c52",
- "reference": "3e43405a9a8b578809426339cc3780e16fba0c52",
+ "url": "https://api.github.com/repos/twigphp/Twig/zipball/ab402673db8746cb3a4c46f3869d6253699f614a",
+ "reference": "ab402673db8746cb3a4c46f3869d6253699f614a",
"shasum": ""
},
"require": {
@@ -4441,7 +4441,7 @@
],
"support": {
"issues": "https://github.com/twigphp/Twig/issues",
- "source": "https://github.com/twigphp/Twig/tree/v2.15.2"
+ "source": "https://github.com/twigphp/Twig/tree/v2.15.3"
},
"funding": [
{
@@ -4453,7 +4453,7 @@
"type": "tidelift"
}
],
- "time": "2022-08-12T06:43:37+00:00"
+ "time": "2022-09-28T08:40:08+00:00"
},
{
"name": "typo3/phar-stream-wrapper",
diff --git a/composer/Metapackage/CoreRecommended/composer.json b/composer/Metapackage/CoreRecommended/composer.json
index 7d0d789ac994..68d1a901e1ce 100644
--- a/composer/Metapackage/CoreRecommended/composer.json
+++ b/composer/Metapackage/CoreRecommended/composer.json
@@ -61,7 +61,7 @@
"symfony/validator": "~v4.4.45",
"symfony/var-dumper": "~v5.4.11",
"symfony/yaml": "~v4.4.45",
- "twig/twig": "~v2.15.2",
+ "twig/twig": "~v2.15.3",
"typo3/phar-stream-wrapper": "~v3.1.7"
}
}
diff --git a/core/assets/scaffold/files/default.services.yml b/core/assets/scaffold/files/default.services.yml
index 8c7f05dcfd4b..0b3b7414895e 100644
--- a/core/assets/scaffold/files/default.services.yml
+++ b/core/assets/scaffold/files/default.services.yml
@@ -93,6 +93,21 @@ parameters:
# Disabling the Twig cache is not recommended in production environments.
# @default true
cache: true
+ # File extensions:
+ #
+ # List of file extensions the Twig system is allowed to load via the
+ # twig.loader.filesystem service. Files with other extensions will not be
+ # loaded unless they are added here. For example, to allow a file named
+ # 'example.partial' to be loaded, add 'partial' to this list. To load files
+ # with no extension, add an empty string '' to the list.
+ #
+ # @default ['css', 'html', 'js', 'svg', 'twig']
+ allowed_file_extensions:
+ - css
+ - html
+ - js
+ - svg
+ - twig
renderer.config:
# Renderer required cache contexts:
#
diff --git a/core/composer.json b/core/composer.json
index 90185f4a00fb..f0c939038049 100644
--- a/core/composer.json
+++ b/core/composer.json
@@ -33,7 +33,7 @@
"symfony/polyfill-php80": "^1.26",
"symfony/yaml": "^4.4.19",
"typo3/phar-stream-wrapper": "^3.1.3",
- "twig/twig": "^2.15",
+ "twig/twig": "^2.15.3",
"doctrine/reflection": "^1.2",
"doctrine/annotations": "^1.13",
"guzzlehttp/guzzle": "^6.5.8 || ^7.4.5",
diff --git a/core/core.services.yml b/core/core.services.yml
index 83febfe533e9..b0c3d13866c6 100644
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -15,6 +15,12 @@ parameters:
debug: false
auto_reload: null
cache: true
+ allowed_file_extensions:
+ - css
+ - html
+ - js
+ - svg
+ - twig
renderer.config:
required_cache_contexts: ['languages:language_interface', 'theme', 'user.permissions']
auto_placeholder_conditions:
@@ -1691,7 +1697,7 @@ services:
# We use '.' instead of '%app.root%' as the path for non-namespaced template
# files so that they match the relative paths of templates loaded via the
# theme registry or via Twig namespaces.
- arguments: ['.', '@module_handler', '@theme_handler']
+ arguments: ['.', '@module_handler', '@theme_handler', '%twig.config%']
tags:
- { name: twig.loader, priority: 100 }
twig.loader.theme_registry:
diff --git a/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php b/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
index 1864b3e128c4..127a94ff0319 100644
--- a/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
+++ b/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
@@ -4,6 +4,7 @@
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Extension\ThemeHandlerInterface;
+use Twig\Error\LoaderError;
use Twig\Loader\FilesystemLoader as TwigFilesystemLoader;
/**
@@ -15,6 +16,13 @@
*/
class FilesystemLoader extends TwigFilesystemLoader {
+ /**
+ * Allowed file extensions.
+ *
+ * @var string[]
+ */
+ protected $allowedFileExtensions = ['css', 'html', 'js', 'svg', 'twig'];
+
/**
* Constructs a new FilesystemLoader object.
*
@@ -24,8 +32,10 @@ class FilesystemLoader extends TwigFilesystemLoader {
* The module handler service.
* @param \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler
* The theme handler service.
+ * @param mixed[] $twig_config
+ * Twig configuration from the service container.
*/
- public function __construct($paths, ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler) {
+ public function __construct($paths, ModuleHandlerInterface $module_handler, ThemeHandlerInterface $theme_handler, array $twig_config = []) {
parent::__construct($paths);
// Add namespaced paths for modules and themes.
@@ -39,6 +49,15 @@ public function __construct($paths, ModuleHandlerInterface $module_handler, Them
foreach ($namespaces as $name => $path) {
$this->addPath($path . '/templates', $name);
+ // Allow accessing the root of an extension by using the namespace without
+ // using directory traversal from the `/templates` directory.
+ $this->addPath($path, $name);
+ }
+ if (!empty($twig_config['allowed_file_extensions'])) {
+ // Provide a safe fallback for sites that have not updated their
+ // services.yml file or rebuilt the container, as well as for child
+ // classes.
+ $this->allowedFileExtensions = $twig_config['allowed_file_extensions'];
}
}
@@ -56,4 +75,38 @@ public function addPath($path, $namespace = self::MAIN_NAMESPACE) {
$this->paths[$namespace][] = rtrim($path, '/\\');
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function findTemplate($name, $throw = TRUE) {
+ $extension = pathinfo($name, PATHINFO_EXTENSION);
+ if (!in_array($extension, $this->allowedFileExtensions, TRUE)) {
+ if (!$throw) {
+ return NULL;
+ }
+ // Customize the list of extensions if no file extension is allowed.
+ $extensions = $this->allowedFileExtensions;
+ $no_extension = array_search('', $extensions, TRUE);
+ if (is_int($no_extension)) {
+ unset($extensions[$no_extension]);
+ $extensions[] = 'or no file extension';
+ }
+ if (empty($extension)) {
+ $extension = 'no file extension';
+ }
+ throw new LoaderError(sprintf("Template %s has an invalid file extension (%s). Only templates ending in one of %s are allowed. Set the twig.config.allowed_file_extensions container parameter to customize the allowed file extensions", $name, $extension, implode(', ', $extensions)));
+ }
+
+ // Previously it was possible to access files in the parent directory of a
+ // namespace. This was removed in Twig 2.15.3. In order to support backwards
+ // compatibility, we are adding path directory as a namespace, and therefore
+ // we can remove the directory traversal from the name.
+ // @todo deprecate this functionality for removal in Drupal 11.
+ if (preg_match('/(^\@[^\/]+\/)\.\.\/(.*)/', $name, $matches)) {
+ $name = $matches[1] . $matches[2];
+ }
+
+ return parent::findTemplate($name, $throw);
+ }
+
}
diff --git a/core/modules/help_topics/src/HelpTopicTwigLoader.php b/core/modules/help_topics/src/HelpTopicTwigLoader.php
index cdfeaf616f42..aa6c8c4a04cd 100644
--- a/core/modules/help_topics/src/HelpTopicTwigLoader.php
+++ b/core/modules/help_topics/src/HelpTopicTwigLoader.php
@@ -95,4 +95,18 @@ public function getSourceContext($name) {
return new Source($contents, $name, $path);
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function findTemplate($name, $throw = TRUE) {
+ if (!str_ends_with($name, '.html.twig')) {
+ if (!$throw) {
+ return NULL;
+ }
+ $extension = pathinfo($name, PATHINFO_EXTENSION);
+ throw new LoaderError(sprintf("Help topic %s has an invalid file extension (%s). Only help topics ending .html.twig are allowed.", $name, $extension));
+ }
+ return parent::findTemplate($name, $throw);
+ }
+
}
diff --git a/core/modules/system/tests/src/Kernel/Theme/TwigIncludeTest.php b/core/modules/system/tests/src/Kernel/Theme/TwigIncludeTest.php
new file mode 100644
index 000000000000..c97e606ad1de
--- /dev/null
+++ b/core/modules/system/tests/src/Kernel/Theme/TwigIncludeTest.php
@@ -0,0 +1,157 @@
+<?php
+
+namespace Drupal\Tests\system\Kernel\Theme;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\KernelTests\KernelTestBase;
+use Twig\Error\LoaderError;
+
+/**
+ * Tests including files in Twig templates.
+ *
+ * @group Theme
+ */
+class TwigIncludeTest extends KernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['system'];
+
+ /**
+ * The Twig configuration to set the container parameter to during rebuilds.
+ *
+ * @var array
+ */
+ private $twigConfig = [];
+
+ /**
+ * Tests template inclusion extension checking.
+ *
+ * @see \Drupal\Core\Template\Loader\FilesystemLoader::findTemplate()
+ */
+ public function testTemplateInclusion(): void {
+ $this->enableModules(['system']);
+ /** @var \Drupal\Core\Render\RendererInterface $renderer */
+ $renderer = \Drupal::service('renderer');
+
+ $element['test'] = [
+ '#type' => 'inline_template',
+ '#template' => "{% include '@system/container.html.twig' %}",
+ ];
+ $this->assertEquals("<div></div>\n", $renderer->renderRoot($element));
+
+ // Test that SQL files cannot be included in Twig templates by default.
+ $element = [];
+ $element['test'] = [
+ '#type' => 'inline_template',
+ '#template' => "{% include '@__main__\/core/tests/fixtures/files/sql-2.sql' %}",
+ ];
+ try {
+ $renderer->renderRoot($element);
+ $this->fail('Expected exception not thrown');
+ }
+ catch (LoaderError $e) {
+ $this->assertStringContainsString('Template "@__main__/core/tests/fixtures/files/sql-2.sql" is not defined', $e->getMessage());
+ }
+ /** @var \Drupal\Core\Template\Loader\FilesystemLoader $loader */
+ $loader = \Drupal::service('twig.loader.filesystem');
+ try {
+ $loader->getSourceContext('@__main__\/core/tests/fixtures/files/sql-2.sql');
+ $this->fail('Expected exception not thrown');
+ }
+ catch (LoaderError $e) {
+ $this->assertStringContainsString('Template @__main__\/core/tests/fixtures/files/sql-2.sql has an invalid file extension (sql). Only templates ending in one of css, html, js, svg, twig are allowed. Set the twig.config.allowed_file_extensions container parameter to customize the allowed file extensions', $e->getMessage());
+ }
+
+ // Allow SQL files to be included.
+ $twig_config = $this->container->getParameter('twig.config');
+ $twig_config['allowed_file_extensions'][] = 'sql';
+ $this->twigConfig = $twig_config;
+ $this->container->get('kernel')->shutdown();
+ $this->container->get('kernel')->boot();
+ /** @var \Drupal\Core\Template\Loader\FilesystemLoader $loader */
+ $loader = \Drupal::service('twig.loader.filesystem');
+ $source = $loader->getSourceContext('@__main__\/core/tests/fixtures/files/sql-2.sql');
+ $this->assertSame(file_get_contents('core/tests/fixtures/files/sql-2.sql'), $source->getCode());
+
+ // Test the fallback to the default list of extensions provided by the
+ // class.
+ $this->assertSame(['css', 'html', 'js', 'svg', 'twig', 'sql'], \Drupal::getContainer()->getParameter('twig.config')['allowed_file_extensions']);
+ unset($twig_config['allowed_file_extensions']);
+ $this->twigConfig = $twig_config;
+ $this->container->get('kernel')->shutdown();
+ $this->container->get('kernel')->boot();
+ $this->assertArrayNotHasKey('allowed_file_extensions', \Drupal::getContainer()->getParameter('twig.config'));
+ /** @var \Drupal\Core\Template\Loader\FilesystemLoader $loader */
+ $loader = \Drupal::service('twig.loader.filesystem');
+ try {
+ $loader->getSourceContext('@__main__\/core/tests/fixtures/files/sql-2.sql');
+ $this->fail('Expected exception not thrown');
+ }
+ catch (LoaderError $e) {
+ $this->assertStringContainsString('Template @__main__\/core/tests/fixtures/files/sql-2.sql has an invalid file extension (sql). Only templates ending in one of css, html, js, svg, twig are allowed. Set the twig.config.allowed_file_extensions container parameter to customize the allowed file extensions', $e->getMessage());
+ }
+
+ // Test a file with no extension.
+ file_put_contents($this->siteDirectory . '/test_file', 'This is a test!');
+ /** @var \Drupal\Core\Template\Loader\FilesystemLoader $loader */
+ $loader = \Drupal::service('twig.loader.filesystem');
+ try {
+ $loader->getSourceContext('@__main__\/' . $this->siteDirectory . '/test_file');
+ $this->fail('Expected exception not thrown');
+ }
+ catch (LoaderError $e) {
+ $this->assertStringContainsString('test_file has an invalid file extension (no file extension). Only templates ending in one of css, html, js, svg, twig are allowed. Set the twig.config.allowed_file_extensions container parameter to customize the allowed file extensions', $e->getMessage());
+ }
+
+ // Allow files with no extension.
+ $twig_config['allowed_file_extensions'] = ['twig', ''];
+ $this->twigConfig = $twig_config;
+ $this->container->get('kernel')->shutdown();
+ $this->container->get('kernel')->boot();
+ /** @var \Drupal\Core\Template\Loader\FilesystemLoader $loader */
+ $loader = \Drupal::service('twig.loader.filesystem');
+ $source = $loader->getSourceContext('@__main__\/' . $this->siteDirectory . '/test_file');
+ $this->assertSame('This is a test!', $source->getCode());
+
+ // Ensure the error message makes sense when no file extension is allowed.
+ try {
+ $loader->getSourceContext('@__main__\/core/tests/fixtures/files/sql-2.sql');
+ $this->fail('Expected exception not thrown');
+ }
+ catch (LoaderError $e) {
+ $this->assertStringContainsString('Template @__main__\/core/tests/fixtures/files/sql-2.sql has an invalid file extension (sql). Only templates ending in one of twig, or no file extension are allowed. Set the twig.config.allowed_file_extensions container parameter to customize the allowed file extensions', $e->getMessage());
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function register(ContainerBuilder $container) {
+ parent::register($container);
+ if (!empty($this->twigConfig)) {
+ $container->setParameter('twig.config', $this->twigConfig);
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUpFilesystem(): void {
+ // Use a real file system and not VFS so that we can include files from the
+ // site using @__main__ in a template.
+ $public_file_directory = $this->siteDirectory . '/files';
+ $private_file_directory = $this->siteDirectory . '/private';
+
+ mkdir($this->siteDirectory, 0775);
+ mkdir($this->siteDirectory . '/files', 0775);
+ mkdir($this->siteDirectory . '/private', 0775);
+ mkdir($this->siteDirectory . '/files/config/sync', 0775, TRUE);
+
+ $this->setSetting('file_public_path', $public_file_directory);
+ $this->setSetting('file_private_path', $private_file_directory);
+ $this->setSetting('config_sync_directory', $this->siteDirectory . '/files/config/sync');
+ }
+
+}
diff --git a/sites/default/default.services.yml b/sites/default/default.services.yml
index 8c7f05dcfd4b..0b3b7414895e 100644
--- a/sites/default/default.services.yml
+++ b/sites/default/default.services.yml
@@ -93,6 +93,21 @@ parameters:
# Disabling the Twig cache is not recommended in production environments.
# @default true
cache: true
+ # File extensions:
+ #
+ # List of file extensions the Twig system is allowed to load via the
+ # twig.loader.filesystem service. Files with other extensions will not be
+ # loaded unless they are added here. For example, to allow a file named
+ # 'example.partial' to be loaded, add 'partial' to this list. To load files
+ # with no extension, add an empty string '' to the list.
+ #
+ # @default ['css', 'html', 'js', 'svg', 'twig']
+ allowed_file_extensions:
+ - css
+ - html
+ - js
+ - svg
+ - twig
renderer.config:
# Renderer required cache contexts:
#
--
GitLab