From 0ca6cc73e2e91b4cbf41946289cb3b512111229b Mon Sep 17 00:00:00 2001
From: xjm <xjm@65776.no-reply.drupal.org>
Date: Wed, 28 Sep 2022 11:52:42 -0500
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 96cba4810e46..2a6b378d3485 100644
--- a/composer.lock
+++ b/composer.lock
@@ -443,7 +443,7 @@
"dist": {
"type": "path",
"url": "core",
- "reference": "9017fb353204d5f0d92ef3bc7f0ac28c065684d2"
+ "reference": "a75d3d082c27d60cdb0c87f56e968c36538800e3"
},
"require": {
"asm89/stack-cors": "^2.1",
@@ -482,7 +482,7 @@
"symfony/serializer": "^6.1",
"symfony/validator": "^6.1",
"symfony/yaml": "^6.1",
- "twig/twig": "^3.4"
+ "twig/twig": "^3.4.3"
},
"conflict": {
"drush/drush": "<8.1.10"
@@ -3876,16 +3876,16 @@
},
{
"name": "twig/twig",
- "version": "v3.4.2",
+ "version": "v3.4.3",
"source": {
"type": "git",
"url": "https://github.com/twigphp/Twig.git",
- "reference": "e07cdd3d430cd7e453c31b36eb5ad6c0c5e43077"
+ "reference": "c38fd6b0b7f370c198db91ffd02e23b517426b58"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/twigphp/Twig/zipball/e07cdd3d430cd7e453c31b36eb5ad6c0c5e43077",
- "reference": "e07cdd3d430cd7e453c31b36eb5ad6c0c5e43077",
+ "url": "https://api.github.com/repos/twigphp/Twig/zipball/c38fd6b0b7f370c198db91ffd02e23b517426b58",
+ "reference": "c38fd6b0b7f370c198db91ffd02e23b517426b58",
"shasum": ""
},
"require": {
@@ -3936,7 +3936,7 @@
],
"support": {
"issues": "https://github.com/twigphp/Twig/issues",
- "source": "https://github.com/twigphp/Twig/tree/v3.4.2"
+ "source": "https://github.com/twigphp/Twig/tree/v3.4.3"
},
"funding": [
{
@@ -3948,7 +3948,7 @@
"type": "tidelift"
}
],
- "time": "2022-08-12T06:47:24+00:00"
+ "time": "2022-09-28T08:42:51+00:00"
}
],
"packages-dev": [
diff --git a/composer/Metapackage/CoreRecommended/composer.json b/composer/Metapackage/CoreRecommended/composer.json
index cb4447075ccf..dec7739d9f84 100644
--- a/composer/Metapackage/CoreRecommended/composer.json
+++ b/composer/Metapackage/CoreRecommended/composer.json
@@ -54,6 +54,6 @@
"symfony/validator": "~v6.1.4",
"symfony/var-dumper": "~v6.1.3",
"symfony/yaml": "~v6.1.4",
- "twig/twig": "~v3.4.2"
+ "twig/twig": "~v3.4.3"
}
}
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 25c3781e1dc4..47e5279b123c 100644
--- a/core/composer.json
+++ b/core/composer.json
@@ -30,7 +30,7 @@
"symfony/process": "^6.1",
"symfony/polyfill-iconv": "^1.26",
"symfony/yaml": "^6.1",
- "twig/twig": "^3.4",
+ "twig/twig": "^3.4.3",
"doctrine/annotations": "^1.13",
"guzzlehttp/guzzle": "^7.5",
"guzzlehttp/psr7": "^2.4",
diff --git a/core/core.services.yml b/core/core.services.yml
index c5a0fbbfc522..e161046a82e3 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:
@@ -1559,7 +1565,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 6f4ddc11a753..e9620f6a57ba 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(string $path, string $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 ccf334019627..7f04f8f29f3a 100644
--- a/core/modules/help_topics/src/HelpTopicTwigLoader.php
+++ b/core/modules/help_topics/src/HelpTopicTwigLoader.php
@@ -95,4 +95,18 @@ public function getSourceContext(string $name): Source {
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