Skip to content

Commit

Permalink
Merge pull request from GHSA-4rmg-292m-wg3w
Browse files Browse the repository at this point in the history
  • Loading branch information
wisskid committed May 28, 2024
1 parent 61db287 commit 0be92bc
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 76 deletions.
2 changes: 2 additions & 0 deletions changelog/GHSA-4rmg-292m-wg3w.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fixed a code injection vulnerability in extends-tag. This addresses CVE-2024-35226.
- Added `$smarty->setCacheModifiedCheck()` setter for cache_modified_check
64 changes: 2 additions & 62 deletions src/Compile/Tag/ExtendsTag.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class ExtendsTag extends Inheritance {
*
* @var array
*/
protected $optional_attributes = ['extends_resource'];
protected $optional_attributes = [];

/**
* Attribute definition: Overwrites base class.
Expand Down Expand Up @@ -64,29 +64,7 @@ public function compile($args, \Smarty\Compiler\Template $compiler, $parameter =
}
// 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']);
return '';
}

Expand All @@ -106,42 +84,4 @@ private function compileEndChild(\Smarty\Compiler\Template $compiler, $template
(isset($template) ? ", {$template}, \$_smarty_current_dir" : '') . ");\n?>"
);
}

/**
* Add code for including subtemplate to end of template
*
* @param \Smarty\Compiler\Template $compiler
* @param string $template subtemplate name
*
* @throws \Smarty\CompilerException
* @throws \Smarty\Exception
*/
private function compileInclude(\Smarty\Compiler\Template $compiler, $template) {
$compiler->getParser()->template_postfix[] = new \Smarty\ParseTree\Tag(
$compiler->getParser(),
$compiler->compileTag(
'include',
[
$template,
['scope' => 'parent'],
]
)
);
}

/**
* Create source code for {extends} from source components array
*
* @param \Smarty\Template $template
*
* @return string
*/
public static function extendsSourceArrayCode(\Smarty\Template $template) {
$resources = [];
foreach ($template->getSource()->components as $source) {
$resources[] = $source->resource;
}
return $template->getLeftDelimiter() . 'extends file=\'extends:' . join('|', $resources) .
'\' extends_resource=true' . $template->getRightDelimiter();
}
}
38 changes: 27 additions & 11 deletions src/Compiler/Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -403,21 +403,37 @@ public function compileTemplateSource(\Smarty\Template $template, \Smarty\Compil
}
// get template source
if (!empty($this->template->getSource()->components)) {
// we have array of inheritance templates by extends: resource
// generate corresponding source code sequence
$_content =
ExtendsTag::extendsSourceArrayCode($this->template);

$_compiled_code = '<?php $_smarty_tpl->getInheritance()->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 .= '<?php $_smarty_tpl->getInheritance()->endChild($_smarty_tpl); ?>';
}
$_compiled_code .= $this->compileTag(
'include',
[
var_export($source->resource, true),
['scope' => 'parent'],
]
);
}
$_compiled_code = $this->smarty->runPostFilters($_compiled_code, $this->template);
} else {
// get template source
$_content = $this->template->getSource()->getContent();
$_compiled_code = $this->smarty->runPostFilters(
$this->doCompile(
$this->smarty->runPreFilters($_content, $this->template),
true
),
$this->template
);
}
$_compiled_code = $this->smarty->runPostFilters(
$this->doCompile(
$this->smarty->runPreFilters($_content, $this->template),
true
),
$this->template
);

} catch (\Exception $e) {
if ($this->smarty->debugging) {
$this->smarty->getDebug()->end_compile($this->template);
Expand Down
9 changes: 9 additions & 0 deletions src/Smarty.php
Original file line number Diff line number Diff line change
Expand Up @@ -2211,5 +2211,14 @@ private function returnOrCreateTemplate($template, $cache_id = null, $compile_id
return $template;
}

/**
* Sets if Smarty should check If-Modified-Since headers to determine cache validity.
* @param bool $cache_modified_check
* @return void
*/
public function setCacheModifiedCheck($cache_modified_check): void {
$this->cache_modified_check = (bool) $cache_modified_check;
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -1193,8 +1193,38 @@ public function dataTestBlockNocache()
);
}

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(\Smarty\Exception::class);
$this->expectExceptionMessageMatches('/Unable to load.*/');
$this->assertEquals('hello world', $this->smarty->fetch('escaping.tpl'));
}

/**
* Test escaping of file parameter 2
*/
public function testEscaping2()
{
$this->expectException(\Smarty\Exception::class);
$this->expectExceptionMessageMatches('/Unable to load.*/');
$this->assertEquals('hello world', $this->smarty->fetch('escaping2.tpl'));
}

/**
* Test escaping of file parameter 3
*/
public function testEscaping3()
{
$this->expectException(\Smarty\Exception::class);
$this->expectExceptionMessageMatches('/Unable to load.*/');
$this->assertEquals('hello world', $this->smarty->fetch('escaping3.tpl'));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{extends "extends:helloworld.tpl', var_dump(shell_exec('ls')), 1, 2, 3);}}?>"}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{extends 'extends:"helloworld.tpl\', var_dump(shell_exec(\'ls\')), 1, 2, 3);}}?>'}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{extends file='extends:"helloworld.tpl'|cat:"', var_dump(shell_exec('ls')), 1, 2, 3);}}?>"}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ public function testSpacing_001V3($merge, $caching, $text)
$this->assertEquals('I1I2I3', $content, $text);
}

/**
* test template name escaping
*/
public function testIncludeFilenameEscaping()
{
$this->expectException(\Smarty\Exception::class);
$this->expectExceptionMessageMatches('/Unable to load.*/');
$tpl = $this->smarty->createTemplate('test_include_security.tpl');
$content = $this->smarty->fetch($tpl);
$this->assertEquals("hello world", $content);
}

/**
* test standard output
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{include file="helloworld.tpl', var_dump(shell_exec('ls')), 1, 2, 3);}}?>"}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,11 @@ public function testextends419()
$this->assertEquals('child', $this->smarty->fetch('extends:001_parent.tpl|001_child.tpl'));
}

public function testextendsSecurity()
{
$this->expectException(\Smarty\Exception::class);
$this->expectExceptionMessageMatches('/Unable to load.*/');
$this->assertEquals('child', $this->smarty->fetch('string:{include "001_parent.tpl\', var_dump(shell_exec(\'ls\')), 1, 2, 3);}}?>"}'));
}

}

0 comments on commit 0be92bc

Please sign in to comment.