From 02249b0c8bed84b1bbe93345b56f4ca3e5f81c2f Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 18 Jun 2026 23:27:42 -0400 Subject: [PATCH 1/4] Fix `include` tag being vulnerable to path traversal Ensure that all relative and absolute file paths are contained by a template path. We're looking at the string values so that any `../` tricks don't work. Thanks to Volker Dusch and the PHP Ecosystem security team for reporting this. --- src/Twig/FileLoader.php | 39 ++++++++++++++++--- tests/TestCase/Twig/FileLoaderTest.php | 11 +++--- tests/TestCase/View/TwigViewTest.php | 12 ++++++ .../templates/template_path_restriction.twig | 5 +++ 4 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 tests/test_app/templates/template_path_restriction.twig diff --git a/src/Twig/FileLoader.php b/src/Twig/FileLoader.php index 5874766..19af51c 100644 --- a/src/Twig/FileLoader.php +++ b/src/Twig/FileLoader.php @@ -92,8 +92,26 @@ public function exists(string $name) */ public function findTemplate(string $name): string { + $templatePaths = App::path('templates'); if (file_exists($name)) { - return $name; + $name = str_replace('//', '/', $name); + // Check both app template paths and all plugins, + // as template from element() end up here too. + // We also need to protect against `{% include var_name %}` + // where var_name is request data. + foreach ($templatePaths as $templatePath) { + if (str_starts_with($name, $templatePath)) { + return $name; + } + } + foreach (Plugin::loaded() as $pluginName) { + $pluginPath = Plugin::templatePath($pluginName); + if (str_starts_with($name, $pluginPath)) { + return $name; + } + } + + throw $this->loaderError($name, $templatePaths); } [$plugin, $name] = pluginSplit($name); @@ -105,24 +123,35 @@ public function findTemplate(string $name): string if ($path !== null) { return $path; } - $error = "Could not find template `{$name}` in plugin `{$plugin}` in these paths:\n\n" . "- `{$templatePath}`\n"; throw new LoaderError($error); } - foreach (App::path('templates') as $templatePath) { + foreach ($templatePaths as $templatePath) { $path = $this->checkExtensions($templatePath . $name); if ($path !== null) { return $path; } } + throw $this->loaderError($name, $templatePaths); + } + /** + * Create a LoaderError with template path list in the message. + * + * @param string $name The name of the template that could not be found. + * @param array $templatePaths List of template paths that were searched + * @return \Twig\Error\LoaderError + */ + protected function loaderError(string $name, array $templatePaths): LoaderError + { $error = "Could not find template `{$name}` in these paths:\n\n"; - foreach (App::path('templates') as $templatePath) { + foreach ($templatePaths as $templatePath) { $error .= "- `{$templatePath}`\n"; } - throw new LoaderError($error); + + return new LoaderError($error); } /** diff --git a/tests/TestCase/Twig/FileLoaderTest.php b/tests/TestCase/Twig/FileLoaderTest.php index dcfdff2..fd289ed 100644 --- a/tests/TestCase/Twig/FileLoaderTest.php +++ b/tests/TestCase/Twig/FileLoaderTest.php @@ -81,13 +81,14 @@ public function testGetCacheKeyPluginNonExistingFile() public function testIsFresh() { - file_put_contents(TMP . 'TwigViewIsFreshTest', 'TwigViewIsFreshTest'); - $time = filemtime(TMP . 'TwigViewIsFreshTest'); + $path = TEST_APP . 'templates/test_is_fresh.twig'; + file_put_contents($path, 'is fresh test'); + $time = filemtime($path); - $this->assertTrue($this->loader->isFresh(TMP . 'TwigViewIsFreshTest', $time + 5)); - $this->assertTrue(!$this->loader->isFresh(TMP . 'TwigViewIsFreshTest', $time - 5)); + $this->assertTrue($this->loader->isFresh($path, $time + 5)); + $this->assertTrue(!$this->loader->isFresh($path, $time - 5)); - unlink(TMP . 'TwigViewIsFreshTest'); + unlink($path); } public function testIsFreshNonExistingFile() diff --git a/tests/TestCase/View/TwigViewTest.php b/tests/TestCase/View/TwigViewTest.php index 13bf538..63e1762 100644 --- a/tests/TestCase/View/TwigViewTest.php +++ b/tests/TestCase/View/TwigViewTest.php @@ -20,6 +20,7 @@ use Cake\TestSuite\TestCase; use TestApp\View\AppView; +use Twig\Error\LoaderError; use Twig\Error\RuntimeError; use Twig\Error\SyntaxError; use Twig\Extra\Markdown\DefaultMarkdown; @@ -242,6 +243,17 @@ public function testThrowSyntaxError() $this->view->render('syntaxerror', false); } + public function testTemplatePathRestriction() + { + $path = TMP . 'secret.txt'; + file_put_contents($path, 'SECRET DATA from /tmp/secret.txt'); + $view = new AppView(); + $view->set('item', $path); + $this->expectException(LoaderError::class); + $this->expectExceptionMessage('Could not find template'); + $view->render('template_path_restriction'); + } + public function testHelperFunction() { $view = new AppView(null, null, null, [ diff --git a/tests/test_app/templates/template_path_restriction.twig b/tests/test_app/templates/template_path_restriction.twig new file mode 100644 index 0000000..c672344 --- /dev/null +++ b/tests/test_app/templates/template_path_restriction.twig @@ -0,0 +1,5 @@ +{# +Ensure that `include` applies template path restriction, so that user data + include don't open +up arbitrary file reads. +#} +{% include item %} \ No newline at end of file From 471566f829a8ded802524a7e9e2b0230564bcbf9 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sat, 20 Jun 2026 00:25:53 -0400 Subject: [PATCH 2/4] Fix deserialization of untrusted data weakness (#119) The unserialize filter has a weakness to arbitrary class usage which can be combined with user input to create unserialization gadgets which are used in RCE vulnerability chains. I've also chosen to deprecate these functions. I see no reason to continue having them when they have so many sharp edges. Applications relying on these features, can add their own filters. Thanks to Volker Dusch and the PHP Ecosystem security team for reporting this. --- src/Twig/Extension/UtilsExtension.php | 13 ++++- .../Twig/Extension/UtilsExtensionTest.php | 55 +++++++++++++++++++ tests/test_app/src/GadgetMarker.php | 15 +++++ 3 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 tests/TestCase/Twig/Extension/UtilsExtensionTest.php create mode 100644 tests/test_app/src/GadgetMarker.php diff --git a/src/Twig/Extension/UtilsExtension.php b/src/Twig/Extension/UtilsExtension.php index 91a7ce7..08f9136 100644 --- a/src/Twig/Extension/UtilsExtension.php +++ b/src/Twig/Extension/UtilsExtension.php @@ -20,6 +20,7 @@ use Twig\Extension\AbstractExtension; use Twig\TwigFilter; +use function Cake\Core\deprecationWarning; /** * Class UtilsExtension. @@ -34,8 +35,16 @@ class UtilsExtension extends AbstractExtension public function getFilters(): array { return [ - new TwigFilter('serialize', 'serialize'), - new TwigFilter('unserialize', 'unserialize'), + new TwigFilter('serialize', function (string $value): mixed { + deprecationWarning('5.0.2', 'Usage of serialize in templates deprecated.'); + + return serialize($value); + }), + new TwigFilter('unserialize', function (string $value): mixed { + deprecationWarning('5.0.2', 'unserialize is deprecated. Its usage creates arbitrary object deserialization issues'); + + return unserialize($value, ['allowed_classes' => false]); + }), new TwigFilter('md5', 'md5'), new TwigFilter('base64_encode', 'base64_encode'), new TwigFilter('base64_decode', 'base64_decode'), diff --git a/tests/TestCase/Twig/Extension/UtilsExtensionTest.php b/tests/TestCase/Twig/Extension/UtilsExtensionTest.php new file mode 100644 index 0000000..f9c1784 --- /dev/null +++ b/tests/TestCase/Twig/Extension/UtilsExtensionTest.php @@ -0,0 +1,55 @@ +extension = new UtilsExtension(); + } + + public function testUnserializePreventObject(): void + { + $this->skipIf(PHP_VERSION_ID < 80300, 'Requires PHP8.3 or higher'); + + $twig = new Environment(new ArrayLoader([ + // {% set %} so we exercise the filter without stringifying the result. + 'object' => '{% set _ = payload|unserialize %}(rendered)', + 'array' => '{{ (payload|unserialize)["role"] }}', + ])); + $twig->addExtension(new UtilsExtension()); + + // 1) Object payload: does a gadget's magic method run? + GadgetMarker::$woken = false; + $this->deprecated(function () use ($twig): void { + $twig->render('object', ['payload' => serialize(new GadgetMarker())]); + $this->assertFalse(GadgetMarker::$woken, 'Should not have modified GadgetMarker'); + + $out = $twig->render('array', ['payload' => serialize(['role' => 'editor'])]); + $this->assertStringContainsString('editor', $out); + }); + } +} diff --git a/tests/test_app/src/GadgetMarker.php b/tests/test_app/src/GadgetMarker.php new file mode 100644 index 0000000..a159879 --- /dev/null +++ b/tests/test_app/src/GadgetMarker.php @@ -0,0 +1,15 @@ + Date: Sat, 20 Jun 2026 23:26:30 -0400 Subject: [PATCH 3/4] Open OS range --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a98fce..6d0be45 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: jobs: testsuite: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: From 1bd48ce2adb540f175743a2db5b44713a9268904 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sat, 20 Jun 2026 23:28:49 -0400 Subject: [PATCH 4/4] Update for older phpunit --- tests/TestCase/Twig/Extension/UtilsExtensionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestCase/Twig/Extension/UtilsExtensionTest.php b/tests/TestCase/Twig/Extension/UtilsExtensionTest.php index f9c1784..e574895 100644 --- a/tests/TestCase/Twig/Extension/UtilsExtensionTest.php +++ b/tests/TestCase/Twig/Extension/UtilsExtensionTest.php @@ -25,7 +25,7 @@ class UtilsExtensionTest extends AbstractExtensionTest { - protected function setUp(): void + public function setUp(): void { parent::setUp(); $this->extension = new UtilsExtension();