diff --git a/changelog/GHSA-4rmg-292m-wg3w.md b/changelog/GHSA-4rmg-292m-wg3w.md new file mode 100644 index 00000000..cd15f3e6 --- /dev/null +++ b/changelog/GHSA-4rmg-292m-wg3w.md @@ -0,0 +1 @@ +- Fixed a code injection vulnerability in extends-tag. This addresses CVE-2024-35226. diff --git a/libs/sysplugins/smarty_internal_compile_extends.php b/libs/sysplugins/smarty_internal_compile_extends.php index d72d2b76..69a7b552 100644 --- a/libs/sysplugins/smarty_internal_compile_extends.php +++ b/libs/sysplugins/smarty_internal_compile_extends.php @@ -30,7 +30,7 @@ class Smarty_Internal_Compile_Extends extends Smarty_Internal_Compile_Shared_Inh * * @var array */ - public $optional_attributes = array('extends_resource'); + public $optional_attributes = array(); /** * Attribute definition: Overwrites base class. @@ -62,29 +62,7 @@ class Smarty_Internal_Compile_Extends extends Smarty_Internal_Compile_Shared_Inh } // add code to initialize inheritance $this->registerInit($compiler, true); - $file = trim($_attr[ 'file' ], '\'"'); - if (strlen($file) > 8 && substr($file, 0, 8) === 'extends:') { - // generate code for each template - $files = array_reverse(explode('|', substr($file, 8))); - $i = 0; - foreach ($files as $file) { - if ($file[ 0 ] === '"') { - $file = trim($file, '".'); - } else { - $file = "'{$file}'"; - } - $i++; - if ($i === count($files) && isset($_attr[ 'extends_resource' ])) { - $this->compileEndChild($compiler); - } - $this->compileInclude($compiler, $file); - } - if (!isset($_attr[ 'extends_resource' ])) { - $this->compileEndChild($compiler); - } - } else { - $this->compileEndChild($compiler, $_attr[ 'file' ]); - } + $this->compileEndChild($compiler, $_attr[ 'file' ]); $compiler->has_code = false; return ''; } @@ -115,44 +93,4 @@ class Smarty_Internal_Compile_Extends extends Smarty_Internal_Compile_Shared_Inh '') . ");\n?>" ); } - - /** - * Add code for including subtemplate to end of template - * - * @param \Smarty_Internal_TemplateCompilerBase $compiler - * @param string $template subtemplate name - * - * @throws \SmartyCompilerException - * @throws \SmartyException - */ - private function compileInclude(Smarty_Internal_TemplateCompilerBase $compiler, $template) - { - $compiler->parser->template_postfix[] = new Smarty_Internal_ParseTree_Tag( - $compiler->parser, - $compiler->compileTag( - 'include', - array( - $template, - array('scope' => 'parent') - ) - ) - ); - } - - /** - * Create source code for {extends} from source components array - * - * @param \Smarty_Internal_Template $template - * - * @return string - */ - public static function extendsSourceArrayCode(Smarty_Internal_Template $template) - { - $resources = array(); - foreach ($template->source->components as $source) { - $resources[] = $source->resource; - } - return $template->smarty->left_delimiter . 'extends file=\'extends:' . join('|', $resources) . - '\' extends_resource=true' . $template->smarty->right_delimiter; - } } diff --git a/libs/sysplugins/smarty_internal_templatecompilerbase.php b/libs/sysplugins/smarty_internal_templatecompilerbase.php index f5d2c438..8ff3e052 100644 --- a/libs/sysplugins/smarty_internal_templatecompilerbase.php +++ b/libs/sysplugins/smarty_internal_templatecompilerbase.php @@ -455,15 +455,29 @@ abstract class Smarty_Internal_TemplateCompilerBase $this->smarty->_current_file = $this->template->source->filepath; // get template source if (!empty($this->template->source->components)) { - // we have array of inheritance templates by extends: resource - // generate corresponding source code sequence - $_content = - Smarty_Internal_Compile_Extends::extendsSourceArrayCode($this->template); + $_compiled_code = '_loadInheritance(); $_smarty_tpl->inheritance->init($_smarty_tpl, true); ?>'; + + $i = 0; + $reversed_components = array_reverse($this->template->getSource()->components); + foreach ($reversed_components as $source) { + $i++; + if ($i === count($reversed_components)) { + $_compiled_code .= 'inheritance->endChild($_smarty_tpl); ?>'; + } + $_compiled_code .= $this->compileTag( + 'include', + [ + var_export($source->resource, true), + ['scope' => 'parent'], + ] + ); + } + $_compiled_code = $this->postFilter($_compiled_code, $this->template); } else { // get template source $_content = $this->template->source->getContent(); + $_compiled_code = $this->postFilter($this->doCompile($this->preFilter($_content), true)); } - $_compiled_code = $this->postFilter($this->doCompile($this->preFilter($_content), true)); if (!empty($this->required_plugins[ 'compiled' ]) || !empty($this->required_plugins[ 'nocache' ])) { $_compiled_code = 'compileRequiredPlugins() . "?>\n" . $_compiled_code; } diff --git a/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php b/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php index b14eab67..91908deb 100644 --- a/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php +++ b/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php @@ -1371,8 +1371,38 @@ class CompileBlockExtendsTest extends PHPUnit_Smarty ); } - public function testBlockWithAssign() { - $this->assertEquals('Captured content is: Content with lots of html here', $this->smarty->fetch('038_child.tpl')); - } + public function testBlockWithAssign() { + $this->assertEquals('Captured content is: Content with lots of html here', $this->smarty->fetch('038_child.tpl')); + } + + /** + * Test escaping of file parameter + */ + public function testEscaping() + { + $this->expectException(SmartyException::class); + $this->expectExceptionMessageRegExp('/Unable to load.*/'); + $this->assertEquals('hello world', $this->smarty->fetch('escaping.tpl')); + } + + /** + * Test escaping of file parameter 2 + */ + public function testEscaping2() + { + $this->expectException(SmartyException::class); + $this->expectExceptionMessageRegExp('/Unable to load.*/'); + $this->assertEquals('hello world', $this->smarty->fetch('escaping2.tpl')); + } + + /** + * Test escaping of file parameter 3 + */ + public function testEscaping3() + { + $this->expectException(SmartyException::class); + $this->expectExceptionMessageRegExp('/Unable to load.*/'); + $this->assertEquals('hello world', $this->smarty->fetch('escaping3.tpl')); + } } diff --git a/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping.tpl b/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping.tpl new file mode 100644 index 00000000..74540365 --- /dev/null +++ b/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping.tpl @@ -0,0 +1 @@ +{extends "extends:helloworld.tpl', var_dump(shell_exec('ls')), 1, 2, 3, 4, 5, 6);}}?>"} \ No newline at end of file diff --git a/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping2.tpl b/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping2.tpl new file mode 100644 index 00000000..4ad9c13b --- /dev/null +++ b/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping2.tpl @@ -0,0 +1 @@ +{extends 'extends:"helloworld.tpl\', var_dump(shell_exec(\'ls\')), 1, 2, 3, 4, 5, 6);}}?>'} \ No newline at end of file diff --git a/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping3.tpl b/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping3.tpl new file mode 100644 index 00000000..8c13ebd2 --- /dev/null +++ b/tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping3.tpl @@ -0,0 +1 @@ +{extends file='extends:"helloworld.tpl'|cat:"', var_dump(shell_exec('ls')), 1, 2, 3, 4, 5, 6);}}?>"} \ No newline at end of file diff --git a/tests/UnitTests/TemplateSource/TagTests/Include/CompileIncludeTest.php b/tests/UnitTests/TemplateSource/TagTests/Include/CompileIncludeTest.php index c9b1c154..4b5c5243 100644 --- a/tests/UnitTests/TemplateSource/TagTests/Include/CompileIncludeTest.php +++ b/tests/UnitTests/TemplateSource/TagTests/Include/CompileIncludeTest.php @@ -82,6 +82,18 @@ class CompileIncludeTest extends PHPUnit_Smarty $this->assertEquals('I1I2I3', $content, $text); } + /** + * test template name escaping + */ + public function testIncludeFilenameEscaping() + { + $this->expectException(SmartyException::class); + $this->expectExceptionMessageRegExp('/Unable to load.*/'); + $tpl = $this->smarty->createTemplate('test_include_security.tpl'); + $content = $this->smarty->fetch($tpl); + $this->assertEquals("hello world", $content); + } + /** * test standard output * diff --git a/tests/UnitTests/TemplateSource/TagTests/Include/templates/test_include_security.tpl b/tests/UnitTests/TemplateSource/TagTests/Include/templates/test_include_security.tpl new file mode 100644 index 00000000..4e449d85 --- /dev/null +++ b/tests/UnitTests/TemplateSource/TagTests/Include/templates/test_include_security.tpl @@ -0,0 +1 @@ +{include file="helloworld.tpl', var_dump(shell_exec('ls')), 1, 2, 3, 4, 5, 6);}}?>"} diff --git a/tests/UnitTests/TemplateSource/_Issues/419/ExtendsIssue419Test.php b/tests/UnitTests/TemplateSource/_Issues/419/ExtendsIssue419Test.php index 5ea20027..322322d7 100644 --- a/tests/UnitTests/TemplateSource/_Issues/419/ExtendsIssue419Test.php +++ b/tests/UnitTests/TemplateSource/_Issues/419/ExtendsIssue419Test.php @@ -32,4 +32,11 @@ class ExtendsIssue419Test extends PHPUnit_Smarty $this->assertEquals('child', $this->smarty->fetch('extends:001_parent.tpl|001_child.tpl')); } + public function testextendsSecurity() + { + $this->expectException(SmartyException::class); + $this->expectExceptionMessageRegExp('/Unable to load.*/'); + $this->assertEquals('child', $this->smarty->fetch('string:{include "001_parent.tpl\', var_dump(shell_exec(\'ls\')), 1, 2, 3, 4, 5, 6);}}?>"}')); + } + }