Merge remote-tracking branch 'origin/2.4.3-develop' into MC-39523

Author: SeruyV

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

@@ -9,6 +9,7 @@
 namespace Magento\Csp\Helper;
 
 use Magento\Csp\Api\InlineUtilInterface;
+use Magento\Csp\Model\Collector\ConfigCollector;
 use Magento\Csp\Model\Collector\DynamicCollector;
 use Magento\Csp\Model\Policy\FetchPolicy;
 use Magento\Framework\App\ObjectManager;
@@ -39,6 +40,11 @@ class InlineUtil implements InlineUtilInterface, SecurityProcessorInterface
      */
     private $htmlRenderer;
 
+    /**
+     * @var ConfigCollector
+     */
+    private $configCollector;
+
     private static $tagMeta = [
         'script' => ['id' => 'script-src', 'remote' => ['src'], 'hash' => true],
         'style' => ['id' => 'style-src', 'remote' => [], 'hash' => true],
@@ -60,15 +66,18 @@ class InlineUtil implements InlineUtilInterface, SecurityProcessorInterface
      * @param DynamicCollector $dynamicCollector
      * @param bool $useUnsafeHashes Use 'unsafe-hashes' policy (not supported by CSP v2).
      * @param HtmlRenderer|null $htmlRenderer
+     * @param ConfigCollector|null $configCollector
      */
     public function __construct(
         DynamicCollector $dynamicCollector,
         bool $useUnsafeHashes = false,
-        ?HtmlRenderer $htmlRenderer = null
+        ?HtmlRenderer $htmlRenderer = null,
+        ?ConfigCollector $configCollector = null
     ) {
         $this->dynamicCollector = $dynamicCollector;
         $this->useUnsafeHashes = $useUnsafeHashes;
         $this->htmlRenderer = $htmlRenderer ?? ObjectManager::getInstance()->get(HtmlRenderer::class);
+        $this->configCollector = $configCollector ?? ObjectManager::getInstance()->get(ConfigCollector::class);
     }
 
     /**
@@ -187,7 +196,10 @@ public function processTag(TagData $tagData): TagData
                     new FetchPolicy($policyId, false, $remotes)
                 );
             }
-            if ($tagData->getContent() && !empty(self::$tagMeta[$tagData->getTag()]['hash'])) {
+            if ($tagData->getContent()
+                && !empty(self::$tagMeta[$tagData->getTag()]['hash'])
+                && $this->isInlineDisabled(self::$tagMeta[$tagData->getTag()]['id'])
+            ) {
                 $this->dynamicCollector->add(
                     new FetchPolicy(
                         $policyId,
@@ -233,4 +245,21 @@ public function processEventHandler(EventHandlerData $eventHandlerData): EventHa
 
         return $eventHandlerData;
     }
+
+    /**
+     * Check if inline sources are prohibited.
+     *
+     * @param string $policyId
+     * @return bool
+     */
+    private function isInlineDisabled(string $policyId): bool
+    {
+        foreach ($this->configCollector->collect() as $policy) {
+            if ($policy->getId() === $policyId && $policy instanceof FetchPolicy) {
+                return !$policy->isInlineAllowed();
+            }
+        }
+
+        return false;
+    }
 }

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

@@ -7,3 +7,4 @@
 /** @var \Magento\Framework\View\Helper\SecureHtmlRenderer $secureRenderer */
 ?>
 <?= /* @NoEscape */ $secureRenderer->renderTag('script', ['type' => 'text/javascript'], 'let var = 1; console.log("var = " + var);', false) ?>
+<?= /* @NoEscape */ $secureRenderer->renderTag('script', ['type' => 'text/javascript', 'src' => 'https://magento.com/scripts/some-script.js']) ?>

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

@@ -17,6 +17,8 @@
 
 /**
  * Cover CSP util use cases.
+ *
+ * @magentoAppArea frontend
  */
 class InlineUtilTest extends TestCase
 {
@@ -70,7 +72,18 @@ protected function tearDown(): void
      * @param string $result Expected result.
      * @param PolicyInterface[] $policiesExpected
      * @return void
+     *
      * @dataProvider getTags
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/policy_id script-src
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/none 0
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/self 1
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/inline 0
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/eval 0
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/event_handlers 1
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/policy_id style-src
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/none 0
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/self 1
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/inline 0
      */
     public function testRenderTag(
         string $tagName,
@@ -305,4 +318,29 @@ public function testSecureHtmlRenderer(): void
         $this->assertTrue(in_array(new FetchPolicy('script-src', false, ['https://test.magento.com']), $policies));
         $this->assertTrue(in_array(new FetchPolicy('script-src', false, [], [], false, true), $policies));
     }
+
+    /**
+     * Verify that hashes are not calculated when inline sources are allowed.
+     *
+     * @return void
+     *
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/policy_id script-src
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/none 0
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/self 1
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/inline 1
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/eval 0
+     * @magentoConfigFixture default_store csp/policies/storefront/scripts/event_handlers 1
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/policy_id style-src
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/none 0
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/self 1
+     * @magentoConfigFixture default_store csp/policies/storefront/styles/inline 1
+     */
+    public function testRenderWithInline(): void
+    {
+        $this->assertEquals(
+            '<script>alert(1);</script>',
+            $this->util->renderTag('script', [], 'alert(1);')
+        );
+        $this->assertEmpty($this->dynamicCollector->consumeAdded());
+    }
 }

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

@@ -219,6 +219,7 @@ public function __construct(
         Filesystem $filesystem = null
     ) {
         $this->directoryList = $directoryList ?: ObjectManager::getInstance()->get(DirectoryList::class);
+        $this->targetDirectory = $targetDirectory ?: ObjectManager::getInstance()->get(TargetDirectory::class);
 
         $this->filesystem = $filesystem ?: ObjectManager::getInstance()->get(FileSystem::class);
         $this->_setUploadFileId($fileId);
@@ -230,7 +231,6 @@ public function __construct(
         }
         $this->fileMime = $fileMime ?: ObjectManager::getInstance()->get(Mime::class);
         $this->driverPool = $driverPool ?: ObjectManager::getInstance()->get(DriverPool::class);
-        $this->targetDirectory = $targetDirectory ?: ObjectManager::getInstance()->get(TargetDirectory::class);
     }
 
     /**
@@ -709,6 +709,7 @@ private function _setUploadFileId($fileId)
      * @param array $fileId
      * @return void
      * @throws \InvalidArgumentException
+     * @throws FileSystemException
      */
     private function validateFileId(array $fileId): void
     {
@@ -730,14 +731,16 @@ private function validateFileId(array $fileId): void
                 ];
 
                 foreach ($allowedFolders as $allowedFolder) {
-                    if (stripos($tmpName, $allowedFolder) === 0) {
+                    $dir = $this->targetDirectory->getDirectoryReadByPath($allowedFolder);
+                    if ($dir->isExist($tmpName)) {
                         $isValid = true;
                         break;
                     }
                 }
 
                 foreach ($disallowedFolders as $disallowedFolder) {
-                    if (stripos($tmpName, $disallowedFolder) === 0) {
+                    $dir = $this->targetDirectory->getDirectoryReadByPath($disallowedFolder);
+                    if ($dir->isExist($tmpName)) {
                         $isValid = false;
                         break;
                     }

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

@@ -9,6 +9,7 @@
 
 use Magento\Framework\Exception\FileSystemException;
 use Magento\Framework\Filesystem;
+use Magento\Framework\Filesystem\DriverPool;
 
 /**
  * A target directory for remote filesystems.
@@ -57,4 +58,16 @@ public function getDirectoryRead(string $directoryCode): ReadInterface
     {
         return $this->filesystem->getDirectoryRead($directoryCode, $this->driverCode);
     }
+
+    /**
+     * Create an instance of directory with read permissions with file path.
+     *
+     * @param String $path
+     * @param string $driverCode
+     * @return ReadInterface
+     */
+    public function getDirectoryReadByPath(String $path, $driverCode = DriverPool::FILE): ReadInterface
+    {
+        return $this->filesystem->getDirectoryReadByPath($path, $driverCode);
+    }
 }