CVE-2021-41170: Insecure Inherited Permissions (CWE-277)
value: neoan3-apps/template is a neoan3 minimal template engine. Versions prior to 1.1.1 have allowed for passing in closures directly into the template engine. As a result values that are callable are executed by the template engine. The issue arises if a value has the same name as a method or function in scope and can therefore be executed either by mistake or maliciously. In theory all users of the package are affected as long as they either deal with direct user input or database values. A multi-step attack on is therefore plausible. Version 1.1.1 has addressed this vulnerability. Unfortunately only working with hardcoded values is safe in prior versions. As this likely defeats the purpose of a template engine, please upgrade.
CWE description: A product defines a set of insecure permissions that are inherited by objects that are created by the program.
CVSS v3 base score: 9.8
Published: 2021-11-08T19:15Z
Repository: https://github.com/sroehrl/neoan3-template
Language: PHP
Files changed: Template.php, TemplateFunctions.php, composer.json, tests/TemplateTest.php
Commit date: 2021-10-21 19:25:24 -0400
Commit message: SECURITY: allowing for direct injection (Issue #8)

Diff: diff --git a/Template.php b/Template.php
index f9a8613..746cb1d 100644
--- a/Template.php
+++ b/Template.php
@@ -22,16 +22,14 @@ class Template
         }
         $saveOpening = preg_quote(TemplateFunctions::getDelimiters()[0]);
         $saveClosing = preg_quote(TemplateFunctions::getDelimiters()[1]);
         foreach ($flatArray as $flatKey => $value){
             $flatKey = preg_replace('/[\/\.\\\+\*\?\[\^\]\$\(\)\{\}\=\!\<\>\|\:\-]/', "\\\\$0",$flatKey);
-            if(is_callable($value)){
-                TemplateFunctions::registerClosure($flatKey,$value);
-            } else {
-                $content = preg_replace("/$saveOpening\s*$flatKey\s*$saveClosing/", $value, $content);
-                $content = TemplateFunctions::tryClosures($flatArray, $content, false);
-            }
+            // PATCHED: direct function injection is not allowed anymore
+            $content = preg_replace("/$saveOpening\s*$flatKey\s*$saveClosing/", $value, $content);
+            $content = TemplateFunctions::tryClosures($flatArray, $content, false);
+
         }
 
         return $content;
     }
 
diff --git a/TemplateFunctions.php b/TemplateFunctions.php
index 783dc49..48941eb 100644
--- a/TemplateFunctions.php
+++ b/TemplateFunctions.php
@@ -101,11 +101,11 @@ class TemplateFunctions
     {
         $pattern = '/';
         if (!$pure) {
             $pattern .= preg_quote(self::$registeredDelimiters[0]) . "\s*";
         }
-        $pattern .= "$closureName\(([a-z0-9,\.\s]+)\)";
+        $pattern .= "$closureName\(([a-z0-9,\.\s_]+)\)";
         if (!$pure) {
             $pattern .= "\s*" . preg_quote(self::$registeredDelimiters[1]);
         }
         return $pattern . "/i";
     }
@@ -199,11 +199,10 @@ class TemplateFunctions
     {
         $bool = false;
         foreach ($flatArray as $key => $value) {
             $pattern = '/' . $key . '([^.]|$)/';
             if (preg_match($pattern, $expression, $matches)) {
-
                 switch (gettype($flatArray[$key])) {
                     case 'boolean':
                         $expression = str_replace($key, $flatArray[$key] ? 'true' : 'false', $expression);
                         break;
                     case 'NULL':
@@ -239,15 +238,16 @@ class TemplateFunctions
         $hits = $xPath->query("//*[@n-if]");
         if ($hits->length < 1) {
             return $content;
         }
 
+        $array = Template::flattenArray($array);
+        // important: first try closures
+        $array = array_merge(self::$registeredClosures, $array);
         foreach ($hits as $hit) {
             $expression = $hit->getAttribute('n-if');
-            $array = Template::flattenArray($array);
             $bool = self::evaluateTypedCondition($array, $expression);
-
             if (!$bool) {
                 $hit->parentNode->removeChild($hit);
             } else {
                 $hit->removeAttribute('n-if');
             }
diff --git a/composer.json b/composer.json
index 97ea57b..dffe8ac 100644
--- a/composer.json
+++ b/composer.json
@@ -1,15 +1,14 @@
 {
     "name": "neoan3-apps/template",
     "description": "neoan3 minimal template engine",
-    "version": "1.1.0",
+    "version": "1.1.1",
     "license": "MIT",
     "autoload": {
         "psr-4": {
             "Neoan3\\Apps\\": "./"
         }
-
     },
     "require": {
         "ext-openssl": "*",
         "ext-mbstring": "*",
         "ext-json": "*",
diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php
index 20e4e2e..dda1d75 100644
--- a/tests/TemplateTest.php
+++ b/tests/TemplateTest.php
@@ -111,15 +111,15 @@ class TemplateTest extends TestCase
     }
 
     public function testCallback()
     {
         $array = [
-            'myFunc' => function ($x) {
-                return strtoupper($x);
-            },
             'some' => 'value'
         ];
+        TemplateFunctions::registerClosure('myFunc',function($x){
+            return strtoupper($x);
+        });
         $t = Template::embraceFromFile('callback.html', $array);
 
         $this->assertStringContainsString('<p>VALUE</p>', $t);
         $this->assertStringContainsString('<li>show me</li>', $t);
     }

Vulnerable code:             if(is_callable($value)){
                TemplateFunctions::registerClosure($flatKey,$value);
            } else {
                $content = preg_replace("/$saveOpening\s*$flatKey\s*$saveClosing/", $value, $content);
                $content = TemplateFunctions::tryClosures($flatArray, $content, false);
            }
        $pattern .= "$closureName\(([a-z0-9,\.\s]+)\)";

            $array = Template::flattenArray($array);

    "version": "1.1.0",

            'myFunc' => function ($x) {
                return strtoupper($x);
            },
Fixed code:             // PATCHED: direct function injection is not allowed anymore
            $content = preg_replace("/$saveOpening\s*$flatKey\s*$saveClosing/", $value, $content);
            $content = TemplateFunctions::tryClosures($flatArray, $content, false);

        $pattern .= "$closureName\(([a-z0-9,\.\s_]+)\)";
        $array = Template::flattenArray($array);
        // important: first try closures
        $array = array_merge(self::$registeredClosures, $array);
    "version": "1.1.1",
        TemplateFunctions::registerClosure('myFunc',function($x){
            return strtoupper($x);
        });