From c160134b6d9941af0947e152e299d6cf3a539240 Mon Sep 17 00:00:00 2001 From: Justin Hileman Date: Sat, 23 Mar 2013 10:46:59 -0700 Subject: [PATCH] Add a 'strict_callables' option to Mustache_Engine Prevents treating array('ClassName', 'methodName') or array($instance, 'methodName') as section or interpolation callables. This helps protect against arbitrary code evaluation when user input is passed directly into the template. This setting is recommended for PHP 5.3+, and all PHP 5.2 projects not using section or interpolation lambdas. The default value is false to preserve backwards compatibility. See #138 --- src/Mustache/Compiler.php | 72 +++++---- src/Mustache/Engine.php | 17 +- .../Functional/StrictCallablesTest.php | 147 ++++++++++++++++++ 3 files changed, 206 insertions(+), 30 deletions(-) create mode 100644 test/Mustache/Test/FiveThree/Functional/StrictCallablesTest.php diff --git a/src/Mustache/Compiler.php b/src/Mustache/Compiler.php index fe02419..a60dc45 100644 --- a/src/Mustache/Compiler.php +++ b/src/Mustache/Compiler.php @@ -22,27 +22,30 @@ class Mustache_Compiler private $indentNextLine; private $customEscape; private $charset; + private $strictCallables; private $pragmas; /** * Compile a Mustache token parse tree into PHP source code. * - * @param string $source Mustache Template source code - * @param string $tree Parse tree of Mustache tokens - * @param string $name Mustache Template class name - * @param bool $customEscape (default: false) - * @param string $charset (default: 'UTF-8') + * @param string $source Mustache Template source code + * @param string $tree Parse tree of Mustache tokens + * @param string $name Mustache Template class name + * @param bool $customEscape (default: false) + * @param string $charset (default: 'UTF-8') + * @param bool $strictCallables (default: false) * * @return string Generated PHP source code */ - public function compile($source, array $tree, $name, $customEscape = false, $charset = 'UTF-8') + public function compile($source, array $tree, $name, $customEscape = false, $charset = 'UTF-8', $strictCallables = false) { - $this->pragmas = array(); - $this->sections = array(); - $this->source = $source; - $this->indentNextLine = true; - $this->customEscape = $customEscape; - $this->charset = $charset; + $this->pragmas = array(); + $this->sections = array(); + $this->source = $source; + $this->indentNextLine = true; + $this->customEscape = $customEscape; + $this->charset = $charset; + $this->strictCallables = $strictCallables; return $this->writeCode($tree, $name); } @@ -165,7 +168,7 @@ class Mustache_Compiler const SECTION = ' private function section%s(Mustache_Context $context, $indent, $value) { $buffer = \'\'; - if (!is_string($value) && is_callable($value)) { + if (%s) { $source = %s; $buffer .= $this->mustache ->loadLambda((string) call_user_func($value, $source, $this->lambdaHelper)%s) @@ -196,9 +199,10 @@ class Mustache_Compiler */ private function section($nodes, $id, $start, $end, $otag, $ctag, $level) { - $method = $this->getFindMethod($id); - $id = var_export($id, true); - $source = var_export(substr($this->source, $start, $end - $start), true); + $method = $this->getFindMethod($id); + $id = var_export($id, true); + $source = var_export(substr($this->source, $start, $end - $start), true); + $callable = $this->getCallable(); if ($otag !== '{{' || $ctag !== '}}') { $delims = ', '.var_export(sprintf('{{= %s %s =}}', $otag, $ctag), true); @@ -209,7 +213,7 @@ class Mustache_Compiler $key = ucfirst(md5($delims."\n".$source)); if (!isset($this->sections[$key])) { - $this->sections[$key] = sprintf($this->prepare(self::SECTION), $key, $source, $delims, $this->walk($nodes, 2)); + $this->sections[$key] = sprintf($this->prepare(self::SECTION), $key, $callable, $source, $delims, $this->walk($nodes, 2)); } return sprintf($this->prepare(self::SECTION_CALL, $level), $id, $key, $method, $id); @@ -265,7 +269,7 @@ class Mustache_Compiler const VARIABLE = ' $value = $context->%s(%s); - if (!is_string($value) && is_callable($value)) { + if (%s) { $value = $this->mustache ->loadLambda((string) call_user_func($value)) ->renderInternal($context, $indent); @@ -290,11 +294,12 @@ class Mustache_Compiler list($id, $filters) = $this->getFilters($id, $level); } - $method = $this->getFindMethod($id); - $id = ($method !== 'last') ? var_export($id, true) : ''; - $value = $escape ? $this->getEscape() : '$value'; + $method = $this->getFindMethod($id); + $id = ($method !== 'last') ? var_export($id, true) : ''; + $callable = $this->getCallable(); + $value = $escape ? $this->getEscape() : '$value'; - return sprintf($this->prepare(self::VARIABLE, $level), $method, $id, $filters, $this->flushIndent(), $value); + return sprintf($this->prepare(self::VARIABLE, $level), $method, $id, $callable, $filters, $this->flushIndent(), $value); } /** @@ -315,7 +320,7 @@ class Mustache_Compiler const FILTER = ' $filter = $context->%s(%s); - if (is_string($filter) || !is_callable($filter)) { + if (!(%s)) { throw new UnexpectedValueException(%s); } $value = call_user_func($filter, $value);%s @@ -335,12 +340,13 @@ class Mustache_Compiler return ''; } - $name = array_shift($filters); - $method = $this->getFindMethod($name); - $filter = ($method !== 'last') ? var_export($name, true) : ''; - $msg = var_export(sprintf('Filter not found: %s', $name), true); + $name = array_shift($filters); + $method = $this->getFindMethod($name); + $filter = ($method !== 'last') ? var_export($name, true) : ''; + $callable = $this->getCallable('$filter'); + $msg = var_export(sprintf('Filter not found: %s', $name), true); - return sprintf($this->prepare(self::FILTER, $level), $method, $filter, $msg, $this->getFilter($filters, $level)); + return sprintf($this->prepare(self::FILTER, $level), $method, $filter, $callable, $msg, $this->getFilter($filters, $level)); } const LINE = '$buffer .= "\n";'; @@ -427,6 +433,16 @@ class Mustache_Compiler } } + const IS_CALLABLE = '!is_string(%s) && is_callable(%s)'; + const STRICT_IS_CALLABLE = 'is_object(%s) && is_callable(%s)'; + + private function getCallable($variable = '$value') + { + $tpl = $this->strictCallables ? self::STRICT_IS_CALLABLE : self::IS_CALLABLE; + + return sprintf($tpl, $variable, $variable); + } + const LINE_INDENT = '$indent . '; /** diff --git a/src/Mustache/Engine.php b/src/Mustache/Engine.php index ddfffca..fa5a9c8 100644 --- a/src/Mustache/Engine.php +++ b/src/Mustache/Engine.php @@ -41,6 +41,7 @@ class Mustache_Engine private $escape; private $charset = 'UTF-8'; private $logger; + private $strictCallables = false; /** * Mustache class constructor. @@ -87,6 +88,13 @@ class Mustache_Engine * // logging library -- such as Monolog -- is highly recommended. A simple stream logger implementation is * // available as well: * 'logger' => new Mustache_StreamLogger('php://stderr'), + * + * // Only treat Closure instances and invokable classes as callable. If true, values like + * // `array('ClassName', 'methodName')` and `array($classInstance, 'methodName')`, which are traditionally + * // "callable" in PHP, are not called to resolve variables for interpolation or section contexts. This + * // helps protect against arbitrary code execution when user input is passed directly into the template. + * // This currently defaults to false, but will default to true in v3.0. + * 'strict_callables' => true, * ); * * @param array $options (default: array()) @@ -136,6 +144,10 @@ class Mustache_Engine if (isset($options['logger'])) { $this->setLogger($options['logger']); } + + if (isset($options['strict_callables'])) { + $this->strictCallables = $options['strict_callables']; + } } /** @@ -452,10 +464,11 @@ class Mustache_Engine public function getTemplateClassName($source) { return $this->templateClassPrefix . md5(sprintf( - 'version:%s,escape:%s,charset:%s,source:%s', + 'version:%s,escape:%s,charset:%s,strict_callables:%s,source:%s', self::VERSION, isset($this->escape) ? 'custom' : 'default', $this->charset, + $this->strictCallables ? 'true' : 'false', $source )); } @@ -616,7 +629,7 @@ class Mustache_Engine array('className' => $name) ); - return $this->getCompiler()->compile($source, $tree, $name, isset($this->escape), $this->charset); + return $this->getCompiler()->compile($source, $tree, $name, isset($this->escape), $this->charset, $this->strictCallables); } /** diff --git a/test/Mustache/Test/FiveThree/Functional/StrictCallablesTest.php b/test/Mustache/Test/FiveThree/Functional/StrictCallablesTest.php new file mode 100644 index 0000000..87edf0d --- /dev/null +++ b/test/Mustache/Test/FiveThree/Functional/StrictCallablesTest.php @@ -0,0 +1,147 @@ + false)); + $tpl = $mustache->loadTemplate('{{# section }}{{ name }}{{/ section }}'); + + $data = new StdClass; + $data->name = $name; + $data->section = $section; + + $this->assertEquals($expected, $tpl->render($data)); + } + + public function callables() + { + $lambda = function($tpl, $mustache) { + return strtoupper($mustache->render($tpl)); + }; + + return array( + // Interpolation lambdas + array( + array($this, 'instanceName'), + $lambda, + 'YOSHI', + ), + array( + array(__CLASS__, 'staticName'), + $lambda, + 'YOSHI', + ), + array( + function() { return 'Yoshi'; }, + $lambda, + 'YOSHI', + ), + + // Section lambdas + array( + 'Yoshi', + array($this, 'instanceCallable'), + 'YOSHI', + ), + array( + 'Yoshi', + array(__CLASS__, 'staticCallable'), + 'YOSHI', + ), + array( + 'Yoshi', + $lambda, + 'YOSHI', + ), + ); + } + + + /** + * @group wip + * @dataProvider strictCallables + */ + public function testStrictCallablesEnabled($name, $section, $expected) + { + $mustache = new Mustache_Engine(array('strict_callables' => true)); + $tpl = $mustache->loadTemplate('{{# section }}{{ name }}{{/ section }}'); + + $data = new StdClass; + $data->name = $name; + $data->section = $section; + + $this->assertEquals($expected, $tpl->render($data)); + } + + public function strictCallables() + { + $lambda = function($tpl, $mustache) { + return strtoupper($mustache->render($tpl)); + }; + + return array( + // Interpolation lambdas + array( + function() { return 'Yoshi'; }, + $lambda, + 'YOSHI', + ), + + // Section lambdas + array( + 'Yoshi', + array($this, 'instanceCallable'), + 'YoshiYoshi', + ), + array( + 'Yoshi', + array(__CLASS__, 'staticCallable'), + 'YoshiYoshi', + ), + array( + 'Yoshi', + function($tpl, $mustache) { + return strtoupper($mustache->render($tpl)); + }, + 'YOSHI', + ), + ); + } + + public function instanceCallable($tpl, $mustache) + { + return strtoupper($mustache->render($tpl)); + } + + public static function staticCallable($tpl, $mustache) + { + return strtoupper($mustache->render($tpl)); + } + + public function instanceName() + { + return 'Yoshi'; + } + + public static function staticName() + { + return 'Yoshi'; + } +}