[SECURITY] Properly escape videoId for YouTube/Vimeo 84/59084/2
authorSusanne Moog <s.moog@neusta.de>
Tue, 11 Dec 2018 09:55:04 +0000 (10:55 +0100)
committerOliver Hader <oliver.hader@typo3.org>
Tue, 11 Dec 2018 09:55:06 +0000 (10:55 +0100)
Resolves: #83184
Releases: master, 8.7, 7.6
Security-Commit: 8da8a3c1609fbd83b025c8a815d9c3b667c7722c
Security-Bulletin: TYPO3-CORE-SA-2018-006
Change-Id: Iaab42d0c00d465582cb48fe473cc345c68144031
Reviewed-on: https://review.typo3.org/59084
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/core/Classes/Resource/OnlineMedia/Helpers/VimeoHelper.php
typo3/sysext/core/Classes/Resource/OnlineMedia/Helpers/YouTubeHelper.php
typo3/sysext/core/Classes/Resource/Rendering/VimeoRenderer.php
typo3/sysext/core/Classes/Resource/Rendering/YouTubeRenderer.php
typo3/sysext/core/Tests/Unit/Resource/Rendering/VimeoRendererTest.php
typo3/sysext/core/Tests/Unit/Resource/Rendering/YouTubeRendererTest.php

index 0f0a5c7..a42b07a 100644 (file)
@@ -34,7 +34,7 @@ class VimeoHelper extends AbstractOEmbedHelper
     public function getPublicUrl(File $file, $relativeToCurrentScript = false)
     {
         $videoId = $this->getOnlineMediaId($file);
-        return sprintf('https://vimeo.com/%s', $videoId);
+        return sprintf('https://vimeo.com/%s', rawurlencode($videoId));
     }
 
     /**
@@ -92,8 +92,8 @@ class VimeoHelper extends AbstractOEmbedHelper
     {
         return sprintf(
             'https://vimeo.com/api/oembed.%s?url=%s',
-            urlencode($format),
-            urlencode(sprintf('https://vimeo.com/%s', $mediaId))
+            rawurlencode($format),
+            rawurlencode(sprintf('https://vimeo.com/%s', rawurlencode($mediaId)))
         );
     }
 }
index 961e95e..0e1a9d0 100644 (file)
@@ -19,7 +19,7 @@ use TYPO3\CMS\Core\Resource\Folder;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
- * SlideShare helper class
+ * Youtube helper class
  */
 class YouTubeHelper extends AbstractOEmbedHelper
 {
@@ -33,7 +33,7 @@ class YouTubeHelper extends AbstractOEmbedHelper
     public function getPublicUrl(File $file, $relativeToCurrentScript = false)
     {
         $videoId = $this->getOnlineMediaId($file);
-        return sprintf('https://www.youtube.com/watch?v=%s', $videoId);
+        return sprintf('https://www.youtube.com/watch?v=%s', rawurlencode($videoId));
     }
 
     /**
@@ -99,8 +99,9 @@ class YouTubeHelper extends AbstractOEmbedHelper
      */
     protected function getOEmbedUrl($mediaId, $format = 'json')
     {
-        return sprintf('https://www.youtube.com/oembed?url=%s&format=%s',
-            urlencode(sprintf('https://www.youtube.com/watch?v=%s', $mediaId)),
+        return sprintf(
+            'https://www.youtube.com/oembed?url=%s&format=%s',
+            rawurlencode(sprintf('https://www.youtube.com/watch?v=%s', rawurlencode($mediaId))),
             rawurlencode($format)
         );
     }
index 2c2b120..cf91b64 100644 (file)
@@ -115,28 +115,50 @@ class VimeoRenderer implements FileRendererInterface
         }
 
         $videoId = $this->getOnlineMediaHelper($file)->getOnlineMediaId($orgFile);
-        $src = sprintf('https://player.vimeo.com/video/%s?%s', $videoId, implode('&amp;', $urlParams));
+        $src = sprintf('https://player.vimeo.com/video/%s?%s', $videoId, implode('&', $urlParams));
 
-        $attributes = ['allowfullscreen'];
+        $attributes = [];
+        $attributes['allowfullscreen'] = true;
         if ((int)$width > 0) {
-            $attributes[] = 'width="' . (int)$width . '"';
+            $attributes['width'] = (int)$width;
         }
         if ((int)$height > 0) {
-            $attributes[] = 'height="' . (int)$height . '"';
+            $attributes['height'] = (int)$height;
         }
-        if (is_object($GLOBALS['TSFE']) && $GLOBALS['TSFE']->config['config']['doctype'] !== 'html5') {
-            $attributes[] = 'frameborder="0"';
+        if (isset($GLOBALS['TSFE']) &&
+            is_object($GLOBALS['TSFE']) &&
+            $GLOBALS['TSFE']->config['config']['doctype'] !== 'html5') {
+            $attributes['frameborder'] = 0;
         }
         foreach (['class', 'dir', 'id', 'lang', 'style', 'title', 'accesskey', 'tabindex', 'onclick'] as $key) {
             if (!empty($options[$key])) {
-                $attributes[] = $key . '="' . htmlspecialchars($options[$key]) . '"';
+                $attributes[$key] = $options[$key];
             }
         }
 
         return sprintf(
             '<iframe src="%s"%s></iframe>',
-            $src,
-            empty($attributes) ? '' : ' ' . implode(' ', $attributes)
+            htmlspecialchars($src, ENT_QUOTES | ENT_HTML5),
+            empty($attributes) ? '' : ' ' . $this->implodeAttributes($attributes)
         );
     }
+
+    /**
+     * @internal
+     * @param array $attributes
+     * @return string
+     */
+    protected function implodeAttributes(array $attributes)
+    {
+        $attributeList = [];
+        foreach ($attributes as $name => $value) {
+            $name = preg_replace('/[^\p{L}0-9_.-]/u', '', $name);
+            if ($value === true) {
+                $attributeList[] = $name;
+            } else {
+                $attributeList[] = $name . '="' . htmlspecialchars($value, ENT_QUOTES | ENT_HTML5) . '"';
+            }
+        }
+        return implode(' ', $attributeList);
+    }
 }
index ce62c93..7f0d677 100644 (file)
@@ -115,43 +115,63 @@ class YouTubeRenderer implements FileRendererInterface
             $urlParams[] = 'autoplay=1';
         }
         if (!empty($options['loop'])) {
-            $urlParams[] = 'loop=1&amp;playlist=' . $videoId;
+            $urlParams[] = 'loop=1&playlist=' . rawurlencode($videoId);
         }
         if (isset($options['relatedVideos'])) {
             $urlParams[] = 'rel=' . (int)(bool)$options['relatedVideos'];
         }
         if (!isset($options['enablejsapi']) || !empty($options['enablejsapi'])) {
-            $urlParams[] = 'enablejsapi=1&amp;origin=' . rawurlencode(GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST'));
+            $urlParams[] = 'enablejsapi=1&origin=' . rawurlencode(GeneralUtility::getIndpEnv('TYPO3_REQUEST_HOST'));
         }
         $urlParams[] = 'showinfo=' . (int)!empty($options['showinfo']);
 
         $src = sprintf(
             'https://www.youtube%s.com/embed/%s?%s',
             !isset($options['no-cookie']) || !empty($options['no-cookie']) ? '-nocookie' : '',
-            $videoId,
-            implode('&amp;', $urlParams)
+            rawurlencode($videoId),
+            implode('&', $urlParams)
         );
 
-        $attributes = ['allowfullscreen'];
+        $attributes = [];
+        $attributes['allowfullscreen'] = true;
         if ((int)$width > 0) {
-            $attributes[] = 'width="' . (int)$width . '"';
+            $attributes['width'] = (int)$width;
         }
         if ((int)$height > 0) {
-            $attributes[] = 'height="' . (int)$height . '"';
+            $attributes['height'] = (int)$height;
         }
-        if (is_object($GLOBALS['TSFE']) && $GLOBALS['TSFE']->config['config']['doctype'] !== 'html5') {
-            $attributes[] = 'frameborder="0"';
+        if (isset($GLOBALS['TSFE']) && is_object($GLOBALS['TSFE']) && $GLOBALS['TSFE']->config['config']['doctype'] !== 'html5') {
+            $attributes['frameborder'] = 0;
         }
         foreach (['class', 'dir', 'id', 'lang', 'style', 'title', 'accesskey', 'tabindex', 'onclick', 'poster', 'preload'] as $key) {
             if (!empty($options[$key])) {
-                $attributes[] = $key . '="' . htmlspecialchars($options[$key]) . '"';
+                $attributes[$key] = $options[$key];
             }
         }
 
         return sprintf(
             '<iframe src="%s"%s></iframe>',
-            $src,
-            empty($attributes) ? '' : ' ' . implode(' ', $attributes)
+            htmlspecialchars($src, ENT_QUOTES | ENT_HTML5),
+            empty($attributes) ? '' : ' ' . $this->implodeAttributes($attributes)
         );
     }
+
+    /**
+     * @internal
+     * @param array $attributes
+     * @return string
+     */
+    protected function implodeAttributes(array $attributes)
+    {
+        $attributeList = [];
+        foreach ($attributes as $name => $value) {
+            $name = preg_replace('/[^\p{L}0-9_.-]/u', '', $name);
+            if ($value === true) {
+                $attributeList[] = $name;
+            } else {
+                $attributeList[] = $name . '="' . htmlspecialchars($value, ENT_QUOTES | ENT_HTML5) . '"';
+            }
+        }
+        return implode(' ', $attributeList);
+    }
 }
index c2e91db..08de837 100644 (file)
@@ -156,4 +156,27 @@ class VimeoRendererTest extends UnitTestCase
             $this->subject->render($fileResourceMock, '300m', '200', ['autoplay' => 1])
         );
     }
+
+    /**
+     * @test
+     */
+    public function renderOutputIsEscaped()
+    {
+        /** @var VimeoHelper|\PHPUnit_Framework_MockObject_MockObject $vimeoHelper */
+        $vimeoHelper = $this->getAccessibleMock(VimeoHelper::class, ['getOnlineMediaId'], ['vimeo']);
+        $vimeoHelper->expects($this->any())->method('getOnlineMediaId')->will(
+            $this->returnValue('7331<script>danger</script>\'"random"quotes;')
+        );
+
+        $subject = $this->getAccessibleMock(VimeoRenderer::class, ['getOnlineMediaHelper'], []);
+        $subject->expects($this->any())->method('getOnlineMediaHelper')->will($this->returnValue($vimeoHelper));
+
+        /** @var File|\PHPUnit_Framework_MockObject_MockObject $fileResourceMock */
+        $fileResourceMock = $this->getMock(File::class, [], [], '', false);
+
+        $this->assertSame(
+            '<iframe src="https://player.vimeo.com/video/7331&lt;script&gt;danger&lt;/script&gt;&apos;&quot;random&quot;quotes;?title=0&amp;byline=0&amp;portrait=0" allowfullscreen width="300" height="200"></iframe>',
+            $subject->render($fileResourceMock, '300m', '200')
+        );
+    }
 }
index a3c8b22..04d395b 100644 (file)
@@ -272,4 +272,27 @@ class YouTubeRendererTest extends UnitTestCase
             $this->subject->render($fileResourceMock, '300m', '200', ['controls' => 0, 'no-cookie' => 0])
         );
     }
+
+    /**
+     * @test
+     */
+    public function renderOutputIsEscaped()
+    {
+        /** @var YouTubeHelper|\PHPUnit_Framework_MockObject_MockObject $youtubeHelper */
+        $youtubeHelper = $this->getAccessibleMock(YouTubeHelper::class, ['getOnlineMediaId'], ['youtube']);
+        $youtubeHelper->expects($this->any())->method('getOnlineMediaId')->will(
+            $this->returnValue('7331<script>danger</script>\'"random"quotes;')
+        );
+
+        $subject = $this->getAccessibleMock(YouTubeRenderer::class, ['getOnlineMediaHelper'], []);
+        $subject->expects($this->any())->method('getOnlineMediaHelper')->will($this->returnValue($youtubeHelper));
+
+        /** @var File|\PHPUnit_Framework_MockObject_MockObject $fileResourceMock */
+        $fileResourceMock = $this->getMock(File::class, [], [], '', false);
+
+        $this->assertSame(
+            '<iframe src="https://www.youtube-nocookie.com/embed/7331%3Cscript%3Edanger%3C%2Fscript%3E%27%22random%22quotes%3B?autohide=1&amp;controls=2&amp;enablejsapi=1&amp;origin=http%3A%2F%2Ftest.server.org&amp;showinfo=0" allowfullscreen width="300" height="200"></iframe>',
+            $subject->render($fileResourceMock, '300m', '200')
+        );
+    }
 }