Skip to content

Commit

Permalink
[BUGFIX] Prevent double escaping for src attribute in ScriptViewHelper
Browse files Browse the repository at this point in the history
This patch prevents double escaping of the src attribute by unescaping
the value before handover to the AssetCollector->addJavaScript() function.

Resolves: #92284
Releases: master, 10.4
Change-Id: Ia9bb22e176f8a58a8bba642599546d13aa4855d2
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/66294
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
  • Loading branch information
NeoBlack authored and ervaude committed Oct 25, 2020
1 parent e693710 commit 5c50128
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 1 deletion.
54 changes: 54 additions & 0 deletions typo3/sysext/core/Tests/Unit/Page/AssetDataProvider.php
Expand Up @@ -61,6 +61,24 @@ public static function filesDataProvider(): array
'js_prio' => '',
]
],
'1 file with suspicious source' => [
[
['file1', '"><script>alert(1)</script><x "', [], []]
],
[
'file1' => [
'source' => '"><script>alert(1)</script><x "',
'attributes' => [],
'options' => [],
]
],
[
'css_no_prio' => '<link href="&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;&lt;x &quot;" rel="stylesheet" type="text/css" >',
'css_prio' => '',
'js_no_prio' => '<script src="&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;&lt;x &quot;"></script>',
'js_prio' => '',
]
],
'1 file from external source' => [
[
['file1', 'https://typo3.org/foo.ext', [], []]
Expand All @@ -79,6 +97,42 @@ public static function filesDataProvider(): array
'js_prio' => '',
]
],
'1 file from external source with one parameter' => [
[
['file1', 'https://typo3.org/foo.ext?foo=bar', [], []]
],
[
'file1' => [
'source' => 'https://typo3.org/foo.ext?foo=bar',
'attributes' => [],
'options' => [],
]
],
[
'css_no_prio' => '<link href="https://typo3.org/foo.ext?foo=bar" rel="stylesheet" type="text/css" >',
'css_prio' => '',
'js_no_prio' => '<script src="https://typo3.org/foo.ext?foo=bar"></script>',
'js_prio' => '',
]
],
'1 file from external source with two parameters' => [
[
['file1', 'https://typo3.org/foo.ext?foo=bar&bar=baz', [], []]
],
[
'file1' => [
'source' => 'https://typo3.org/foo.ext?foo=bar&bar=baz',
'attributes' => [],
'options' => [],
]
],
[
'css_no_prio' => '<link href="https://typo3.org/foo.ext?foo=bar&amp;bar=baz" rel="stylesheet" type="text/css" >',
'css_prio' => '',
'js_no_prio' => '<script src="https://typo3.org/foo.ext?foo=bar&amp;bar=baz"></script>',
'js_prio' => '',
]
],
'2 files' => [
[
['file1', 'fileadmin/foo.ext', [], []],
Expand Down
Expand Up @@ -98,7 +98,7 @@ public function render(): string
'priority' => $this->arguments['priority']
];
if ($src !== null) {
$this->assetCollector->addJavaScript($identifier, $src, $attributes, $options);
$this->assetCollector->addJavaScript($identifier, html_entity_decode($src), $attributes, $options);
} else {
$content = (string)$this->renderChildren();
if ($content !== '') {
Expand Down
@@ -0,0 +1,71 @@
<?php

declare(strict_types=1);

/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/

namespace TYPO3\CMS\Fluid\Tests\Unit\ViewHelpers\Format;

use Prophecy\Argument;
use TYPO3\CMS\Core\Page\AssetCollector;
use TYPO3\CMS\Fluid\ViewHelpers\Asset\ScriptViewHelper;
use TYPO3\TestingFramework\Fluid\Unit\ViewHelpers\ViewHelperBaseTestcase;

class ScriptViewHelperTest extends ViewHelperBaseTestcase
{
/**
* @var ScriptViewHelper
*/
protected $viewHelper;

protected function setUp(): void
{
parent::setUp();
$this->viewHelper = new ScriptViewHelper();
$this->injectDependenciesIntoViewHelper($this->viewHelper);
}

/**
* @return array
*/
public function valueDataProvider(): array
{
return [
'fileadmin reference' => ['fileadmin/JavaScript/foo.js'],
'EXT: reference' => ['EXT:core/Resources/Public/JavaScript/foo.js'],
'external reference' => ['https://typo3.com/foo.js'],
'external reference with 1 parameter' => ['https://typo3.com/foo.js?foo=bar'],
'external reference with 2 parameters' => ['https://typo3.com/foo.js?foo=bar&bar=baz'],
];
}

/**
* @param string $src
* @test
* @dataProvider valueDataProvider
*/
public function render(string $src): void
{
$assetCollector = $this->prophesize(AssetCollector::class);
$assetCollector
->addJavaScript('test', $src, Argument::any(), Argument::any())
->shouldBeCalled();
$this->viewHelper->injectAssetCollector($assetCollector->reveal());
$this->setArgumentsUnderTest($this->viewHelper, [
'identifier' => 'test',
'src' => $src,
]);
$this->viewHelper->initializeArgumentsAndRender();
}
}

0 comments on commit 5c50128

Please sign in to comment.