From 7ad97ad030b4289711e30819c928b8bc33c62b23 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Mon, 10 Jan 2022 00:01:43 +0100 Subject: [PATCH] Merge pull request from GHSA-29gp-2c3m-3j6m * Temporary fix. Waiting for CVE * Add CVE --- CHANGELOG.md | 3 ++ libs/plugins/function.math.php | 32 ++++++++++++++++++- .../ValueTests/Math/MathTest.php | 31 ++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5093a0ee2..18ff857a9 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 +- Prevent arbitrary PHP code execution through maliciously crafted expression for the math function. This addresses CVE-2021-29454 + ## [3.1.41] - 2022-01-09 ### Security diff --git a/libs/plugins/function.math.php b/libs/plugins/function.math.php index 7348d9649..569182910 100644 --- a/libs/plugins/function.math.php +++ b/libs/plugins/function.math.php @@ -28,7 +28,12 @@ function smarty_function_math($params, $template) 'int' => true, 'abs' => true, 'ceil' => true, + 'acos' => true, + 'acosh' => true, 'cos' => true, + 'cosh' => true, + 'deg2rad' => true, + 'rad2deg' => true, 'exp' => true, 'floor' => true, 'log' => true, @@ -39,27 +44,51 @@ function smarty_function_math($params, $template) 'pow' => true, 'rand' => true, 'round' => true, + 'asin' => true, + 'asinh' => true, 'sin' => true, + 'sinh' => true, 'sqrt' => true, 'srand' => true, - 'tan' => true + 'atan' => true, + 'atanh' => true, + 'tan' => true, + 'tanh' => true ); + // be sure equation parameter is present if (empty($params[ 'equation' ])) { trigger_error("math: missing equation parameter", E_USER_WARNING); return; } $equation = $params[ 'equation' ]; + + // Remove whitespaces + $equation = preg_replace('/\s+/', '', $equation); + + // Adapted from https://www.php.net/manual/en/function.eval.php#107377 + $number = '(?:\d+(?:[,.]\d+)?|pi|π)'; // What is a number + $functionsOrVars = '((?:0x[a-fA-F0-9]+)|([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*))'; + $operators = '[+\/*\^%-]'; // Allowed math operators + $regexp = '/^(('.$number.'|'.$functionsOrVars.'|('.$functionsOrVars.'\s*\((?1)+\)|\((?1)+\)))(?:'.$operators.'(?2))?)+$/'; + + if (!preg_match($regexp, $equation)) { + trigger_error("math: illegal characters", E_USER_WARNING); + return; + } + // make sure parenthesis are balanced if (substr_count($equation, '(') !== substr_count($equation, ')')) { trigger_error("math: unbalanced parenthesis", E_USER_WARNING); return; } + // disallow backticks if (strpos($equation, '`') !== false) { trigger_error("math: backtick character not allowed in equation", E_USER_WARNING); return; } + // also disallow dollar signs if (strpos($equation, '$') !== false) { trigger_error("math: dollar signs not allowed in equation", E_USER_WARNING); @@ -96,6 +125,7 @@ function smarty_function_math($params, $template) } $smarty_math_result = null; eval("\$smarty_math_result = " . $equation . ";"); + if (empty($params[ 'format' ])) { if (empty($params[ 'assign' ])) { return $smarty_math_result; diff --git a/tests/UnitTests/TemplateSource/ValueTests/Math/MathTest.php b/tests/UnitTests/TemplateSource/ValueTests/Math/MathTest.php index ff0b3a1c4..6250f061b 100644 --- a/tests/UnitTests/TemplateSource/ValueTests/Math/MathTest.php +++ b/tests/UnitTests/TemplateSource/ValueTests/Math/MathTest.php @@ -107,4 +107,35 @@ public function testFunctionString() $tpl = $this->smarty->createTemplate('eval:{$x = "4"}{$y = "5.5"}{math equation="x * y" x=$x y=$y format="%0.2f"} -- {math equation="20.5 / 5" format="%0.2f"}'); $this->assertEquals($expected, $this->smarty->fetch($tpl)); } + + /** + * @expectedException PHPUnit_Framework_Error_Warning + */ + public function testBackticksIllegal() + { + $expected = "22.00"; + $tpl = $this->smarty->createTemplate('eval:{$x = "4"}{$y = "5.5"}{math equation="`ls` x * y" x=$x y=$y}'); + $this->assertEquals($expected, $this->smarty->fetch($tpl)); + } + + /** + * @expectedException PHPUnit_Framework_Error_Warning + */ + public function testDollarSignsIllegal() + { + $expected = "22.00"; + $tpl = $this->smarty->createTemplate('eval:{$x = "4"}{$y = "5.5"}{math equation="$" x=$x y=$y}'); + $this->assertEquals($expected, $this->smarty->fetch($tpl)); + } + + /** + * @expectedException PHPUnit_Framework_Error_Warning + */ + public function testBracketsIllegal() + { + $expected = "I"; + $tpl = $this->smarty->createTemplate('eval:{$x = "0"}{$y = "1"}{math equation="((y/x).(x))[x]" x=$x y=$y}'); + $this->assertEquals($expected, $this->smarty->fetch($tpl)); + } + }