[TASK] Allow nonce values explicitly in PageRenderer inline methods

Allow to use nonce explicitly for `PageRenderer` methods `addJsInlineCode()`, `addJsFooterInlineCode()` and `addCssInlineBlock()`. The backend login styles are already using those nonce values. As a consequence, the variable values `loginBackgroundImage` and `loginHighlightColor` have to be sanitized by the new `GeneralUtility::sanitizeCssVariableValue()` function. Resolves: #100664 Releases: main, 12.4 Change-Id: I4b79c1d6f13ea32d8d880c11b4299207411ad289 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79167 Tested-by: core-ci <typo3@b13.com> Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · Jun 3, 2023 · 17a2f98 · 17a2f98
1 parent f8d048d
commit 17a2f98
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 50 deletions.
diff --git a/typo3/sysext/backend/Classes/Controller/LoginController.php b/typo3/sysext/backend/Classes/Controller/LoginController.php
@@ -247,20 +247,20 @@ protected function provideCustomLoginStyling(): void
         $languageService = $this->getLanguageService();
         $authenticationStyleInformation = GeneralUtility::makeInstance(AuthenticationStyleInformation::class);
         if (($backgroundImageStyles = $authenticationStyleInformation->getBackgroundImageStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles);
+            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles, useNonce: true);
         }
         if (($footerNote = $authenticationStyleInformation->getFooterNote()) !== '') {
             $this->view->assign('loginFootnote', $footerNote);
         }
         if (($highlightColorStyles = $authenticationStyleInformation->getHighlightColorStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles);
+            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles, useNonce: true);
         }
         if (($logo = $authenticationStyleInformation->getLogo()) !== '') {
             $logoAlt = $authenticationStyleInformation->getLogoAlt() ?: $languageService->sL('LLL:EXT:backend/Resources/Private/Language/locallang_login.xlf:typo3.altText');
         } else {
             $logo = $authenticationStyleInformation->getDefaultLogo();
             $logoAlt = $languageService->sL('LLL:EXT:backend/Resources/Private/Language/locallang_login.xlf:typo3.altText');
-            $this->pageRenderer->addCssInlineBlock('loginLogo', $authenticationStyleInformation->getDefaultLogoStyles());
+            $this->pageRenderer->addCssInlineBlock('loginLogo', $authenticationStyleInformation->getDefaultLogoStyles(), useNonce: true);
         }
         $this->view->assignMultiple([
             'logo' => $logo,

diff --git a/typo3/sysext/backend/Classes/Controller/MfaController.php b/typo3/sysext/backend/Classes/Controller/MfaController.php
@@ -217,10 +217,10 @@ protected function getMfaProviderFromRequest(ServerRequestInterface $request): ?
     protected function addCustomAuthenticationFormStyles(): void
     {
         if (($backgroundImageStyles = $this->authenticationStyleInformation->getBackgroundImageStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles);
+            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles, useNonce: true);
         }
         if (($highlightColorStyles = $this->authenticationStyleInformation->getHighlightColorStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles);
+            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles, useNonce: true);
         }
     }
 }
diff --git a/typo3/sysext/backend/Classes/Controller/MfaSetupController.php b/typo3/sysext/backend/Classes/Controller/MfaSetupController.php
@@ -222,10 +222,10 @@ protected function initializeView(ServerRequestInterface $request): ViewInterfac
     protected function addCustomAuthenticationFormStyles(): void
     {
         if (($backgroundImageStyles = $this->authenticationStyleInformation->getBackgroundImageStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles);
+            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles, useNonce: true);
         }
         if (($highlightColorStyles = $this->authenticationStyleInformation->getHighlightColorStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles);
+            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles, useNonce: true);
         }
     }
 

diff --git a/typo3/sysext/backend/Classes/Controller/ResetPasswordController.php b/typo3/sysext/backend/Classes/Controller/ResetPasswordController.php
@@ -222,21 +222,21 @@ protected function provideCustomLoginStyling(): void
     {
         $languageService = $this->getLanguageService();
         if (($backgroundImageStyles = $this->authenticationStyleInformation->getBackgroundImageStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles);
+            $this->pageRenderer->addCssInlineBlock('loginBackgroundImage', $backgroundImageStyles, useNonce: true);
         }
         if (($footerNote = $this->authenticationStyleInformation->getFooterNote()) !== '') {
             $this->view->assign('loginFootnote', $footerNote);
         }
         if (($highlightColorStyles = $this->authenticationStyleInformation->getHighlightColorStyles()) !== '') {
-            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles);
+            $this->pageRenderer->addCssInlineBlock('loginHighlightColor', $highlightColorStyles, useNonce: true);
         }
         if (($logo = $this->authenticationStyleInformation->getLogo()) !== '') {
             $logoAlt = $this->authenticationStyleInformation->getLogoAlt()
                 ?: $languageService->sL('LLL:EXT:backend/Resources/Private/Language/locallang_login.xlf:typo3.altText');
         } else {
             $logo = $this->authenticationStyleInformation->getDefaultLogo();
             $logoAlt = $languageService->sL('LLL:EXT:backend/Resources/Private/Language/locallang_login.xlf:typo3.altText');
-            $this->pageRenderer->addCssInlineBlock('loginLogo', $this->authenticationStyleInformation->getDefaultLogoStyles());
+            $this->pageRenderer->addCssInlineBlock('loginLogo', $this->authenticationStyleInformation->getDefaultLogoStyles(), useNonce: true);
         }
         $this->view->assignMultiple([
             'logo' => $logo,

diff --git a/typo3/sysext/backend/Classes/View/AuthenticationStyleInformation.php b/typo3/sysext/backend/Classes/View/AuthenticationStyleInformation.php
@@ -58,7 +58,7 @@ public function getBackgroundImageStyles(): string
             .typo3-login-carousel-control.right,
             .typo3-login-carousel-control.left,
             .card-login { border: 0; }
-            .typo3-login { background-image: url("' . $backgroundImageUri . '"); }
+            .typo3-login { background-image: url("' . GeneralUtility::sanitizeCssVariableValue($backgroundImageUri) . '"); }
             .typo3-login-footnote { background-color: #000000; color: #ffffff; opacity: 0.5; }
         ';
     }
@@ -79,8 +79,8 @@ public function getHighlightColorStyles(): string
             .btn-login.disabled.active, .btn-login[disabled].active, fieldset[disabled] .btn-login.active,
             .btn-login:hover, .btn-login:focus, .btn-login:active,
             .btn-login:active:hover, .btn-login:active:focus,
-            .btn-login { background-color: ' . $highlightColor . '; }
-            .card-login .card-footer { border-color: ' . $highlightColor . '; }
+            .btn-login { background-color: ' . GeneralUtility::sanitizeCssVariableValue($highlightColor) . '; }
+            .card-login .card-footer { border-color: ' . GeneralUtility::sanitizeCssVariableValue($highlightColor) . '; }
         ';
     }
 

diff --git a/typo3/sysext/core/Classes/Page/PageRenderer.php b/typo3/sysext/core/Classes/Page/PageRenderer.php
@@ -1310,14 +1310,15 @@ public function addJsFooterFile($file, $type = '', $compress = true, $forceOnTop
      * @param bool $compress
      * @param bool $forceOnTop
      */
-    public function addJsInlineCode($name, $block, $compress = true, $forceOnTop = false)
+    public function addJsInlineCode($name, $block, $compress = true, $forceOnTop = false, bool $useNonce = false)
     {
         if (!isset($this->jsInline[$name]) && !empty($block)) {
             $this->jsInline[$name] = [
                 'code' => $block . LF,
                 'section' => self::PART_HEADER,
                 'compress' => $compress,
                 'forceOnTop' => $forceOnTop,
+                'useNonce' => $useNonce,
             ];
         }
     }
@@ -1330,14 +1331,15 @@ public function addJsInlineCode($name, $block, $compress = true, $forceOnTop = f
      * @param bool $compress
      * @param bool $forceOnTop
      */
-    public function addJsFooterInlineCode($name, $block, $compress = true, $forceOnTop = false)
+    public function addJsFooterInlineCode($name, $block, $compress = true, $forceOnTop = false, bool $useNonce = false)
     {
         if (!isset($this->jsInline[$name]) && !empty($block)) {
             $this->jsInline[$name] = [
                 'code' => $block . LF,
                 'section' => self::PART_FOOTER,
                 'compress' => $compress,
                 'forceOnTop' => $forceOnTop,
+                'useNonce' => $useNonce,
             ];
         }
     }
@@ -1418,13 +1420,14 @@ public function addCssLibrary($file, $rel = 'stylesheet', $media = 'all', $title
      * @param bool $compress
      * @param bool $forceOnTop
      */
-    public function addCssInlineBlock($name, $block, $compress = false, $forceOnTop = false)
+    public function addCssInlineBlock($name, $block, $compress = false, $forceOnTop = false, bool $useNonce = false)
     {
         if (!isset($this->cssInline[$name]) && !empty($block)) {
             $this->cssInline[$name] = [
                 'code' => $block,
                 'compress' => $compress,
                 'forceOnTop' => $forceOnTop,
+                'useNonce' => $useNonce,
             ];
         }
     }
@@ -2156,7 +2159,6 @@ protected function renderMainJavaScriptLibraries()
             if ($this->getApplicationType() === 'BE') {
                 $this->javaScriptRenderer->addGlobalAssignment(['TYPO3' => $assignments]);
             } else {
-                // @todo apply nonce for CSP (means dropping static `inlineJavascriptWrap`)
                 $out .= $this->wrapInlineScript(
                     sprintf(
                         "var TYPO3 = Object.assign(TYPO3 || {}, %s);\r\n",
@@ -2166,7 +2168,8 @@ protected function renderMainJavaScriptLibraries()
                             . "!['__proto__', 'prototype', 'constructor'].includes(entry[0])))",
                             json_encode($assignments)
                         )
-                    )
+                    ),
+                    $this->nonce !== null ? ['nonce' => $this->nonce->consume()] : []
                 );
             }
         }
@@ -2345,19 +2348,25 @@ private function createCssTag(array $properties, string $file): string
      */
     protected function renderCssInline()
     {
-        $cssInline = '';
-        if (!empty($this->cssInline)) {
-            foreach ($this->cssInline as $name => $properties) {
-                $cssCode = '/*' . htmlspecialchars($name) . '*/' . LF . ($properties['code'] ?? '') . LF;
-                if ($properties['forceOnTop'] ?? false) {
-                    $cssInline = $cssCode . $cssInline;
-                } else {
-                    $cssInline .= $cssCode;
-                }
+        if (empty($this->cssInline)) {
+            return '';
+        }
+        $cssItems = [0 => [], 1 => []];
+        foreach ($this->cssInline as $name => $properties) {
+            $nonceKey = (int)(!empty($properties['useNonce']));
+            $cssCode = '/*' . htmlspecialchars($name) . '*/' . LF . ($properties['code'] ?? '') . LF;
+            if ($properties['forceOnTop'] ?? false) {
+                array_unshift($cssItems[$nonceKey], $cssCode);
+            } else {
+                $cssItems[$nonceKey][] = $cssCode;
             }
-            $cssInline = $this->wrapInlineStyle($cssInline);
         }
-        return $cssInline;
+        $cssItems = array_filter($cssItems);
+        foreach ($cssItems as $useNonce => $items) {
+            $attributes = $useNonce && $this->nonce !== null ? ['nonce' => $this->nonce->consume()] : [];
+            $cssItems[$useNonce] = $this->wrapInlineStyle(implode('', $items), $attributes);
+        }
+        return implode(LF, $cssItems);
     }
 
     /**
@@ -2491,35 +2500,43 @@ protected function renderJavaScriptFiles()
      */
     protected function renderInlineJavaScript()
     {
-        $jsInline = '';
-        $jsFooterInline = '';
-        if (!empty($this->jsInline)) {
-            foreach ($this->jsInline as $name => $properties) {
-                $jsCode = '/*' . htmlspecialchars($name) . '*/' . LF . ($properties['code'] ?? '') . LF;
-                if ($properties['forceOnTop'] ?? false) {
-                    if (($properties['section'] ?? 0) === self::PART_HEADER) {
-                        $jsInline = $jsCode . $jsInline;
-                    } else {
-                        $jsFooterInline = $jsCode . $jsFooterInline;
-                    }
-                } elseif (($properties['section'] ?? 0) === self::PART_HEADER) {
-                    $jsInline .= $jsCode;
+        if (empty($this->jsInline)) {
+            return ['', ''];
+        }
+        $regularItems = [0 => [], 1 => []];
+        $footerItems = [0 => [], 1 => []];
+        foreach ($this->jsInline as $name => $properties) {
+            $nonceKey = (int)(!empty($properties['useNonce'])); // 0 or 1
+            $jsCode = '/*' . htmlspecialchars($name) . '*/' . LF . ($properties['code'] ?? '') . LF;
+            if ($properties['forceOnTop'] ?? false) {
+                if (($properties['section'] ?? 0) === self::PART_HEADER) {
+                    array_unshift($regularItems[$nonceKey], $jsCode);
                 } else {
-                    $jsFooterInline .= $jsCode;
+                    array_unshift($footerItems[$nonceKey], $jsCode);
                 }
+            } elseif (($properties['section'] ?? 0) === self::PART_HEADER) {
+                $regularItems[$nonceKey][] = $jsCode;
+            } else {
+                $footerItems[$nonceKey][] = $jsCode;
             }
         }
-        if ($jsInline) {
-            $jsInline = $this->wrapInlineScript($jsInline);
+        $regularItems = array_filter($regularItems);
+        $footerItems = array_filter($footerItems);
+        foreach ($regularItems as $useNonce => $items) {
+            $attributes = $useNonce && $this->nonce !== null ? ['nonce' => $this->nonce->consume()] : [];
+            $regularItems[$useNonce] = $this->wrapInlineScript(implode('', $items), $attributes);
         }
-        if ($jsFooterInline) {
-            $jsFooterInline = $this->wrapInlineScript($jsFooterInline);
+        foreach ($footerItems as $useNonce => $items) {
+            $attributes = $useNonce && $this->nonce !== null ? ['nonce' => $this->nonce->consume()] : [];
+            $footerItems[$useNonce] = $this->wrapInlineScript(implode('', $items), $attributes);
         }
+        $regularCode = implode(LF, $regularItems);
+        $footerCode = implode(LF, $footerItems);
         if ($this->moveJsFromHeaderToFooter) {
-            $jsFooterInline = $jsInline . $jsFooterInline;
-            $jsInline = '';
+            $footerCode = $regularCode . $footerCode;
+            $regularCode = '';
         }
-        return [$jsInline, $jsFooterInline];
+        return [$regularCode, $footerCode];
     }
 
     /**

diff --git a/typo3/sysext/core/Classes/Utility/GeneralUtility.php b/typo3/sysext/core/Classes/Utility/GeneralUtility.php
@@ -3330,6 +3330,17 @@ public static function jsonEncodeForJavaScript($value): string
         );
     }
 
+    /**
+     * Very, very, very basic CSS sanitizer which removes `{`, `}`, `\n`, `\r`
+     * from CSS variable values and encodes potential HTML entities `<`+`>`.
+     */
+    public static function sanitizeCssVariableValue(string $value): string
+    {
+        $value = str_replace(['{', '}', "\n", "\r"], '', $value);
+        // keep quotes, e.g. for `background: url("/res/background.png")`
+        return htmlspecialchars($value, ENT_SUBSTITUTE);
+    }
+
     /**
      * @return LoggerInterface
      */

diff --git a/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php b/typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php
@@ -1905,6 +1905,23 @@ public function jsonEncodeForJavaScriptTest($value, string $expectation): void
         self::assertSame($expectation, GeneralUtility::jsonEncodeForJavaScript($value));
     }
 
+    public static function sanitizeCssVariableValueDataProvider(): \Generator
+    {
+        yield 'double quotes' => ['url("/my-background.png")', 'url("/my-background.png")'];
+        yield 'single quotes' => ["url('/my-background.png')", "url('/my-background.png')"];
+        yield 'newline chars' => ["url('/my-background.png'\r\n\r\n)", "url('/my-background.png')"];
+        yield 'HTML markup' => ['url(</style>)', 'url(&lt;/style&gt;)'];
+    }
+
+    /**
+     * @test
+     * @dataProvider sanitizeCssVariableValueDataProvider
+     */
+    public function sanitizeCssVariableValue(string $value, string $expectation): void
+    {
+        self::assertSame($expectation, GeneralUtility::sanitizeCssVariableValue($value));
+    }
+
     ///////////////////////////////
     // Tests concerning fixPermissions
     ///////////////////////////////