[BUGFIX] Ensure escaping of escapable ExpressionNode

Prevents a potential security issue when expression nodes are used to output variables, in which case, they would not be properly escaped. The fix implements escaping interception for these expression nodes. https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:H/PR:N/UI:R/S:C/C:L/I:L/A:N/E:F/RL:O/RC:C
thomashohn · May 7, 2019 · e4ca051 · e4ca051
1 parent 19a195c
commit e4ca051
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 3 deletions.
diff --git a/examples/Resources/Private/Singles/Variables.html b/examples/Resources/Private/Singles/Variables.html
@@ -29,6 +29,7 @@
 
 <!-- Passing arguments to sections/partials -->
 <f:render section="Secondary" arguments="{
+    ternaryCheck: 1,
 	myVariable: 'Nice string',
 	array: {
 		baz: 42,
@@ -43,6 +44,8 @@
 </f:section>
 
 <f:section name="Secondary">
+Escaped ternary expression: {ternaryCheck ? array.foobar : array.foobar}
+Escaped cast expression: {array.foobar as string}
 Received $array.foobar with value {array.foobar -> f:format.raw()} (same using "value" argument: {f:format.raw(value: array.foobar)})
 Received $array.printf with formatted string {array.printf -> f:format.printf(arguments: {0: 'formatted'})}
 Received $array.baz with value {array.baz}

diff --git a/src/Core/Parser/Interceptor/Escape.php b/src/Core/Parser/Interceptor/Escape.php
@@ -9,6 +9,7 @@
 use TYPO3Fluid\Fluid\Core\Parser\InterceptorInterface;
 use TYPO3Fluid\Fluid\Core\Parser\ParsingState;
 use TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\EscapingNode;
+use TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\Expression\ExpressionNodeInterface;
 use TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\NodeInterface;
 use TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\ObjectAccessorNode;
 use TYPO3Fluid\Fluid\Core\Parser\SyntaxTree\ViewHelperNode;
@@ -62,7 +63,7 @@ public function process(NodeInterface $node, $interceptorPosition, ParsingState
             if ($this->childrenEscapingEnabled && $node->getUninitializedViewHelper()->isOutputEscapingEnabled()) {
                 $node = new EscapingNode($node);
             }
-        } elseif ($this->childrenEscapingEnabled && $node instanceof ObjectAccessorNode) {
+        } elseif ($this->childrenEscapingEnabled && ($node instanceof ObjectAccessorNode || $node instanceof ExpressionNodeInterface)) {
             $node = new EscapingNode($node);
         }
         return $node;
@@ -78,7 +79,8 @@ public function getInterceptionPoints()
         return [
             InterceptorInterface::INTERCEPT_OPENING_VIEWHELPER,
             InterceptorInterface::INTERCEPT_CLOSING_VIEWHELPER,
-            InterceptorInterface::INTERCEPT_OBJECTACCESSOR
+            InterceptorInterface::INTERCEPT_OBJECTACCESSOR,
+            InterceptorInterface::INTERCEPT_EXPRESSION,
         ];
     }
 }
diff --git a/src/Core/Parser/InterceptorInterface.php b/src/Core/Parser/InterceptorInterface.php
@@ -19,6 +19,7 @@ interface InterceptorInterface
     const INTERCEPT_CLOSING_VIEWHELPER = 2;
     const INTERCEPT_TEXT = 3;
     const INTERCEPT_OBJECTACCESSOR = 4;
+    const INTERCEPT_EXPRESSION = 5;
 
     /**
      * The interceptor can process the given node at will and must return a node

diff --git a/src/Core/Parser/TemplateParser.php b/src/Core/Parser/TemplateParser.php
@@ -625,6 +625,8 @@ protected function textAndShorthandSyntaxHandler(ParsingState $state, $text, $co
                                 if ($expressionStartPosition > 0) {
                                     $state->getNodeFromStack()->addChildNode(new TextNode(substr($section, 0, $expressionStartPosition)));
                                 }
+
+                                $this->callInterceptor($expressionNode, InterceptorInterface::INTERCEPT_EXPRESSION, $state);
                                 $state->getNodeFromStack()->addChildNode($expressionNode);
 
                                 $expressionEndPosition = $expressionStartPosition + strlen($matchedVariableSet[0]);

diff --git a/tests/Functional/ExamplesTest.php b/tests/Functional/ExamplesTest.php
@@ -214,6 +214,8 @@ public function getExampleScriptTestValues()
                     'Output of variable whose name is stored in a variable: string foo',
                     'Direct access of numeric prefixed variable: Numeric prefixed variable',
                     'Aliased access of numeric prefixed variable: Numeric prefixed variable',
+                    'Escaped ternary expression: &lt;b&gt;Unescaped string&lt;/b&gt;',
+                    'Escaped cast expression: &lt;b&gt;Unescaped string&lt;/b&gt;',
                     'Received $array.foobar with value <b>Unescaped string</b> (same using "value" argument: <b>Unescaped string</b>)',
                     'Received $array.printf with formatted string Formatted string, value: formatted',
                     'Received $array.baz with value 42',
@@ -256,7 +258,7 @@ public function getExampleScriptTestValues()
                     'ViewHelper error: Undeclared arguments passed to ViewHelper TYPO3Fluid\Fluid\ViewHelpers\IfViewHelper: notregistered. Valid arguments are: then, else, condition - Offending code: <f:if notregistered="1" />',
                     'Parser error: The ViewHelper "<f:invalid>" could not be resolved.',
                     'Based on your spelling, the system would load the class "TYPO3Fluid\Fluid\ViewHelpers\InvalidViewHelper", however this class does not exist. Offending code: <f:invalid />',
-                    'Invalid expression: Invalid target conversion type "invalidtype" specified in casting expression "{foobar as invalidtype}".',
+                    'Invalid expression: Invalid target conversion type &quot;invalidtype&quot; specified in casting expression &quot;{foobar as invalidtype}&quot;.',
                 ]
             ]
         ];