Skip to content

Commit 76881c8

Browse files
wisskidwisskid
authored
Merge pull request from GHSA-4rmg-292m-wg3w
* Fixed a code injection vulnerability in extends-tag * update tests for smarty v4
1 parent 4549822 commit 76881c8

File tree

10 files changed

+78
-72
lines changed

10 files changed

+78
-72
lines changed

‎changelog/GHSA-4rmg-292m-wg3w.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fixed a code injection vulnerability in extends-tag. This addresses CVE-2024-35226.

‎libs/sysplugins/smarty_internal_compile_extends.php

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class Smarty_Internal_Compile_Extends extends Smarty_Internal_Compile_Shared_Inh
3030
*
3131
* @var array
3232
*/
33-
public $optional_attributes = array('extends_resource');
33+
public $optional_attributes = array();
3434

3535
/**
3636
* Attribute definition: Overwrites base class.
@@ -62,29 +62,7 @@ public function compile($args, Smarty_Internal_TemplateCompilerBase $compiler)
6262
}
6363
// add code to initialize inheritance
6464
$this->registerInit($compiler, true);
65-
$file = trim($_attr[ 'file' ], '\'"');
66-
if (strlen($file) > 8 && substr($file, 0, 8) === 'extends:') {
67-
// generate code for each template
68-
$files = array_reverse(explode('|', substr($file, 8)));
69-
$i = 0;
70-
foreach ($files as $file) {
71-
if ($file[ 0 ] === '"') {
72-
$file = trim($file, '".');
73-
} else {
74-
$file = "'{$file}'";
75-
}
76-
$i++;
77-
if ($i === count($files) && isset($_attr[ 'extends_resource' ])) {
78-
$this->compileEndChild($compiler);
79-
}
80-
$this->compileInclude($compiler, $file);
81-
}
82-
if (!isset($_attr[ 'extends_resource' ])) {
83-
$this->compileEndChild($compiler);
84-
}
85-
} else {
86-
$this->compileEndChild($compiler, $_attr[ 'file' ]);
87-
}
65+
$this->compileEndChild($compiler, $_attr[ 'file' ]);
8866
$compiler->has_code = false;
8967
return '';
9068
}
@@ -115,44 +93,4 @@ private function compileEndChild(Smarty_Internal_TemplateCompilerBase $compiler,
11593
'') . ");\n?>"
11694
);
11795
}
118-
119-
/**
120-
* Add code for including subtemplate to end of template
121-
*
122-
* @param \Smarty_Internal_TemplateCompilerBase $compiler
123-
* @param string $template subtemplate name
124-
*
125-
* @throws \SmartyCompilerException
126-
* @throws \SmartyException
127-
*/
128-
private function compileInclude(Smarty_Internal_TemplateCompilerBase $compiler, $template)
129-
{
130-
$compiler->parser->template_postfix[] = new Smarty_Internal_ParseTree_Tag(
131-
$compiler->parser,
132-
$compiler->compileTag(
133-
'include',
134-
array(
135-
$template,
136-
array('scope' => 'parent')
137-
)
138-
)
139-
);
140-
}
141-
142-
/**
143-
* Create source code for {extends} from source components array
144-
*
145-
* @param \Smarty_Internal_Template $template
146-
*
147-
* @return string
148-
*/
149-
public static function extendsSourceArrayCode(Smarty_Internal_Template $template)
150-
{
151-
$resources = array();
152-
foreach ($template->source->components as $source) {
153-
$resources[] = $source->resource;
154-
}
155-
return $template->smarty->left_delimiter . 'extends file=\'extends:' . join('|', $resources) .
156-
'\' extends_resource=true' . $template->smarty->right_delimiter;
157-
}
15896
}

‎libs/sysplugins/smarty_internal_templatecompilerbase.php

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -455,15 +455,29 @@ public function compileTemplateSource(
455455
$this->smarty->_current_file = $this->template->source->filepath;
456456
// get template source
457457
if (!empty($this->template->source->components)) {
458-
// we have array of inheritance templates by extends: resource
459-
// generate corresponding source code sequence
460-
$_content =
461-
Smarty_Internal_Compile_Extends::extendsSourceArrayCode($this->template);
458+
$_compiled_code = '<?php $_smarty_tpl->_loadInheritance(); $_smarty_tpl->inheritance->init($_smarty_tpl, true); ?>';
459+
460+
$i = 0;
461+
$reversed_components = array_reverse($this->template->getSource()->components);
462+
foreach ($reversed_components as $source) {
463+
$i++;
464+
if ($i === count($reversed_components)) {
465+
$_compiled_code .= '<?php $_smarty_tpl->inheritance->endChild($_smarty_tpl); ?>';
466+
}
467+
$_compiled_code .= $this->compileTag(
468+
'include',
469+
[
470+
var_export($source->resource, true),
471+
['scope' => 'parent'],
472+
]
473+
);
474+
}
475+
$_compiled_code = $this->postFilter($_compiled_code, $this->template);
462476
} else {
463477
// get template source
464478
$_content = $this->template->source->getContent();
479+
$_compiled_code = $this->postFilter($this->doCompile($this->preFilter($_content), true));
465480
}
466-
$_compiled_code = $this->postFilter($this->doCompile($this->preFilter($_content), true));
467481
if (!empty($this->required_plugins[ 'compiled' ]) || !empty($this->required_plugins[ 'nocache' ])) {
468482
$_compiled_code = '<?php ' . $this->compileRequiredPlugins() . "?>\n" . $_compiled_code;
469483
}

‎tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,8 +1371,38 @@ public function dataTestBlockNocache()
13711371
);
13721372
}
13731373

1374-
public function testBlockWithAssign() {
1375-
$this->assertEquals('Captured content is: Content with lots of html here', $this->smarty->fetch('038_child.tpl'));
1376-
}
1374+
public function testBlockWithAssign() {
1375+
$this->assertEquals('Captured content is: Content with lots of html here', $this->smarty->fetch('038_child.tpl'));
1376+
}
1377+
1378+
/**
1379+
* Test escaping of file parameter
1380+
*/
1381+
public function testEscaping()
1382+
{
1383+
$this->expectException(SmartyException::class);
1384+
$this->expectExceptionMessageRegExp('/Unable to load.*/');
1385+
$this->assertEquals('hello world', $this->smarty->fetch('escaping.tpl'));
1386+
}
1387+
1388+
/**
1389+
* Test escaping of file parameter 2
1390+
*/
1391+
public function testEscaping2()
1392+
{
1393+
$this->expectException(SmartyException::class);
1394+
$this->expectExceptionMessageRegExp('/Unable to load.*/');
1395+
$this->assertEquals('hello world', $this->smarty->fetch('escaping2.tpl'));
1396+
}
1397+
1398+
/**
1399+
* Test escaping of file parameter 3
1400+
*/
1401+
public function testEscaping3()
1402+
{
1403+
$this->expectException(SmartyException::class);
1404+
$this->expectExceptionMessageRegExp('/Unable to load.*/');
1405+
$this->assertEquals('hello world', $this->smarty->fetch('escaping3.tpl'));
1406+
}
13771407

13781408
}

‎tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping.tpl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{extends "extends:helloworld.tpl', var_dump(shell_exec('ls')), 1, 2, 3, 4, 5, 6);}}?>"}

‎tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping2.tpl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{extends 'extends:"helloworld.tpl\', var_dump(shell_exec(\'ls\')), 1, 2, 3, 4, 5, 6);}}?>'}

‎tests/UnitTests/TemplateSource/TagTests/BockExtend/templates/escaping3.tpl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{extends file='extends:"helloworld.tpl'|cat:"', var_dump(shell_exec('ls')), 1, 2, 3, 4, 5, 6);}}?>"}

‎tests/UnitTests/TemplateSource/TagTests/Include/CompileIncludeTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,18 @@ public function testSpacing_001V3($merge, $caching, $text)
8282
$this->assertEquals('I1I2I3', $content, $text);
8383
}
8484

85+
/**
86+
* test template name escaping
87+
*/
88+
public function testIncludeFilenameEscaping()
89+
{
90+
$this->expectException(SmartyException::class);
91+
$this->expectExceptionMessageRegExp('/Unable to load.*/');
92+
$tpl = $this->smarty->createTemplate('test_include_security.tpl');
93+
$content = $this->smarty->fetch($tpl);
94+
$this->assertEquals("hello world", $content);
95+
}
96+
8597
/**
8698
* test standard output
8799
*

‎tests/UnitTests/TemplateSource/TagTests/Include/templates/test_include_security.tpl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{include file="helloworld.tpl', var_dump(shell_exec('ls')), 1, 2, 3, 4, 5, 6);}}?>"}

‎tests/UnitTests/TemplateSource/_Issues/419/ExtendsIssue419Test.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,11 @@ public function testextends419()
3232
$this->assertEquals('child', $this->smarty->fetch('extends:001_parent.tpl|001_child.tpl'));
3333
}
3434

35+
public function testextendsSecurity()
36+
{
37+
$this->expectException(SmartyException::class);
38+
$this->expectExceptionMessageRegExp('/Unable to load.*/');
39+
$this->assertEquals('child', $this->smarty->fetch('string:{include "001_parent.tpl\', var_dump(shell_exec(\'ls\')), 1, 2, 3, 4, 5, 6);}}?>"}'));
40+
}
41+
3542
}

0 commit comments

Comments
 (0)