From fedc1270574cf722d9d7462534e0a7c11b03d3b7 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Sat, 16 Jan 2021 23:01:15 +0100 Subject: [PATCH 01/19] Mark tests that use sleep calls as slow, so we can ignore them when running unit tests in development --- .../_shared/CacheResourceTestCommon.php | 6 +++++- .../ResourceTests/Extends/ExtendsResourceTest.php | 4 ++-- .../TagTests/BockExtend/CompileBlockExtendsTest.php | 8 +++++--- .../TemplateSource/TagTests/Insert/CompileInsertTest.php | 1 + .../ValueTests/SmartySpecialVars/Now/SmartyNowTest.php | 2 +- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/UnitTests/CacheResourceTests/_shared/CacheResourceTestCommon.php b/tests/UnitTests/CacheResourceTests/_shared/CacheResourceTestCommon.php index 7deaebd2..129fb6ba 100644 --- a/tests/UnitTests/CacheResourceTests/_shared/CacheResourceTestCommon.php +++ b/tests/UnitTests/CacheResourceTests/_shared/CacheResourceTestCommon.php @@ -339,6 +339,10 @@ class CacheResourceTestCommon extends PHPUnit_Smarty $this->assertNull($tpl->cached->handler->getCachedContent($tpl3)); $this->assertEquals('hello world', $tpl->cached->handler->getCachedContent($tpl4)); } + + /** + * @group slow + */ public function testClearCacheExpired() { $this->smarty->caching = true; @@ -399,7 +403,7 @@ class CacheResourceTestCommon extends PHPUnit_Smarty * @runInSeparateProcess * @preserveGlobalState disabled * @dataProvider data - * + * @group slow */ public function testCache($lockTime, $lockTimeout, $compile_id, $cache_id, $isCached, $tmin, $tmax, $forceCompile, $forceCache, $update, $testNumber, $compileTestNumber, $renderTestNumber, $testName) { diff --git a/tests/UnitTests/ResourceTests/Extends/ExtendsResourceTest.php b/tests/UnitTests/ResourceTests/Extends/ExtendsResourceTest.php index 3012f34f..06fe239a 100644 --- a/tests/UnitTests/ResourceTests/Extends/ExtendsResourceTest.php +++ b/tests/UnitTests/ResourceTests/Extends/ExtendsResourceTest.php @@ -125,7 +125,7 @@ class ExtendsResourceTest extends PHPUnit_Smarty * test grandchild/child/parent dependency test2 * @runInSeparateProcess * @preserveGlobalState disabled - * + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_2() { @@ -193,7 +193,7 @@ class ExtendsResourceTest extends PHPUnit_Smarty * test grandchild/child/parent dependency test4 * @runInSeparateProcess * @preserveGlobalState disabled - * + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_4() { diff --git a/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php b/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php index 0f3eb406..0fb1cbe0 100644 --- a/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php +++ b/tests/UnitTests/TemplateSource/TagTests/BockExtend/CompileBlockExtendsTest.php @@ -610,7 +610,7 @@ class CompileBlockExtendsTest extends PHPUnit_Smarty * * @runInSeparateProcess * @preserveGlobalState disabled - * + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_2() { @@ -645,7 +645,7 @@ class CompileBlockExtendsTest extends PHPUnit_Smarty * * @runInSeparateProcess * @preserveGlobalState disabled - * + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_3() { @@ -670,7 +670,7 @@ class CompileBlockExtendsTest extends PHPUnit_Smarty * * @runInSeparateProcess * @preserveGlobalState disabled - * + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_32() { @@ -692,6 +692,7 @@ class CompileBlockExtendsTest extends PHPUnit_Smarty * * @runInSeparateProcess * @preserveGlobalState disabled + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_4() { @@ -716,6 +717,7 @@ class CompileBlockExtendsTest extends PHPUnit_Smarty * * @runInSeparateProcess * @preserveGlobalState disabled + * @group slow */ public function testCompileBlockGrandChildMustCompile_021_42() { diff --git a/tests/UnitTests/TemplateSource/TagTests/Insert/CompileInsertTest.php b/tests/UnitTests/TemplateSource/TagTests/Insert/CompileInsertTest.php index 61e8835c..7498d2cf 100644 --- a/tests/UnitTests/TemplateSource/TagTests/Insert/CompileInsertTest.php +++ b/tests/UnitTests/TemplateSource/TagTests/Insert/CompileInsertTest.php @@ -210,6 +210,7 @@ class CompileInsertTest extends PHPUnit_Smarty * test insert plugin caching 2 * @runInSeparateProcess * @preserveGlobalState disabled + * @group slow */ public function testInsertPluginCaching3_2() { diff --git a/tests/UnitTests/TemplateSource/ValueTests/SmartySpecialVars/Now/SmartyNowTest.php b/tests/UnitTests/TemplateSource/ValueTests/SmartySpecialVars/Now/SmartyNowTest.php index 724f2bf5..21f4bf26 100644 --- a/tests/UnitTests/TemplateSource/ValueTests/SmartySpecialVars/Now/SmartyNowTest.php +++ b/tests/UnitTests/TemplateSource/ValueTests/SmartySpecialVars/Now/SmartyNowTest.php @@ -35,7 +35,7 @@ class SmartyNowTest extends PHPUnit_Smarty } /** * test {$smarty.now nocache} - * + * @group slow */ public function testSmartyNowNocache() { $this->smarty->setCaching(true); From 6463519a6c05e614158d00d59df906433e869da3 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Sun, 24 Jan 2021 23:13:26 +0100 Subject: [PATCH 02/19] Prevent access to .template_object when in security mode to prevent PHP code injection vulnerability --- CHANGELOG.md | 3 +++ .../smarty_internal_compile_private_special_variable.php | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06b89822..c26136bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Prevent access to `$smarty.template_object` in Security mode + ## [3.1.38] - 2021-01-08 ### Fixed diff --git a/libs/sysplugins/smarty_internal_compile_private_special_variable.php b/libs/sysplugins/smarty_internal_compile_private_special_variable.php index de7d4a22..d53ef51f 100644 --- a/libs/sysplugins/smarty_internal_compile_private_special_variable.php +++ b/libs/sysplugins/smarty_internal_compile_private_special_variable.php @@ -81,6 +81,10 @@ class Smarty_Internal_Compile_Private_Special_Variable extends Smarty_Internal_C case 'template': return 'basename($_smarty_tpl->source->filepath)'; case 'template_object': + if (isset($compiler->smarty->security_policy)) { + $compiler->trigger_template_error("(secure mode) template_object not permitted"); + break; + } return '$_smarty_tpl'; case 'current_dir': return 'dirname($_smarty_tpl->source->filepath)'; From 165f1bd4d2eec328cfeaca517a725b46001de838 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Sun, 24 Jan 2021 23:44:07 +0100 Subject: [PATCH 03/19] Fixed Code injection vulnerability by using illegal function names --- CHANGELOG.md | 3 +++ libs/sysplugins/smarty_internal_compile_function.php | 5 +++++ .../TagTests/TemplateFunction/CompileFunctionTest.php | 11 ++++++++++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06b89822..ecbd404b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security +- Code injection vulnerability by using illegal function names in `{function name='blah'}{/function}` + ## [3.1.38] - 2021-01-08 ### Fixed diff --git a/libs/sysplugins/smarty_internal_compile_function.php b/libs/sysplugins/smarty_internal_compile_function.php index 6e408ca7..d0f2b0f4 100644 --- a/libs/sysplugins/smarty_internal_compile_function.php +++ b/libs/sysplugins/smarty_internal_compile_function.php @@ -58,6 +58,11 @@ class Smarty_Internal_Compile_Function extends Smarty_Internal_CompileBase } unset($_attr[ 'nocache' ]); $_name = trim($_attr[ 'name' ], '\'"'); + + if (!preg_match('/^[a-zA-Z0-9_\x80-\xff]+$/', $_name)) { + $compiler->trigger_template_error("Function name contains invalid characters: {$_name}", null, true); + } + $compiler->parent_compiler->tpl_function[ $_name ] = array(); $save = array( $_attr, $compiler->parser->current_buffer, $compiler->template->compiled->has_nocache_code, diff --git a/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php b/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php index d2be82ca..6c902a68 100644 --- a/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php +++ b/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php @@ -431,5 +431,14 @@ class CompileFunctionTest extends PHPUnit_Smarty array("{function name=simple}A{\$foo}\nC{/function}{call name='simple'}", "Abar\nC", 'T14', $i++), array("{function name=simple}A\n{\$foo}\nC{/function}{call name='simple'}", "A\nbar\nC", 'T15', $i++), ); - } + } + + /** + * Test handling of function names that are a security risk + */ + public function testIllegalFunctionName() { + $this->expectException(SmartyCompilerException::class); + $this->smarty->fetch('string:{function name=\'rce(){};echo "hi";function \'}{/function}'); + } + } From 288a54f6b0194121efa49f0aa9d70eb3ff439370 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Sun, 24 Jan 2021 23:52:45 +0100 Subject: [PATCH 04/19] Add unit test --- tests/UnitTests/SecurityTests/SecurityTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/UnitTests/SecurityTests/SecurityTest.php b/tests/UnitTests/SecurityTests/SecurityTest.php index 2a4d3e59..4af37e16 100644 --- a/tests/UnitTests/SecurityTests/SecurityTest.php +++ b/tests/UnitTests/SecurityTests/SecurityTest.php @@ -382,6 +382,15 @@ class SecurityTest extends PHPUnit_Smarty $this->smarty->security_policy->trusted_uri = array(); $this->assertContains('Preface | Smarty', $this->smarty->fetch('string:{fetch file="https://www.smarty.net/docs/en/preface.tpl"}')); } + + /** + * In security mode, accessing $smarty.template_object should be illegal. + */ + public function testSmartyTemplateObject() { + $this->expectException(SmartyCompilerException::class); + $this->smarty->display('string:{$smarty.template_object}'); + } + } class mysecuritystaticclass From 2543174460adc34d51150b9e9b076a622e72dfeb Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Mon, 1 Feb 2021 10:31:20 +0100 Subject: [PATCH 05/19] Cannot use in Smarty3 yet, revert to @expectedException --- .../TagTests/TemplateFunction/CompileFunctionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php b/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php index 6c902a68..65d3b2de 100644 --- a/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php +++ b/tests/UnitTests/TemplateSource/TagTests/TemplateFunction/CompileFunctionTest.php @@ -435,9 +435,9 @@ class CompileFunctionTest extends PHPUnit_Smarty /** * Test handling of function names that are a security risk + * @expectedException SmartyCompilerException */ public function testIllegalFunctionName() { - $this->expectException(SmartyCompilerException::class); $this->smarty->fetch('string:{function name=\'rce(){};echo "hi";function \'}{/function}'); } From 8fc66e27a7566e0ecba77ee2412406dc2154ce4b Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Mon, 1 Feb 2021 10:33:00 +0100 Subject: [PATCH 06/19] Cannot use in Smarty3 yet, revert to @expectedException --- expectException | 0 tests/UnitTests/SecurityTests/SecurityTest.php | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 expectException diff --git a/expectException b/expectException new file mode 100644 index 00000000..e69de29b diff --git a/tests/UnitTests/SecurityTests/SecurityTest.php b/tests/UnitTests/SecurityTests/SecurityTest.php index 4af37e16..bbb8b4e8 100644 --- a/tests/UnitTests/SecurityTests/SecurityTest.php +++ b/tests/UnitTests/SecurityTests/SecurityTest.php @@ -385,10 +385,10 @@ class SecurityTest extends PHPUnit_Smarty /** * In security mode, accessing $smarty.template_object should be illegal. + * @expectedException SmartyCompilerException */ public function testSmartyTemplateObject() { - $this->expectException(SmartyCompilerException::class); - $this->smarty->display('string:{$smarty.template_object}'); + $this->smarty->display('string:{$smarty.template_object}'); } } From 74cab5a56b3ced14c282fc6b5fc1ce248d3a7dda Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Wed, 17 Feb 2021 22:30:35 +0100 Subject: [PATCH 07/19] updated changelog header to security --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c26136bc..3e607e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Fixed +### Security - Prevent access to `$smarty.template_object` in Security mode ## [3.1.38] - 2021-01-08 From e66e293a8a65b45a8b0bf360d6c555fdde1445d7 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Wed, 17 Feb 2021 22:50:52 +0100 Subject: [PATCH 08/19] Do not push release automatically in make release script, to enable a chance to catch any errors. --- make-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/make-release.sh b/make-release.sh index 8357ecf9..b3a9df64 100755 --- a/make-release.sh +++ b/make-release.sh @@ -14,6 +14,6 @@ git pull git merge --no-ff "release/$1" git branch -d "release/$1" git tag -a "v$1" -m "Release $1" -git push --follow-tags printf 'Done creating release %s\n' "$1" +printf 'Run `git push --follow-tags origin` to publish it.\n' From 3148d406a061cb3db351cd10904513c5301de5e8 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Wed, 17 Feb 2021 22:57:33 +0100 Subject: [PATCH 09/19] changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1caacb30..e05d636f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Security -- Prevent access to `$smarty.template_object` in Security mode -- Code injection vulnerability by using illegal function names in `{function name='blah'}{/function}` +- Prevent access to `$smarty.template_object` in sandbox mode +- Fixed code injection vulnerability by using illegal function names in `{function name='blah'}{/function}` ## [3.1.38] - 2021-01-08 From a21f59663c7a94a7e85d69f364b152155d7974e0 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Wed, 17 Feb 2021 22:57:50 +0100 Subject: [PATCH 10/19] version bump --- CHANGELOG.md | 2 ++ libs/Smarty.class.php | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e05d636f..e3bb93a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [3.1.39] - 2021-02-17 + ### Security - Prevent access to `$smarty.template_object` in sandbox mode - Fixed code injection vulnerability by using illegal function names in `{function name='blah'}{/function}` diff --git a/libs/Smarty.class.php b/libs/Smarty.class.php index 6564be6d..375bab13 100644 --- a/libs/Smarty.class.php +++ b/libs/Smarty.class.php @@ -111,7 +111,7 @@ class Smarty extends Smarty_Internal_TemplateBase /** * smarty version */ - const SMARTY_VERSION = '3.1.38'; + const SMARTY_VERSION = '3.1.39'; /** * define variable scopes */ From e2485fa45e53816037e25c2977fbc064ee358972 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Sun, 21 Feb 2021 22:03:44 +0100 Subject: [PATCH 11/19] Create SECURITY.md --- SECURITY.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..d98ea018 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,19 @@ +# Security Policy + +## Supported Versions + +Smarty currently supports the latest minor version of Smarty 3 and Smarty 4. (Smarty 4 has not been released yet.) + +| Version | Supported | +| ------- | ------------------ | +| 4.0.x | :white_check_mark: | +| 3.1.x | :white_check_mark: | +| < 3.1 | :x: | + +## Reporting a Vulnerability + + If you have discovered a security issue with Smarty, please contact us at mail [at] simonwisselink.nl. Do not + disclose your findings publicly and PLEASE PLEASE do not file an Issue. + +We will try to confirm the vulnerability and develop a fix if appropriate. When we release the fix, we will publish +a security release. Please let us know if you want to be credited. From 290aee6db33403a4b9426b572b79a990232b31dd Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Sun, 21 Feb 2021 22:23:45 +0100 Subject: [PATCH 12/19] Update CHANGELOG.md Add CVE's --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3bb93a4..45286fee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [3.1.39] - 2021-02-17 ### Security -- Prevent access to `$smarty.template_object` in sandbox mode -- Fixed code injection vulnerability by using illegal function names in `{function name='blah'}{/function}` +- Prevent access to `$smarty.template_object` in sandbox mode. This addresses CVE-2021-26119. +- Fixed code injection vulnerability by using illegal function names in `{function name='blah'}{/function}`. This addresses CVE-2021-26120. ## [3.1.38] - 2021-01-08 From 9cde36e3bc263b261b4f123876c5bf9c54817925 Mon Sep 17 00:00:00 2001 From: Mihail Haritonov Date: Sun, 28 Feb 2021 16:43:54 +0300 Subject: [PATCH 13/19] plugins: escape: javascript escaping secure fix --- libs/plugins/modifier.escape.php | 6 +++++- libs/plugins/modifiercompiler.escape.php | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libs/plugins/modifier.escape.php b/libs/plugins/modifier.escape.php index 150901c7..5f01362c 100644 --- a/libs/plugins/modifier.escape.php +++ b/libs/plugins/modifier.escape.php @@ -184,7 +184,11 @@ function smarty_modifier_escape($string, $esc_type = 'html', $char_set = null, $ '"' => '\\"', "\r" => '\\r', "\n" => '\\n', - ' '<\/' + ' '<\/', + // see https://html.spec.whatwg.org/multipage/scripting.html#restrictions-for-contents-of-script-elements + '