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: 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/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/Extension/UtilsExtensionTest.php b/tests/TestCase/Twig/Extension/UtilsExtensionTest.php new file mode 100644 index 0000000..e574895 --- /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/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/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 @@ +