[SECURITY] Prevent XSS with fe_users data in felogin/TSFE

Two occurrences allow to render data of the currently logged in
frontend user that is not sanitized and thus allow XSS attacks
by frontend users.

1. EXT:fe_login adds ###FEUSER_{fieldname}### for each
field that exists in the fe_users DB table, which CAN be processed
by TypoScript but is insecure by default.

2. config.USERNAME_substToken = <!--###USERNAME###-->
sets the username dynamically, which is then insecure.

Adding htmlspecialchars as a default configuration
solves this problem.

Resolves: #87053
Releases: master, 8.7, 7.6
Security-Commit: 7f7a326fc656360ffec71415d730e40df99d63a0
Security-Bulletin: TYPO3-CORE-SA-2018-008
Change-Id: I973e350b727d20d137dd70f755913d02e8f5644e
Reviewed-on: https://review.typo3.org/59086
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>

typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php

@@ -932,7 +932,9 @@ protected function getUserFieldMarkers()
         if ($this->frontendController->fe_user->user) {
             // All fields of fe_user will be replaced, scheme is ###FEUSER_FIELDNAME###
             foreach ($this->frontendController->fe_user->user as $field => $value) {
-                $marker['###FEUSER_' . GeneralUtility::strtoupper($field) . '###'] = $this->cObj->stdWrap($value, $this->conf['userfields.'][$field . '.']);
+                $conf = isset($this->conf['userfields.'][$field . '.']) ? $this->conf['userfields.'][$field . '.'] : [];
+                $conf = array_replace_recursive(['htmlSpecialChars' => '1'], $conf);
+                $marker['###FEUSER_' . GeneralUtility::strtoupper($field) . '###'] = $this->cObj->stdWrap($value, $conf);
             }
             // Add ###USER### for compatibility
             $marker['###USER###'] = $marker['###FEUSER_USERNAME###'];

typo3/sysext/felogin/Tests/Unit/Controller/FrontendLoginControllerTest.php

@@ -14,7 +14,10 @@
  * The TYPO3 project - inspiring people to share!
  */
 
+use Prophecy\Argument;
+use TYPO3\CMS\Core\Charset\CharsetConverter;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 
 /**
@@ -489,4 +492,106 @@ public function processRedirectReferrerDomainsMatchesDomains()
         $tsfe->loginUser = true;
         $this->assertSame(['http://www.example.com/snafu'], $this->accessibleFixture->_call('processRedirect'));
     }
+
+    /**
+     *
+     */
+    public function processUserFieldsRespectsDefaultConfigurationForStdWrapDataProvider()
+    {
+        return [
+            'Simple casing' => [
+                [
+                    'username' => 'Holy',
+                    'lastname' => 'Wood',
+                ],
+                [
+                    'username.' => [
+                        'case' => 'upper'
+                    ]
+                ],
+                [
+                    '###FEUSER_USERNAME###' => 'HOLY',
+                    '###FEUSER_LASTNAME###' => 'Wood',
+                    '###USER###' => 'HOLY'
+                ]
+            ],
+            'Default config applies' => [
+                [
+                    'username' => 'Holy',
+                    'lastname' => 'O" Mally',
+                ],
+                [
+                    'username.' => [
+                        'case' => 'upper'
+                    ]
+                ],
+                [
+                    '###FEUSER_USERNAME###' => 'HOLY',
+                    '###FEUSER_LASTNAME###' => 'O" Mally',
+                    '###USER###' => 'HOLY'
+                ]
+            ],
+            'Specific config overrides default config' => [
+                [
+                    'username' => 'Holy',
+                    'lastname' => 'O" Mally',
+                ],
+                [
+                    'username.' => [
+                        'case' => 'upper'
+                    ],
+                    'lastname.' => [
+                        'htmlSpecialChars' => '0'
+                    ]
+                ],
+                [
+                    '###FEUSER_USERNAME###' => 'HOLY',
+                    '###FEUSER_LASTNAME###' => 'O" Mally',
+                    '###USER###' => 'HOLY'
+                ]
+            ],
+            'No given user returns empty array' => [
+                null,
+                [
+                    'username.' => [
+                        'case' => 'upper'
+                    ],
+                    'lastname.' => [
+                        'htmlSpecialChars' => '0'
+                    ]
+                ],
+                []
+            ],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider processUserFieldsRespectsDefaultConfigurationForStdWrapDataProvider
+     */
+    public function processUserFieldsRespectsDefaultConfigurationForStdWrap($userRecord, $fieldConf, $expectedMarkers)
+    {
+        $charsetConverter = $this->prophesize(CharsetConverter::class);
+        $charsetConverter->conv_case('utf-8', Argument::any(), 'toUpper')
+            ->will(function(array $args) {
+                return mb_strtoupper($args[1]);
+            });
+        /** @var TypoScriptFrontendController $tsfe */
+        $tsfe = $this->getMock(
+            TypoScriptFrontendController::class,
+            [],
+            [],
+            '',
+            false
+        );
+        $tsfe->csConvObj = $charsetConverter->reveal();
+        $tsfe->fe_user = new \stdClass();
+        $tsfe->fe_user->user = $userRecord;
+        $conf = ['userfields.' => $fieldConf];
+        $this->accessibleFixture->_set('cObj', new ContentObjectRenderer($tsfe));
+        $this->accessibleFixture->_set('frontendController', $tsfe);
+        $this->accessibleFixture->_set('conf', $conf);
+        $actualResult = $this->accessibleFixture->_call('getUserFieldMarkers');
+        $this->assertEquals($expectedMarkers, $actualResult);
+    }
 }

typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php

@@ -3839,7 +3839,7 @@ public function contentStrReplace()
             // User name:
             $token = isset($this->config['config']['USERNAME_substToken']) ? trim($this->config['config']['USERNAME_substToken']) : '';
             $search[] = $token ? $token : '<!--###USERNAME###-->';
-            $replace[] = $this->fe_user->user['username'];
+            $replace[] = htmlspecialchars($this->fe_user->user['username']);
             // User uid (if configured):
             $token = isset($this->config['config']['USERUID_substToken']) ? trim($this->config['config']['USERUID_substToken']) : '';
             if ($token) {