Skip to content
This repository
Browse code

Fix another potential security vector

  • Loading branch information...
commit 537a811a09579be1a34b32325e9669069b38d531 1 parent 6d0e085
Andy Thompson authored weierophinney committed
38  library/Phly/Mustache/Renderer.php
@@ -73,19 +73,6 @@ public function render(array $tokens, $view, array $partials = null)
73 73
         $renderer = $this;
74 74
         $inLoop   = false;
75 75
 
76  
-        if (is_object($view)) {
77  
-            // If we have an object, get a list of properties and methods, 
78  
-            // giving methods precedence.
79  
-            $props = get_object_vars($view);
80  
-            foreach (get_class_methods($view) as $method) {
81  
-                if ('__' == substr($method, 0, 2)) {
82  
-                    // Omit magic methods
83  
-                    continue;
84  
-                }
85  
-                $props[$method] = array($view, $method);
86  
-            }
87  
-            $view = $props;
88  
-        }
89 76
         if (is_scalar($view)) {
90 77
             // Iteration over lists will sometimes involve scalars
91 78
             $inLoop = true;
@@ -111,7 +98,7 @@ public function render(array $tokens, $view, array $partials = null)
111 98
                     if (is_scalar($value)) {
112 99
                         $value = ('' === $value) ? '' : $this->escape($value);
113 100
                     } else {
114  
-                        $pragmaView = array_merge($view, array($data => $value));
  101
+                        $pragmaView = array($data => $value);
115 102
                         if ($test = $this->handlePragmas($type, $data, $pragmaView)) {
116 103
                             $value = $test;
117 104
                         } else {
@@ -363,10 +350,9 @@ public function getValue($key, $view)
363 350
             return '';
364 351
         }
365 352
         if (is_object($view)) {
366  
-            if (isset($view->$key)) {
367  
-                if (is_callable($view->$key) && $this->isValidCallback($view->$key)) {
368  
-                    return call_user_func($view->$key);
369  
-                }
  353
+            if (method_exists($view, $key)) {
  354
+                return call_user_func(array($view, $key));
  355
+            } else if (isset($view->$key)) {
370 356
                 return $view->$key;
371 357
             }
372 358
             return '';
@@ -467,21 +453,13 @@ protected function handlePragmas($token, $data, $view)
467 453
      */
468 454
     protected function isValidCallback($callback)
469 455
     {
470  
-        if (is_string($callback)) {
471  
-            if (strstr($callback, '::')) {
472  
-                // Referencing a static method call
473  
-                return true;
474  
-            }
475  
-            if (strstr($callback, '\\')) {
476  
-                // Referencing a namespaced function
477  
-                return true;
478  
-            }
479  
-
480  
-            // For security purposes, we don't want to call global functions
  456
+        if (is_string($callback) || is_array($callback)) {
  457
+            // For security purposes, we don't want to call anything that isn't
  458
+            // an object callback
481 459
             return false;
482 460
         }
483 461
 
484  
-        // Object or array callback -- always okay
  462
+        // Object callback -- always okay
485 463
         return true;
486 464
     }
487 465
 }
23  tests/PhlyTest/Mustache/MustacheTest.php
@@ -12,6 +12,8 @@
12 12
 /** @namespace */
13 13
 namespace PhlyTest\Mustache;
14 14
 
  15
+use Phly\Mustache\Pragma\ImplicitIterator;
  16
+
15 17
 use Phly\Mustache\Mustache,
16 18
     Phly\Mustache\Pragma;
17 19
 
@@ -573,4 +575,25 @@ public function testObjectPropertiesThatReferToPHPBuiltInsShouldNotCallThem()
573 575
         $test  = $this->mustache->render('template-referencing-php-function', $model);
574 576
         $this->assertEquals('time', trim($test));
575 577
     }
  578
+
  579
+    /**
  580
+     * @group injection-issues
  581
+     */
  582
+    public function testArrayValuesThatReferToStaticMethodsShouldNotCallThem()
  583
+    {
  584
+        $model = array('message' => 'DateTime::createFromFormat');
  585
+        $test = $this->mustache->render('template-referencing-php-function', $model);
  586
+        $this->assertEquals('DateTime::createFromFormat', trim($test));
  587
+    }
  588
+
  589
+    /**
  590
+     * @group injection-issues
  591
+     */
  592
+    public function testArrayValuesThatReferToStaticMethodsInArraySyntaxShouldNotCallThem()
  593
+    {
  594
+        $model = array('section' => array('DateTime', 'createFromFormat'));
  595
+        $this->mustache->getRenderer()->addPragma(new ImplicitIterator());
  596
+        $test = $this->mustache->render('template-referencing-static-function', $model);
  597
+        $this->assertEquals("DateTime\ncreateFromFormat", trim($test));
  598
+    }
576 599
 }
4  tests/PhlyTest/Mustache/templates/template-referencing-static-function.mustache
... ...
@@ -0,0 +1,4 @@
  1
+{{%IMPLICIT-ITERATOR}}
  2
+{{#section}}
  3
+{{.}}
  4
+{{/section}}

0 notes on commit 537a811

Please sign in to comment.
Something went wrong with that request. Please try again.