[SECURITY] Avoid XSS by correctly encoding typolink results

In order to avoid XSS through typolink, anchor text is encoded correctly to be used in a HTML context. Fallback link texts of links to pages are encoded per default in case lib.parseFunc has not been configured. Resolves: #88635 Releases: master, 9.5, 8.7 Security-Commit: 1bd95ac9a73260b0edd5e0608fb0207d52daa3e7 Security-Bulletin: TYPO3-CORE-SA-2019-022 Change-Id: I53c884e748a825c47cdfcfdcfa92dc249f3986a5 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62710 Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
TYPO3 · Dec 17, 2019 · 25f796b · 25f796b
1 parent e971b01
commit 25f796b
Show file tree

Hide file tree

Showing 8 changed files with 509 additions and 32 deletions.
diff --git a/typo3/sysext/core/Classes/LinkHandling/UrlLinkHandler.php b/typo3/sysext/core/Classes/LinkHandling/UrlLinkHandler.php
@@ -54,6 +54,7 @@ protected function addHttpSchemeAsFallback(string $url): string
             $scheme = parse_url($url, PHP_URL_SCHEME);
             if (empty($scheme)) {
                 $url = 'http://' . $url;
+            // 'java{TAB}script:' is parsed as empty URL scheme, thus not ending up here
             } elseif (in_array(strtolower($scheme), ['javascript', 'data'], true)) {
                 // deny using insecure scheme's like `javascript:` or `data:` as URL scheme
                 $url = '';

diff --git a/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php b/typo3/sysext/frontend/Classes/ContentObject/ContentObjectRenderer.php
@@ -4923,7 +4923,7 @@ protected function resolveMixedLinkParameter($linkText, $mixedLinkParameter, &$c
                 // Resource was not found
                 return $linkText;
             }
-        } elseif (in_array(strtolower(trim($linkHandlerKeyword)), ['javascript', 'data'], true)) {
+        } elseif (in_array(strtolower(preg_replace('#\s|[[:cntrl:]]#', '', $linkHandlerKeyword)), ['javascript', 'data'], true)) {
             // Disallow insecure scheme's like javascript: or data:
             return $linkText;
         } else {

diff --git a/typo3/sysext/frontend/Classes/Typolink/AbstractTypolinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/AbstractTypolinkBuilder.php
@@ -116,6 +116,20 @@ protected function forceAbsoluteUrl(string $url, array $configuration): string
         return $url;
     }
 
+    /**
+     * Determines whether lib.parseFunc is defined.
+     *
+     * @return bool
+     */
+    protected function isLibParseFuncDefined(): bool
+    {
+        $configuration = $this->contentObjectRenderer->mergeTSRef(
+            ['parseFunc' => '< lib.parseFunc'],
+            'parseFunc'
+        );
+        return !empty($configuration['parseFunc.']) && is_array($configuration['parseFunc.']);
+    }
+
     /**
      * Helper method to a fallback method parsing HTML out of it
      *
@@ -125,10 +139,29 @@ protected function forceAbsoluteUrl(string $url, array $configuration): string
      */
     protected function parseFallbackLinkTextIfLinkTextIsEmpty(string $originalLinkText, string $fallbackLinkText): string
     {
-        if ($originalLinkText === '') {
+        if ($originalLinkText !== '') {
+            return $originalLinkText;
+        }
+        if ($this->isLibParseFuncDefined()) {
             return $this->contentObjectRenderer->parseFunc($fallbackLinkText, ['makelinks' => 0], '< lib.parseFunc');
         }
-        return $originalLinkText;
+        // encode in case `lib.parseFunc` is not configured
+        return $this->encodeFallbackLinkTextIfLinkTextIsEmpty($originalLinkText, $fallbackLinkText);
+    }
+
+    /**
+     * Helper method to a fallback method properly encoding HTML.
+     *
+     * @param string $originalLinkText the original string, if empty, the fallback link text
+     * @param string $fallbackLinkText the string to be used.
+     * @return string the final text
+     */
+    protected function encodeFallbackLinkTextIfLinkTextIsEmpty(string $originalLinkText, string $fallbackLinkText): string
+    {
+        if ($originalLinkText !== '') {
+            return $originalLinkText;
+        }
+        return htmlspecialchars($fallbackLinkText, ENT_QUOTES);
     }
 
     /**

diff --git a/typo3/sysext/frontend/Classes/Typolink/ExternalUrlLinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/ExternalUrlLinkBuilder.php
@@ -28,7 +28,7 @@ public function build(array &$linkDetails, string $linkText, string $target, arr
     {
         return [
             $this->processUrl(UrlProcessorInterface::CONTEXT_EXTERNAL, $linkDetails['url'], $conf),
-            $this->parseFallbackLinkTextIfLinkTextIsEmpty($linkText, $linkDetails['url']),
+            $this->encodeFallbackLinkTextIfLinkTextIsEmpty($linkText, $linkDetails['url']),
             $target ?: $this->resolveTargetAttribute($conf, 'extTarget', true, $this->getTypoScriptFrontendController()->extTarget)
         ];
     }

diff --git a/typo3/sysext/frontend/Classes/Typolink/FileOrFolderLinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/FileOrFolderLinkBuilder.php
@@ -47,7 +47,7 @@ public function build(array &$linkDetails, string $linkText, string $target, arr
             $linkLocation = '';
         }
         // Setting title if blank value to link
-        $linkText = $this->parseFallbackLinkTextIfLinkTextIsEmpty($linkText, rawurldecode($linkLocation));
+        $linkText = $this->encodeFallbackLinkTextIfLinkTextIsEmpty($linkText, rawurldecode($linkLocation));
         if (strpos($linkLocation, '/') !== 0
             && parse_url($linkLocation, PHP_URL_SCHEME) === null
         ) {

diff --git a/typo3/sysext/frontend/Classes/Typolink/LegacyLinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/LegacyLinkBuilder.php
@@ -33,15 +33,15 @@ public function build(array &$linkDetails, string $linkText, string $target, arr
             $linkDetails['type'] = LinkService::TYPE_FILE;
             $linkLocation = $linkDetails['file'];
             // Setting title if blank value to link
-            $linkText = $this->parseFallbackLinkTextIfLinkTextIsEmpty($linkText, rawurldecode($linkLocation));
+            $linkText = $this->encodeFallbackLinkTextIfLinkTextIsEmpty($linkText, rawurldecode($linkLocation));
             $linkLocation = (strpos($linkLocation, '/') !== 0 ? $tsfe->absRefPrefix : '') . $linkLocation;
             $url = $this->processUrl(UrlProcessorInterface::CONTEXT_FILE, $linkLocation, $conf);
             $url = $this->forceAbsoluteUrl($url, $conf);
             $target = $target ?: $this->resolveTargetAttribute($conf, 'fileTarget', false, $tsfe->fileTarget);
         } elseif ($linkDetails['url']) {
             $linkDetails['type'] = LinkService::TYPE_URL;
             $target = $target ?: $this->resolveTargetAttribute($conf, 'extTarget', true, $tsfe->extTarget);
-            $linkText = $this->parseFallbackLinkTextIfLinkTextIsEmpty($linkText, $linkDetails['url']);
+            $linkText = $this->encodeFallbackLinkTextIfLinkTextIsEmpty($linkText, $linkDetails['url']);
             $url = $this->processUrl(UrlProcessorInterface::CONTEXT_EXTERNAL, $linkDetails['url'], $conf);
         } else {
             throw new UnableToLinkException('Unknown link detected, so ' . $linkText . ' was not linked.', 1490990031, null, $linkText);

diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/TypoLinkScenario.yaml b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/TypoLinkScenario.yaml
@@ -0,0 +1,40 @@
+__variables:
+  - &pageStandard 0
+  - &pageShortcut 4
+
+entitySettings:
+  '*':
+    nodeColumnName: 'pid'
+    columnNames: {id: 'uid', language: 'sys_language_uid'}
+    defaultValues: {pid: 0}
+  page:
+    isNode: true
+    tableName: 'pages'
+    parentColumnName: 'pid'
+    languageColumnNames: ['l10n_parent', 'l10n_source']
+    columnNames: {type: 'doktype', root: 'is_siteroot'}
+    defaultValues: {hidden: 0, doktype: *pageStandard}
+    valueInstructions:
+      shortcut:
+        first: {shortcut: 0, shortcut_mode: 1}
+  content:
+    tableName: 'tt_content'
+    languageColumnNames: ['l18n_parent', 'l10n_source']
+    columnNames: {title: 'header', type: 'CType'}
+    defaultValues: {hidden: 0, CType: 'text'}
+
+entities:
+  page:
+    - self: {id: 1000, title: 'ACME Inc', type: *pageShortcut, shortcut: 'first', root: true, slug: '/'}
+      children:
+        - self: {id: 1100, title: 'EN: Welcome', slug: '/welcome', subtitle: 'hello-and-welcome'}
+          entities:
+            content:
+              - self: {title: 'EN: Content Element #1'}
+              - self: {title: 'EN: Content Element #2'}
+        - self: {id: 1200, title: 'EN: Features', slug: '/features'}
+        - self: {id: 9911, title: '<good>', slug: '/test/good'}
+        - self: {id: 9912, title: '<good a="a/" b="thing(1)">', slug: '/test/good-a-b-spaced'}
+        - self: {id: 9913, title: '<good%20a="a/"%20b="thing(1)">', slug: '/test/good-a-b-enc-a'}
+        - self: {id: 9914, title: '<good/a="a/"/b="thing(1)">', slug: '/test/good-a-b-enc-b'}
+        - self: {id: 9921, title: '<bad>', slug: '/test/bad'}