Skip to content

Commit 8c6d583

Browse files
dvoskoboinikovdvoskoboinikov
committed
Merge branch '2.4.3-develop' into 2.4.3-develop-2.4-develop-sync-022421
2 parents d3f202a + ba65daf commit 8c6d583

File tree

5 files changed

+89
-5
lines changed

5 files changed

+89
-5
lines changed

‎app/code/Magento/Csp/Helper/InlineUtil.php‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace Magento\Csp\Helper;
1010

1111
use Magento\Csp\Api\InlineUtilInterface;
12+
use Magento\Csp\Model\Collector\ConfigCollector;
1213
use Magento\Csp\Model\Collector\DynamicCollector;
1314
use Magento\Csp\Model\Policy\FetchPolicy;
1415
use Magento\Framework\App\ObjectManager;
@@ -39,6 +40,11 @@ class InlineUtil implements InlineUtilInterface, SecurityProcessorInterface
3940
*/
4041
private $htmlRenderer;
4142

43+
/**
44+
* @var ConfigCollector
45+
*/
46+
private $configCollector;
47+
4248
private static $tagMeta = [
4349
'script' => ['id' => 'script-src', 'remote' => ['src'], 'hash' => true],
4450
'style' => ['id' => 'style-src', 'remote' => [], 'hash' => true],
@@ -60,15 +66,18 @@ class InlineUtil implements InlineUtilInterface, SecurityProcessorInterface
6066
* @param DynamicCollector $dynamicCollector
6167
* @param bool $useUnsafeHashes Use 'unsafe-hashes' policy (not supported by CSP v2).
6268
* @param HtmlRenderer|null $htmlRenderer
69+
* @param ConfigCollector|null $configCollector
6370
*/
6471
public function __construct(
6572
DynamicCollector $dynamicCollector,
6673
bool $useUnsafeHashes = false,
67-
?HtmlRenderer $htmlRenderer = null
74+
?HtmlRenderer $htmlRenderer = null,
75+
?ConfigCollector $configCollector = null
6876
) {
6977
$this->dynamicCollector = $dynamicCollector;
7078
$this->useUnsafeHashes = $useUnsafeHashes;
7179
$this->htmlRenderer = $htmlRenderer ?? ObjectManager::getInstance()->get(HtmlRenderer::class);
80+
$this->configCollector = $configCollector ?? ObjectManager::getInstance()->get(ConfigCollector::class);
7281
}
7382

7483
/**
@@ -187,7 +196,10 @@ public function processTag(TagData $tagData): TagData
187196
new FetchPolicy($policyId, false, $remotes)
188197
);
189198
}
190-
if ($tagData->getContent() && !empty(self::$tagMeta[$tagData->getTag()]['hash'])) {
199+
if ($tagData->getContent()
200+
&& !empty(self::$tagMeta[$tagData->getTag()]['hash'])
201+
&& $this->isInlineDisabled(self::$tagMeta[$tagData->getTag()]['id'])
202+
) {
191203
$this->dynamicCollector->add(
192204
new FetchPolicy(
193205
$policyId,
@@ -233,4 +245,21 @@ public function processEventHandler(EventHandlerData $eventHandlerData): EventHa
233245

234246
return $eventHandlerData;
235247
}
248+
249+
/**
250+
* Check if inline sources are prohibited.
251+
*
252+
* @param string $policyId
253+
* @return bool
254+
*/
255+
private function isInlineDisabled(string $policyId): bool
256+
{
257+
foreach ($this->configCollector->collect() as $policy) {
258+
if ($policy->getId() === $policyId && $policy instanceof FetchPolicy) {
259+
return !$policy->isInlineAllowed();
260+
}
261+
}
262+
263+
return false;
264+
}
236265
}

‎dev/tests/integration/_files/Magento/TestModuleCspUtil/view/frontend/templates/secure.phtml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@
77
/** @var \Magento\Framework\View\Helper\SecureHtmlRenderer $secureRenderer */
88
?>
99
<?= /* @NoEscape */ $secureRenderer->renderTag('script', ['type' => 'text/javascript'], 'let var = 1; console.log("var = " + var);', false) ?>
10+
<?= /* @NoEscape */ $secureRenderer->renderTag('script', ['type' => 'text/javascript', 'src' => 'https://magento.com/scripts/some-script.js']) ?>

‎dev/tests/integration/testsuite/Magento/Csp/Helper/InlineUtilTest.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
/**
1919
* Cover CSP util use cases.
20+
*
21+
* @magentoAppArea frontend
2022
*/
2123
class InlineUtilTest extends TestCase
2224
{
@@ -70,7 +72,18 @@ protected function tearDown(): void
7072
* @param string $result Expected result.
7173
* @param PolicyInterface[] $policiesExpected
7274
* @return void
75+
*
7376
* @dataProvider getTags
77+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/policy_id script-src
78+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/none 0
79+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/self 1
80+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/inline 0
81+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/eval 0
82+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/event_handlers 1
83+
* @magentoConfigFixture default_store csp/policies/storefront/styles/policy_id style-src
84+
* @magentoConfigFixture default_store csp/policies/storefront/styles/none 0
85+
* @magentoConfigFixture default_store csp/policies/storefront/styles/self 1
86+
* @magentoConfigFixture default_store csp/policies/storefront/styles/inline 0
7487
*/
7588
public function testRenderTag(
7689
string $tagName,
@@ -305,4 +318,29 @@ public function testSecureHtmlRenderer(): void
305318
$this->assertTrue(in_array(new FetchPolicy('script-src', false, ['https://test.magento.com']), $policies));
306319
$this->assertTrue(in_array(new FetchPolicy('script-src', false, [], [], false, true), $policies));
307320
}
321+
322+
/**
323+
* Verify that hashes are not calculated when inline sources are allowed.
324+
*
325+
* @return void
326+
*
327+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/policy_id script-src
328+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/none 0
329+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/self 1
330+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/inline 1
331+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/eval 0
332+
* @magentoConfigFixture default_store csp/policies/storefront/scripts/event_handlers 1
333+
* @magentoConfigFixture default_store csp/policies/storefront/styles/policy_id style-src
334+
* @magentoConfigFixture default_store csp/policies/storefront/styles/none 0
335+
* @magentoConfigFixture default_store csp/policies/storefront/styles/self 1
336+
* @magentoConfigFixture default_store csp/policies/storefront/styles/inline 1
337+
*/
338+
public function testRenderWithInline(): void
339+
{
340+
$this->assertEquals(
341+
'<script>alert(1);</script>',
342+
$this->util->renderTag('script', [], 'alert(1);')
343+
);
344+
$this->assertEmpty($this->dynamicCollector->consumeAdded());
345+
}
308346
}

‎lib/internal/Magento/Framework/File/Uploader.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ public function __construct(
219219
Filesystem $filesystem = null
220220
) {
221221
$this->directoryList = $directoryList ?: ObjectManager::getInstance()->get(DirectoryList::class);
222+
$this->targetDirectory = $targetDirectory ?: ObjectManager::getInstance()->get(TargetDirectory::class);
222223

223224
$this->filesystem = $filesystem ?: ObjectManager::getInstance()->get(FileSystem::class);
224225
$this->_setUploadFileId($fileId);
@@ -230,7 +231,6 @@ public function __construct(
230231
}
231232
$this->fileMime = $fileMime ?: ObjectManager::getInstance()->get(Mime::class);
232233
$this->driverPool = $driverPool ?: ObjectManager::getInstance()->get(DriverPool::class);
233-
$this->targetDirectory = $targetDirectory ?: ObjectManager::getInstance()->get(TargetDirectory::class);
234234
}
235235

236236
/**
@@ -709,6 +709,7 @@ private function _setUploadFileId($fileId)
709709
* @param array $fileId
710710
* @return void
711711
* @throws \InvalidArgumentException
712+
* @throws FileSystemException
712713
*/
713714
private function validateFileId(array $fileId): void
714715
{
@@ -730,14 +731,16 @@ private function validateFileId(array $fileId): void
730731
];
731732

732733
foreach ($allowedFolders as $allowedFolder) {
733-
if (stripos($tmpName, $allowedFolder) === 0) {
734+
$dir = $this->targetDirectory->getDirectoryReadByPath($allowedFolder);
735+
if ($dir->isExist($tmpName)) {
734736
$isValid = true;
735737
break;
736738
}
737739
}
738740

739741
foreach ($disallowedFolders as $disallowedFolder) {
740-
if (stripos($tmpName, $disallowedFolder) === 0) {
742+
$dir = $this->targetDirectory->getDirectoryReadByPath($disallowedFolder);
743+
if ($dir->isExist($tmpName)) {
741744
$isValid = false;
742745
break;
743746
}

‎lib/internal/Magento/Framework/Filesystem/Directory/TargetDirectory.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use Magento\Framework\Exception\FileSystemException;
1111
use Magento\Framework\Filesystem;
12+
use Magento\Framework\Filesystem\DriverPool;
1213

1314
/**
1415
* A target directory for remote filesystems.
@@ -57,4 +58,16 @@ public function getDirectoryRead(string $directoryCode): ReadInterface
5758
{
5859
return $this->filesystem->getDirectoryRead($directoryCode, $this->driverCode);
5960
}
61+
62+
/**
63+
* Create an instance of directory with read permissions with file path.
64+
*
65+
* @param String $path
66+
* @param string $driverCode
67+
* @return ReadInterface
68+
*/
69+
public function getDirectoryReadByPath(String $path, $driverCode = DriverPool::FILE): ReadInterface
70+
{
71+
return $this->filesystem->getDirectoryReadByPath($path, $driverCode);
72+
}
6073
}

0 commit comments

Comments
 (0)