From 6863f73818c36b0b88c677ba533765c8074907b4 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Tue, 13 Sep 2022 10:09:57 +0200
Subject: [PATCH] [SECURITY] Encode child node variables in f:asset.css view
 helper

Variables in child nodes like `<f:asset.css>{value}</f:asset.css>`
were not encoded and allow cross-site scripting. In case values shall
be taken as is, corresponding `f:format.raw` instruction has to be used.

Resolves: #97900
Releases: main, 11.5, 10.4
Change-Id: Id843a41c42bbe1f74cdc4efbc117b24d20026b97
Security-Bulletin: TYPO3-CORE-SA-2022-010
Security-References: CVE-2022-36108
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75719
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
 .../ViewHelpers/Asset/CssViewHelper.php       |  2 +-
 .../ViewHelpers/Asset/CssViewHelperTest.php   | 74 +++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/typo3/sysext/fluid/Classes/ViewHelpers/Asset/CssViewHelper.php b/typo3/sysext/fluid/Classes/ViewHelpers/Asset/CssViewHelper.php
index 3de645353f40..320ac8412d1f 100644
--- a/typo3/sysext/fluid/Classes/ViewHelpers/Asset/CssViewHelper.php
+++ b/typo3/sysext/fluid/Classes/ViewHelpers/Asset/CssViewHelper.php
@@ -51,7 +51,7 @@ final class CssViewHelper extends AbstractTagBasedViewHelper
      *
      * @var bool
      */
-    protected $escapeChildren = false;
+    protected $escapeChildren = true;
 
     protected AssetCollector $assetCollector;
 
diff --git a/typo3/sysext/fluid/Tests/Functional/ViewHelpers/Asset/CssViewHelperTest.php b/typo3/sysext/fluid/Tests/Functional/ViewHelpers/Asset/CssViewHelperTest.php
index be388d5a7ba9..0b2095c8df06 100644
--- a/typo3/sysext/fluid/Tests/Functional/ViewHelpers/Asset/CssViewHelperTest.php
+++ b/typo3/sysext/fluid/Tests/Functional/ViewHelpers/Asset/CssViewHelperTest.php
@@ -18,6 +18,8 @@ declare(strict_types=1);
 namespace TYPO3\CMS\Fluid\Tests\Functional\ViewHelpers\Asset;
 
 use TYPO3\CMS\Core\Page\AssetCollector;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Fluid\View\StandaloneView;
 use TYPO3\CMS\Fluid\ViewHelpers\Asset\CssViewHelper;
 use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
 
@@ -79,4 +81,76 @@ class CssViewHelperTest extends FunctionalTestCase
         self::assertSame($collectedJavaScripts['test']['source'], 'my.css');
         self::assertSame($collectedJavaScripts['test']['attributes'], ['disabled' => 'disabled']);
     }
+
+    public static function childNodeRenderingIsCorrectDataProvider(): array
+    {
+        return [
+            // Double quotes
+            'variable with double quotes is encoded' => [
+                '</style>/* " ', // variable value
+                'body { color: #{color}; }', // inner template source
+                'body { color: #&lt;/style&gt;/* &quot; ; }', // expectation
+            ],
+            'variable with double quotes is encoded in single quotes' => [
+                '</style>/* " ', // variable value
+                'body { color: \'#{color}\'; }', // inner template source
+                'body { color: \'#&lt;/style&gt;/* &quot; \'; }', // expectation
+            ],
+            'variable with double quotes is encoded in double quotes' => [
+                '</style>/* " ', // variable value
+                'body { color: "#{color}"; }', // inner template source
+                'body { color: "#&lt;/style&gt;/* &quot; "; }', // expectation
+            ],
+            // Single quotes
+            'variable with single quotes is encoded' => [
+                '</style>/* \' ', // variable value
+                'body { color: #{color}; }', // inner template source
+                'body { color: #&lt;/style&gt;/* &#039; ; }', // expectation
+            ],
+            'variable with single quotes is encoded in single quotes' => [
+                '</style>/* \' ', // variable value
+                'body { color: \'#{color}\'; }', // inner template source
+                'body { color: \'#&lt;/style&gt;/* &#039; \'; }', // expectation
+            ],
+            'variable with single quotes is encoded in double quotes' => [
+                '</style>/* \' ', // variable value
+                'body { color: "#{color}"; }', // inner template source
+                'body { color: "#&lt;/style&gt;/* &#039; "; }', // expectation
+            ],
+            // Raw instruction
+            'raw instruction is passed' => [
+                '</style>/* " ',
+                'body { color: #{color -> f:format.raw()}; }',
+                'body { color: #</style>/* " ; }',
+            ],
+            'raw instruction is passed in sigle quotes' => [
+                '</style>/* " ',
+                'body { color: \'#{color -> f:format.raw()}\'; }',
+                'body { color: \'#</style>/* " \'; }',
+            ],
+            'raw instruction is passed in double quotes' => [
+                '</style>/* " ',
+                'body { color: "#{color -> f:format.raw()}"; }',
+                'body { color: "#</style>/* " "; }',
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider childNodeRenderingIsCorrectDataProvider
+     */
+    public function childNodeRenderingIsCorrect(string $value, string $source, string $expectation): void
+    {
+        $assetCollector = new AssetCollector();
+        GeneralUtility::setSingletonInstance(AssetCollector::class, $assetCollector);
+
+        $view = new StandaloneView();
+        $view->setTemplateSource(sprintf('<f:asset.css identifier="test">%s</f:asset.css>', $source));
+        $view->assign('color', $value);
+        $view->render();
+        GeneralUtility::removeSingletonInstance(AssetCollector::class, $assetCollector);
+
+        self::assertSame($expectation, $assetCollector->getInlineStyleSheets()['test']['source']);
+    }
 }
-- 
GitLab