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

@@ -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 $source          Mustache Template source code
-     * @param string $tree         Parse tree of Mustache tokens
+     * @param string $tree            Parse tree of Mustache tokens
-     * @param string $name         Mustache Template class name
+     * @param string $name            Mustache Template class name
-     * @param bool   $customEscape (default: false)
+     * @param bool   $customEscape    (default: false)
-     * @param string $charset      (default: 'UTF-8')
+     * @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->pragmas         = array();
-        $this->sections       = array();
+        $this->sections        = array();
-        $this->source         = $source;
+        $this->source          = $source;
-        $this->indentNextLine = true;
+        $this->indentNextLine  = true;
-        $this->customEscape   = $customEscape;
+        $this->customEscape    = $customEscape;
-        $this->charset        = $charset;
+        $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);
+        $method   = $this->getFindMethod($id);
-        $id     = var_export($id, true);
+        $id       = var_export($id, true);
-        $source = var_export(substr($this->source, $start, $end - $start), 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);
+        $method   = $this->getFindMethod($id);
-        $id     = ($method !== 'last') ? var_export($id, true) : '';
+        $id       = ($method !== 'last') ? var_export($id, true) : '';
-        $value  = $escape ? $this->getEscape() : '$value';
+        $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);
+        $name     = array_shift($filters);
-        $method = $this->getFindMethod($name);
+        $method   = $this->getFindMethod($name);
-        $filter = ($method !== 'last') ? var_export($name, true) : '';
+        $filter   = ($method !== 'last') ? var_export($name, true) : '';
-        $msg    = var_export(sprintf('Filter not found: %s', $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 . ';
 
     /**

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);
     }
 
     /**

test/Mustache/Test/FiveThree/Functional/StrictCallablesTest.php

@@ -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';
+    }
+}