Permalink
Browse files

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
  • Loading branch information...
1 parent 6ac9cf0 commit c160134b6d9941af0947e152e299d6cf3a539240 @bobthecow committed Mar 23, 2013
Showing with 206 additions and 30 deletions.
  1. +44 −28 src/Mustache/Compiler.php
  2. +15 −2 src/Mustache/Engine.php
  3. +147 −0 test/Mustache/Test/FiveThree/Functional/StrictCallablesTest.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 @@ private function writeCode($tree, $name)
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 @@ private function section%s(Mustache_Context $context, $indent, $value) {
*/
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 @@ private function section($nodes, $id, $start, $end, $otag, $ctag, $level)
$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 @@ private function partial($id, $indent, $level)
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 @@ private function variable($id, $escape, $level)
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 @@ private function getFilters($id, $level)
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 @@ private function getFilter(array $filters, $level)
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 @@ private function getFindMethod($id)
}
}
+ 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 . ';
/**
@@ -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 @@ public function __construct(array $options = array())
if (isset($options['logger'])) {
$this->setLogger($options['logger']);
}
+
+ if (isset($options['strict_callables'])) {
+ $this->strictCallables = $options['strict_callables'];
+ }
}
/**
@@ -452,10 +464,11 @@ public function getCompiler()
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 @@ private function compile($source)
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);
}
/**
@@ -0,0 +1,147 @@
+<?php
+
+/*
+ * This file is part of Mustache.php.
+ *
+ * (c) 2013 Justin Hileman
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+/**
+ * @group lambdas
+ * @group functional
+ */
+class Mustache_Test_FiveThree_Functional_StrictCallablesTest extends PHPUnit_Framework_TestCase
+{
+ /**
+ * @dataProvider callables
+ */
+ public function testStrictCallablesDisabled($name, $section, $expected)
+ {
+ $mustache = new Mustache_Engine(array('strict_callables' => 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';
+ }
+}

0 comments on commit c160134

Please sign in to comment.