From 678a4bc1c4c050743481f955793836380bf8319b Mon Sep 17 00:00:00 2001 From: Artur Kyryliuk Date: Sun, 23 Aug 2026 17:14:07 +0200 Subject: [PATCH] fix(core): removed eval from PHx and hardened @FILE --- core/src/Core.php | 194 ++++++-- core/src/Legacy/Modifiers.php | 11 +- core/src/Legacy/Phx.php | 9 +- core/src/Support/ArithmeticExpression.php | 466 ++++++++++++++++++ .../Unit/Security/ParserEvalHardeningTest.php | 236 +++++++++ .../Unit/Support/ArithmeticExpressionTest.php | 160 ++++++ 6 files changed, 1039 insertions(+), 37 deletions(-) create mode 100644 core/src/Support/ArithmeticExpression.php create mode 100644 core/tests/Unit/Security/ParserEvalHardeningTest.php create mode 100644 core/tests/Unit/Support/ArithmeticExpressionTest.php diff --git a/core/src/Core.php b/core/src/Core.php index 70441c9963..b5feb885d3 100644 --- a/core/src/Core.php +++ b/core/src/Core.php @@ -87,6 +87,24 @@ class Core extends AbstractLaravel implements Interfaces\CoreInterface public $documentOutput; public $tstart = 0; public $mstart = 0; + /** + * Conditional-tag commands referenced by index from the source generated in + * mergeConditionalTagsContent(), so they never have to be quoted into it. + * + * @var array + */ + private $ctagCmds = []; + + /** + * Extensions @FILE refuses to serve. `.php` alone left `.phtml`/`.php5`/`.inc` readable as + * plain text, which discloses their source. + * + * @var string[] + */ + private const AT_BIND_FILE_DENIED_EXTENSIONS = [ + '.php', '.php3', '.php4', '.php5', '.php7', '.php8', '.phps', '.phtml', '.phar', '.inc', + ]; + public $minParserPasses = 2; public $maxParserPasses = 10; public $documentObject = []; @@ -1872,6 +1890,12 @@ public function mergeConditionalTagsContent( $content ); + // The command is handed to _parseCTagCMD() by index instead of being quoted into the + // generated source. Escaping it was never sufficient: a backslash in front of the quote + // consumed the escape and let the rest of the command close the string literal and run as + // PHP. Passing a reference removes the concatenation, so there is nothing left to escape. + $ctagOffset = count($this->ctagCmds); + $pieces = explode('<@IF:', $content); foreach ($pieces as $i => $split) { if ($i === 0) { @@ -1879,8 +1903,8 @@ public function mergeConditionalTagsContent( continue; } [$cmd, $text] = explode('>', $split, 2); - $cmd = str_replace("'", "\'", $cmd); - $content .= "_parseCTagCMD('" . $cmd . "')): ?>"; + $index = array_push($this->ctagCmds, $cmd) - 1; + $content .= '_parseCTagCMD($this->ctagCmds[' . $index . '])): ?>'; $content .= $text; } $pieces = explode('<@ELSEIF:', $content); @@ -1890,15 +1914,21 @@ public function mergeConditionalTagsContent( continue; } [$cmd, $text] = explode('>', $split, 2); - $cmd = str_replace("'", "\'", $cmd); - $content .= "_parseCTagCMD('" . $cmd . "')): ?>"; + $index = array_push($this->ctagCmds, $cmd) - 1; + $content .= '_parseCTagCMD($this->ctagCmds[' . $index . '])): ?>'; $content .= $text; } $content = str_replace(['<@ELSE>', '<@ENDIF>'], ['', ''], $content); ob_start(); - eval ('?>' . $content); - $content = ob_get_clean(); + try { + eval ('?>' . $content); + } finally { + $content = ob_get_clean(); + // A nested parse has already trimmed its own entries, so the indices baked into the + // source above stayed valid for the whole eval. + array_splice($this->ctagCmds, $ctagOffset); + } $content = str_replace( ["{$sp}h", "{$sp}p", "{$sp}s", "{$sp}e"], [''], @@ -2316,23 +2346,12 @@ public function _getSGVar($value) $this->setConfig('enable_filter', $_); $key = str_replace(['(', ')'], ["['", "']"], $key); $key = rtrim($key, ';'); - if (Str::contains($key, '$_SESSION')) { - $_ = $_SESSION; - $key = str_replace('$_SESSION', '$_', $key); - if (isset($_['mgrFormValues'])) { - unset($_['mgrFormValues']); - } - if (isset($_['token'])) { - unset($_['token']); - } - } - if (Str::contains($key, '[')) { - $value = $key ? eval ("return {$key};") : ''; - } elseif (0 < eval ("return count({$key});")) { - $value = eval ("return print_r({$key},true);"); - } else { - $value = ''; - } + + // The superglobal is read by walking the array, not by evaluating the tag. eval() only + // looked safe here because `(` and `)` were rewritten away, but PHP's backtick operator + // needs no parentheses, so `[[$_SERVER . `id` ]]` reached the shell. + $value = $this->resolveSGVar($key); + if ($modifiers !== false) { $value = $this->applyFilter($value, $modifiers, $key); } @@ -2340,6 +2359,83 @@ public function _getSGVar($value) return $value; } + /** + * Read one superglobal entry named by a parser tag. + * + * Accepts `$_GET(key)` and `$_GET['key']` (the former is rewritten into the latter by the + * caller), nested to any depth, plus the bare `$_SERVER` form that dumps the whole array. + * Anything else - arithmetic, concatenation, backticks - is refused rather than evaluated. + * + * @param string $key + * @return mixed + * @since 3.5.8 + */ + private function resolveSGVar($key) + { + if (!preg_match('@^\$_(GET|POST|SESSION|COOKIE|REQUEST|SERVER|FILES|ENV)@', $key, $matches)) { + return ''; + } + + $path = []; + $rest = substr($key, strlen($matches[0])); + while ($rest !== '' && $rest !== false) { + if (!preg_match('@^\[\s*([\'"]?)([^\[\]\'"]*)\1\s*\]@', $rest, $accessor)) { + // Trailing characters that are not an array access: refuse the whole tag. + return ''; + } + $path[] = $accessor[2]; + $rest = substr($rest, strlen($accessor[0])); + } + + $container = $this->getSuperGlobal($matches[1]); + + if ($path === []) { + return count($container) > 0 ? print_r($container, true) : ''; + } + + $cursor = $container; + foreach ($path as $segment) { + if (!is_array($cursor) || !array_key_exists($segment, $cursor)) { + return ''; + } + $cursor = $cursor[$segment]; + } + + return $cursor; + } + + /** + * @param string $name + * @return array + * @since 3.5.8 + */ + private function getSuperGlobal($name) + { + switch ($name) { + case 'GET': + return $_GET; + case 'POST': + return $_POST; + case 'COOKIE': + return $_COOKIE; + case 'REQUEST': + return $_REQUEST; + case 'SERVER': + return $_SERVER; + case 'FILES': + return $_FILES; + case 'ENV': + return $_ENV; + case 'SESSION': + $session = isset($_SESSION) && is_array($_SESSION) ? $_SESSION : []; + unset($session['mgrFormValues'], $session['token']); + + return $session; + } + + return []; + } + /** * @param $piece * @return null|string @@ -6300,6 +6396,47 @@ public function isSafeCode($phpcode = '', $safe_functions = '') * @param string $str * @return bool|mixed|string */ + /** + * Resolve one @FILE candidate to a real path inside the installation, or false. + * + * The old check compared the unresolved concatenation against EVO_MANAGER_PATH, so a `..` + * segment walked straight past it - and past EVO_BASE_PATH - to anywhere the web user could + * read. Containment is decided on the resolved path instead. + * + * @param string $candidate + * @return string|false + * @since 3.5.8 + */ + private function resolveAtBindFilePath($candidate) + { + $resolved = realpath($candidate); + if ($resolved === false || !is_file($resolved)) { + return false; + } + + $base = realpath(EVO_BASE_PATH); + if ($base === false) { + return false; + } + + $resolved = str_replace(DIRECTORY_SEPARATOR, '/', $resolved); + $base = rtrim(str_replace(DIRECTORY_SEPARATOR, '/', $base), '/') . '/'; + + if (strpos($resolved, $base) !== 0) { + return false; + } + + $manager = realpath(EVO_MANAGER_PATH); + if ($manager !== false) { + $manager = rtrim(str_replace(DIRECTORY_SEPARATOR, '/', $manager), '/') . '/'; + if (strpos($resolved, $manager) === 0) { + return false; + } + } + + return $resolved; + } + public function atBindFileContent($str = '') { @@ -6310,7 +6447,7 @@ public function atBindFileContent($str = '') $str = substr($str, 0, strpos("\n", $str)); } - if ($this->getExtFromFilename($str) === '.php') { + if (in_array($this->getExtFromFilename($str), self::AT_BIND_FILE_DENIED_EXTENSIONS, true)) { return 'Could not retrieve PHP file.'; } @@ -6325,16 +6462,11 @@ public function atBindFileContent($str = '') $search_path = ['assets/tvs/', 'assets/chunks/', 'assets/templates/', $this->getConfig('rb_base_url') . 'files/', '']; foreach ($search_path as $path) { - $file_path = EVO_BASE_PATH . $path . $str; - if (strpos($file_path, EVO_MANAGER_PATH) === 0) { - return $errorMsg; - } + $file_path = $this->resolveAtBindFilePath(EVO_BASE_PATH . $path . $str); - if (is_file($file_path)) { + if ($file_path !== false) { break; } - - $file_path = false; } if (!$file_path) { diff --git a/core/src/Legacy/Modifiers.php b/core/src/Legacy/Modifiers.php index 9871cf135e..e6f3863abe 100644 --- a/core/src/Legacy/Modifiers.php +++ b/core/src/Legacy/Modifiers.php @@ -2,6 +2,7 @@ use EvolutionCMS\Interfaces\ModifiersInterface; use EvolutionCMS\Models\SiteTemplate; +use EvolutionCMS\Support\ArithmeticExpression; use EvolutionCMS\Support\DataGrid; class Modifiers implements ModifiersInterface @@ -479,7 +480,7 @@ public function getValueFromPreset($key, $value, $cmd, $opt) case 'show': case 'this': $conditional = implode(' ', $this->condition); - $isvalid = (int)(eval("return ({$conditional});")); + $isvalid = (int)ArithmeticExpression::evaluate($conditional); if ($isvalid) { return $this->srcValue; } @@ -487,7 +488,7 @@ public function getValueFromPreset($key, $value, $cmd, $opt) return null; case 'then': $conditional = implode(' ', $this->condition); - $isvalid = (int)eval("return ({$conditional});"); + $isvalid = (int)ArithmeticExpression::evaluate($conditional); if ($isvalid) { return $opt; } @@ -495,7 +496,7 @@ public function getValueFromPreset($key, $value, $cmd, $opt) return null; case 'else': $conditional = implode(' ', $this->condition); - $isvalid = (int)eval("return ({$conditional});"); + $isvalid = (int)ArithmeticExpression::evaluate($conditional); if (!$isvalid) { return $opt; } @@ -910,7 +911,9 @@ public function getValueFromPreset($key, $value, $cmd, $opt) } $filter = str_replace('?', $value, $filter); - return eval("return {$filter};"); + // The letter strip above leaves `$`, quotes and backslashes in place, which is + // enough to reach PHP through octal escapes; only arithmetic gets through now. + return ArithmeticExpression::evaluate($filter); case 'count': if ($value == '') { return 0; diff --git a/core/src/Legacy/Phx.php b/core/src/Legacy/Phx.php index 7f6917cf5f..a5c8617a5e 100644 --- a/core/src/Legacy/Phx.php +++ b/core/src/Legacy/Phx.php @@ -1,6 +1,7 @@ ', + '==', '!=', '<>', '<=', '>=', '&&', '||', + '+', '-', '*', '/', '%', '<', '>', '!', + ]; + + /** + * Binary operator precedence, mirroring PHP's own table. Higher binds tighter. + */ + private const PRECEDENCE = [ + '*' => 60, '/' => 60, '%' => 60, + '+' => 50, '-' => 50, + '<' => 40, '<=' => 40, '>' => 40, '>=' => 40, '<=>' => 40, + '==' => 30, '!=' => 30, '<>' => 30, '===' => 30, '!==' => 30, + '&&' => 20, + '||' => 10, + ]; + + /** + * Unary operators, all right-associative and binding tighter than any binary operator. + */ + private const UNARY = ['u+' => 70, 'u-' => 70, '!' => 70]; + + /** + * Bounds that keep a hostile expression from costing more than it is worth. Real modifier + * arguments are a handful of characters; these are orders of magnitude above anything genuine. + */ + private const MAX_LENGTH = 512; + private const MAX_TOKENS = 256; + private const MAX_DEPTH = 32; + + /** + * Evaluate an expression, falling back to $default when it is not one we accept. + * + * @param mixed $expression + * @param mixed $default + * @return mixed + */ + public static function evaluate($expression, $default = 0) + { + $result = self::tryEvaluate($expression); + + return $result === null ? $default : $result; + } + + /** + * Evaluate an expression, returning null when it is not one we accept. + * + * @param mixed $expression + * @return int|float|bool|null + */ + public static function tryEvaluate($expression) + { + if (!is_scalar($expression)) { + return null; + } + + $expression = trim((string) $expression); + if ($expression === '' || strlen($expression) > self::MAX_LENGTH) { + return null; + } + + $tokens = self::tokenize($expression); + if ($tokens === null) { + return null; + } + + $rpn = self::toReversePolish($tokens); + if ($rpn === null) { + return null; + } + + return self::evaluateReversePolish($rpn); + } + + /** + * Split the expression into number literals, operators and parentheses. + * + * Unary `+`/`-`/`!` are distinguished from their binary forms here, while we still know whether + * the previous token closed an operand. + * + * @param string $expression + * @return array|null + */ + private static function tokenize($expression) + { + $tokens = []; + $length = strlen($expression); + $offset = 0; + // False directly after an operand (a number or a closing paren), which is the only position + // where `+`/`-` are binary. + $expectOperand = true; + + while ($offset < $length) { + if (count($tokens) > self::MAX_TOKENS) { + return null; + } + + $char = $expression[$offset]; + + if ($char === ' ' || $char === "\t" || $char === "\n" || $char === "\r") { + $offset++; + continue; + } + + if ($char === '(') { + if (!$expectOperand) { + // `2(3)` was never valid PHP either. + return null; + } + $tokens[] = ['(', null]; + $offset++; + continue; + } + + if ($char === ')') { + if ($expectOperand) { + return null; + } + $tokens[] = [')', null]; + $offset++; + $expectOperand = false; + continue; + } + + if (self::isDigit($char) || ($char === '.' && isset($expression[$offset + 1]) && self::isDigit($expression[$offset + 1]))) { + if (!$expectOperand) { + return null; + } + $number = self::readNumber($expression, $offset); + if ($number === null) { + return null; + } + $tokens[] = ['num', $number]; + $expectOperand = false; + continue; + } + + $operator = self::readOperator($expression, $offset); + if ($operator === null) { + return null; + } + + if ($expectOperand) { + // Only `+`, `-` and `!` have a unary form; anything else here is a syntax error. + if ($operator === '+' || $operator === '-') { + $tokens[] = ['op', 'u' . $operator]; + continue; + } + if ($operator === '!') { + $tokens[] = ['op', '!']; + continue; + } + + return null; + } + + if (!isset(self::PRECEDENCE[$operator])) { + // `!` cannot be binary. + return null; + } + + $tokens[] = ['op', $operator]; + $expectOperand = true; + } + + if ($expectOperand || $tokens === []) { + // A trailing operator, or nothing at all. + return null; + } + + return $tokens; + } + + /** + * @param string $char + * @return bool + */ + private static function isDigit($char) + { + return $char >= '0' && $char <= '9'; + } + + /** + * Read one decimal literal, advancing $offset past it. + * + * Exponents are deliberately unsupported: the callers strip `e` before we ever see the string, + * so accepting them here would only invent a syntax that never worked. + * + * @param string $expression + * @param int $offset + * @return int|float|null + */ + private static function readNumber($expression, &$offset) + { + $start = $offset; + $length = strlen($expression); + $seenDot = false; + + while ($offset < $length) { + $char = $expression[$offset]; + if (self::isDigit($char)) { + $offset++; + continue; + } + if ($char === '.' && !$seenDot) { + $seenDot = true; + $offset++; + continue; + } + break; + } + + $literal = substr($expression, $start, $offset - $start); + if ($literal === '' || $literal === '.') { + return null; + } + + if (!$seenDot && ctype_digit($literal)) { + // Stay on int while the value fits, so `2*3` keeps returning int(6) as eval() did. + $asInt = (int) $literal; + if ((string) $asInt === ltrim($literal, '0') || $literal === '0' || ltrim($literal, '0') === '') { + return $asInt; + } + + return (float) $literal; + } + + return (float) $literal; + } + + /** + * Read one operator, advancing $offset past it. + * + * @param string $expression + * @param int $offset + * @return string|null + */ + private static function readOperator($expression, &$offset) + { + foreach (self::OPERATORS as $operator) { + if (substr($expression, $offset, strlen($operator)) === $operator) { + $offset += strlen($operator); + + return $operator; + } + } + + return null; + } + + /** + * Shunting-yard: infix tokens to reverse polish notation. + * + * @param array $tokens + * @return array|null + */ + private static function toReversePolish(array $tokens) + { + $output = []; + $stack = []; + + foreach ($tokens as $token) { + [$type, $value] = $token; + + if ($type === 'num') { + $output[] = $token; + continue; + } + + if ($type === '(') { + $stack[] = $token; + if (count($stack) > self::MAX_DEPTH) { + return null; + } + continue; + } + + if ($type === ')') { + $matched = false; + while ($stack !== []) { + $top = array_pop($stack); + if ($top[0] === '(') { + $matched = true; + break; + } + $output[] = $top; + } + if (!$matched) { + return null; + } + continue; + } + + $isUnary = isset(self::UNARY[$value]); + $precedence = $isUnary ? self::UNARY[$value] : self::PRECEDENCE[$value]; + + while ($stack !== []) { + $top = end($stack); + if ($top[0] !== 'op') { + break; + } + $topIsUnary = isset(self::UNARY[$top[1]]); + $topPrecedence = $topIsUnary ? self::UNARY[$top[1]] : self::PRECEDENCE[$top[1]]; + + // Unary operators are right-associative, so an equal precedence does not pop. + if ($topPrecedence > $precedence || ($topPrecedence === $precedence && !$isUnary)) { + $output[] = array_pop($stack); + continue; + } + break; + } + + $stack[] = $token; + if (count($stack) > self::MAX_DEPTH) { + return null; + } + } + + while ($stack !== []) { + $top = array_pop($stack); + if ($top[0] === '(') { + return null; + } + $output[] = $top; + } + + return $output; + } + + /** + * @param array $rpn + * @return int|float|bool|null + */ + private static function evaluateReversePolish(array $rpn) + { + $stack = []; + + foreach ($rpn as $token) { + [$type, $value] = $token; + + if ($type === 'num') { + $stack[] = $value; + continue; + } + + if (isset(self::UNARY[$value])) { + if ($stack === []) { + return null; + } + $operand = array_pop($stack); + switch ($value) { + case 'u-': + $stack[] = -$operand; + break; + case 'u+': + $stack[] = +$operand; + break; + default: + $stack[] = !$operand; + } + continue; + } + + if (count($stack) < 2) { + return null; + } + $right = array_pop($stack); + $left = array_pop($stack); + + $result = self::apply($value, $left, $right); + if ($result === null) { + return null; + } + $stack[] = $result; + } + + if (count($stack) !== 1) { + return null; + } + + return $stack[0]; + } + + /** + * Apply one binary operator using PHP's own semantics. + * + * @param string $operator + * @param int|float|bool $left + * @param int|float|bool $right + * @return int|float|bool|null + */ + private static function apply($operator, $left, $right) + { + switch ($operator) { + case '+': + return $left + $right; + case '-': + return $left - $right; + case '*': + return $left * $right; + case '/': + // eval() raised DivisionByZeroError here; reporting "not evaluable" lets the caller + // fall back to its default instead of taking the request down. + if ((float) $right === 0.0) { + return null; + } + + return $left / $right; + case '%': + if ((int) $right === 0) { + return null; + } + + return (int) $left % (int) $right; + case '<': + return $left < $right; + case '<=': + return $left <= $right; + case '>': + return $left > $right; + case '>=': + return $left >= $right; + case '<=>': + return $left <=> $right; + case '==': + return $left == $right; + case '===': + return $left === $right; + case '!=': + case '<>': + return $left != $right; + case '!==': + return $left !== $right; + case '&&': + return (bool) $left && (bool) $right; + case '||': + return (bool) $left || (bool) $right; + } + + return null; + } +} diff --git a/core/tests/Unit/Security/ParserEvalHardeningTest.php b/core/tests/Unit/Security/ParserEvalHardeningTest.php new file mode 100644 index 0000000000..1475ab835d --- /dev/null +++ b/core/tests/Unit/Security/ParserEvalHardeningTest.php @@ -0,0 +1,236 @@ + conditional tags +| - _getSGVar() [[$_GET(x)]] superglobal reads +| - atBindFileContent() @FILE: template includes +| +| All three are reachable from content that the parser re-scans across passes, so a snippet echoing +| request data can carry a payload into them without any editing privilege. These tests drive the +| real methods on a Core instance and assert that a payload cannot execute or read outside the tree, +| while the legitimate syntax each method exists to serve keeps working. +| +*/ + +use EvolutionCMS\Core; + +beforeAll(function () { + if (!defined('IN_INSTALL_MODE')) { + define('IN_INSTALL_MODE', false); + } + if (!defined('EVO_API_MODE')) { + define('EVO_API_MODE', true); + } + if (!defined('IN_MANAGER_MODE')) { + define('IN_MANAGER_MODE', false); + } + $root = str_replace('\\', '/', dirname(__DIR__, 3)) . '/'; + if (!defined('EVO_BASE_PATH')) { + define('EVO_BASE_PATH', $root); + } + if (!defined('EVO_CORE_PATH')) { + define('EVO_CORE_PATH', $root . 'core/'); + } + if (!defined('EVO_MANAGER_PATH')) { + define('EVO_MANAGER_PATH', $root . 'manager/'); + } + $autoload = EVO_CORE_PATH . 'vendor/autoload.php'; + if (file_exists($autoload)) { + require_once $autoload; + } +}); + +class ParserHardeningCore extends Core +{ + public $cfg = ['enable_filter' => 1, 'rb_base_url' => 'assets/']; + + public function getConfig($name = '', $default = null) + { + return $this->cfg[$name] ?? $default; + } + + public function setConfig($name, $value = null): void + { + $this->cfg[$name] = $value; + } +} + +/** + * A Core with the two config keys the tested methods read, built without the heavy constructor so + * no bootstrap (storage paths, container, DB) is required. + */ +function parserHardeningCore(): Core +{ + $core = (new ReflectionClass(ParserHardeningCore::class))->newInstanceWithoutConstructor(); + + $_SERVER['REQUEST_TIME'] = $_SERVER['REQUEST_TIME'] ?? time(); + + return $core; +} + +describe('conditional tags (<@IF:>)', function () { + + test('a quote/backslash breakout neither executes nor fatals', function () { + $core = parserHardeningCore(); + $marker = str_replace('\\', '/', sys_get_temp_dir()) . '/evo_ctag_' . bin2hex(random_bytes(6)); + + // A backslash immediately before the quote defeated the str_replace("'", "\'") escaping: + // the doubled backslash was consumed, the quote closed the generated string literal early, + // and the tail ran as PHP. + $bs = chr(92); + $sq = chr(39); + $cmd = '1' . $bs . $sq . '.file_put_contents("' . $marker . '","x").' . $bs . $sq . '1'; + + $out = $core->mergeConditionalTagsContent('<@IF:' . $cmd . '>body<@ENDIF>'); + + expect(file_exists($marker))->toBeFalse() + ->and($out)->toBeString(); + }); + + test('legitimate numeric conditionals still resolve', function () { + $core = parserHardeningCore(); + + expect($core->mergeConditionalTagsContent('<@IF:5>A<@ELSE>B<@ENDIF>'))->toBe('A') + ->and($core->mergeConditionalTagsContent('<@IF:0>A<@ELSE>B<@ENDIF>'))->toBe('B') + ->and($core->mergeConditionalTagsContent('<@IF:5>A<@ELSEIF:1>X<@ELSE>B<@ENDIF>'))->toBe('A') + ->and($core->mergeConditionalTagsContent('<@IF:0>A<@ELSEIF:1>X<@ELSE>B<@ENDIF>'))->toBe('X') + ->and($core->mergeConditionalTagsContent('<@IF: !0 >neg<@ENDIF>'))->toBe('neg'); + }); + + test('nested conditionals resolve without index corruption', function () { + $core = parserHardeningCore(); + + // The inner block trims the shared command list; the outer indices must survive that. + $tpl = '<@IF:1>outer <@IF:1>inner<@ELSE>x<@ENDIF> end<@ELSE>no<@ENDIF>'; + + expect($core->mergeConditionalTagsContent($tpl))->toBe('outer inner end'); + }); + + test('content with no conditional tag is returned untouched', function () { + $core = parserHardeningCore(); + $plain = 'plain [+ph+] content with no tags'; + + expect($core->mergeConditionalTagsContent($plain))->toBe($plain); + }); +}); + +describe('superglobal reads ([[$_GET(x)]])', function () { + + test('a backtick payload is not executed', function () { + $core = parserHardeningCore(); + // A colon-free relative name: a `:` in the tag is the modifier delimiter, unrelated here. + $marker = 'evo_sg_' . bin2hex(random_bytes(6)) . '.txt'; + + // Backticks need no parentheses, so the old `(`/`)` rewrite did not stop them. + $payload = '$_SERVER . `echo x > ' . $marker . '`'; + + $value = $core->_getSGVar($payload); + + expect(file_exists($marker))->toBeFalse() + ->and($value)->toBe(''); + }); + + test('a statement-separator payload is refused', function () { + $core = parserHardeningCore(); + + expect($core->_getSGVar('$_GET[id];phpinfo()'))->toBe(''); + }); + + test('the documented accessor forms still read the value', function () { + $core = parserHardeningCore(); + $_GET['id'] = 'hello'; + $_POST['name'] = 'world'; + + // The caller rewrites (key) into ['key'] before _getSGVar sees it; accept both spellings. + expect($core->_getSGVar("\$_GET['id']"))->toBe('hello') + ->and($core->_getSGVar('$_GET(id)'))->toBe('hello') + ->and($core->_getSGVar("\$_POST['name']"))->toBe('world'); + + unset($_GET['id'], $_POST['name']); + }); + + test('a missing key yields empty string, not a notice', function () { + $core = parserHardeningCore(); + unset($_GET['nope']); + + expect($core->_getSGVar("\$_GET['nope']"))->toBe(''); + }); + + test('mgrFormValues and token stay hidden from $_SESSION dumps', function () { + $core = parserHardeningCore(); + $_SESSION = ['visible' => '1', 'mgrFormValues' => 'secret', 'token' => 'csrf']; + + $dump = $core->_getSGVar('$_SESSION'); + + expect($dump)->toContain('visible') + ->and($dump)->not->toContain('mgrFormValues') + ->and($dump)->not->toContain('csrf'); + + $_SESSION = []; + }); + + test('a variable outside the allow list is refused', function () { + $core = parserHardeningCore(); + + expect($core->_getSGVar('$GLOBALS'))->toBe('') + ->and($core->_getSGVar('$this'))->toBe(''); + }); +}); + +describe('@FILE binding', function () { + + test('directory traversal outside the base path is refused', function () { + $core = parserHardeningCore(); + + // A real file that certainly exists outside EVO_BASE_PATH. + $traversalDepth = str_repeat('../', 20); + + expect($core->atBindFileContent('@FILE:' . $traversalDepth . 'Windows/win.ini')) + ->toContain('Could not retrieve') + ->and($core->atBindFileContent('@FILE:' . $traversalDepth . 'etc/passwd')) + ->toContain('Could not retrieve'); + }); + + test('a php file inside the tree is still refused, including alternate extensions', function () { + $core = parserHardeningCore(); + + expect($core->atBindFileContent('@FILE:index.php'))->toBe('Could not retrieve PHP file.') + ->and($core->atBindFileContent('@FILE:index.phtml'))->toBe('Could not retrieve PHP file.') + ->and($core->atBindFileContent('@FILE:x.inc'))->toBe('Could not retrieve PHP file.'); + }); + + test('a permitted file inside the tree is read', function () { + $core = parserHardeningCore(); + + $relative = 'assets/evo_atfile_' . bin2hex(random_bytes(6)) . '.txt'; + $absolute = EVO_BASE_PATH . $relative; + file_put_contents($absolute, 'included-body'); + + try { + expect($core->atBindFileContent('@FILE:' . $relative))->toBe('included-body'); + } finally { + @unlink($absolute); + } + }); + + test('a traversal that resolves back inside the tree is still allowed', function () { + $core = parserHardeningCore(); + + $relative = 'assets/evo_atfile_' . bin2hex(random_bytes(6)) . '.txt'; + $absolute = EVO_BASE_PATH . $relative; + file_put_contents($absolute, 'roundtrip'); + + try { + // assets/../assets/ normalises to a path under the base, so it must resolve. + expect($core->atBindFileContent('@FILE:assets/../' . $relative))->toBe('roundtrip'); + } finally { + @unlink($absolute); + } + }); +}); diff --git a/core/tests/Unit/Support/ArithmeticExpressionTest.php b/core/tests/Unit/Support/ArithmeticExpressionTest.php new file mode 100644 index 0000000000..9eda282bed --- /dev/null +++ b/core/tests/Unit/Support/ArithmeticExpressionTest.php @@ -0,0 +1,160 @@ +toBe($expected); + })->with([ + '1+1', + '2*3', + '10-4', + '7/2', + '6/3', + '10%3', + '2+3*4', + '(2+3)*4', + '((1+2)*(3+4))', + '-5+3', + '+5-3', + '2*-3', + '1.5+2.25', + '0.1*3', + '100/8', + '1<2', + '2<=2', + '3>4', + '4>=4', + '1==1', + '1!=2', + '1&&0', + '1||0', + '!0', + '!1', + '2+3>4', + '1&&1||0', + '10-2-3', + '100/10/2', + '2*3%4', + '0', + '42', + '-0', + ]); +}); + +describe('operator handling', function () { + + test('left associativity is preserved for subtraction and division', function () { + expect(ArithmeticExpression::evaluate('10-2-3'))->toBe(5) + ->and(ArithmeticExpression::evaluate('100/10/2'))->toBe(5); + }); + + test('unary minus binds tighter than multiplication but not than parentheses', function () { + expect(ArithmeticExpression::evaluate('-2*3'))->toBe(-6) + ->and(ArithmeticExpression::evaluate('-(2*3)'))->toBe(-6) + ->and(ArithmeticExpression::evaluate('2--3'))->toBe(5); + }); + + test('integer arithmetic stays integer', function () { + expect(ArithmeticExpression::evaluate('2*3'))->toBeInt() + ->and(ArithmeticExpression::evaluate('6/3'))->toBeInt() + ->and(ArithmeticExpression::evaluate('7/2'))->toBeFloat(); + }); + + test('division and modulo by zero fall back instead of raising', function () { + // eval() raised DivisionByZeroError here, which took the whole request down. + expect(ArithmeticExpression::evaluate('1/0'))->toBe(0) + ->and(ArithmeticExpression::evaluate('1%0'))->toBe(0) + ->and(ArithmeticExpression::evaluate('1/0', 'n/a'))->toBe('n/a'); + }); +}); + +describe('rejects everything that is not arithmetic', function () { + + // Every payload below survives `preg_replace('@([a-zA-Z\n\r\t\s])@', '', $filter)` - the filter + // the callers apply before handing the string over - because it contains no letters at all. + $payloads = [ + 'octal escaped system() call' => '"\163\171\163\164\145\155"("\151\144")', + 'octal escaped phpinfo' => '"\160\150\160\151\156\146\157"()', + 'backtick shell operator' => '1 . `\151\144`', + 'variable variable' => '${"\137\107\105\124"}', + 'statement separator' => '1;print_r($_SERVER)', + 'superglobal read' => '$_SERVER', + 'string concatenation' => '"1"."2"', + 'array literal' => '[1,2][0]', + 'xor built string' => '("\1"^"\1")', + 'heredoc-ish quoting' => '"1"', + 'bare quote' => "'", + 'backslash' => '\\', + 'dollar' => '$', + 'braces' => '{1}', + 'closing paren only' => ')', + 'opening paren only' => '(', + 'unbalanced parens' => '(1+2', + 'trailing operator' => '1+', + 'leading binary operator' => '*2', + 'empty' => '', + 'two numbers' => '1 2', + 'implicit multiplication' => '2(3)', + ]; + + test('refuses the payload', function (string $payload) { + expect(ArithmeticExpression::tryEvaluate($payload))->toBeNull() + ->and(ArithmeticExpression::evaluate($payload))->toBe(0); + })->with($payloads); + + test('no payload reaches PHP even when it would be valid PHP', function () { + // If any of these were still evaluated the marker file would exist afterwards. + $marker = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'evo_arith_' . bin2hex(random_bytes(6)); + + // file_put_contents("", "x") spelled without a single letter. + $call = '"\146\151\154\145\137\160\165\164\137\143\157\156\164\145\156\164\163"("' + . addcslashes($marker, "\\\"") + . '","\170")'; + + ArithmeticExpression::evaluate($call); + + expect(file_exists($marker))->toBeFalse(); + }); +}); + +describe('bounds', function () { + + test('an over-long expression is refused rather than parsed', function () { + $long = str_repeat('1+', 400) . '1'; + + expect(ArithmeticExpression::tryEvaluate($long))->toBeNull(); + }); + + test('deeply nested parentheses are refused rather than recursed', function () { + $nested = str_repeat('(', 100) . '1' . str_repeat(')', 100); + + expect(ArithmeticExpression::tryEvaluate($nested))->toBeNull(); + }); + + test('a nesting depth a template might really use still works', function () { + expect(ArithmeticExpression::evaluate('((((1+2))))'))->toBe(3); + }); + + test('non-scalar input is refused', function () { + expect(ArithmeticExpression::tryEvaluate([1, 2]))->toBeNull() + ->and(ArithmeticExpression::tryEvaluate(null))->toBeNull(); + }); +});