bug #36678 [WebProfiler] Do not add src-elem CSP directives if they do not exist (ndench)

nicolas-grekas · nicolas-grekas · commit cf0d086d144b · 2020-05-04T15:33:45.000+02:00
This PR was merged into the 3.4 branch. Discussion ---------- [WebProfiler] Do not add src-elem CSP directives if they do not exist | Q | A | ------------- | --- | Branch? | 3.4, 4.4, 5.0 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36643 | License | MIT | Doc PR | n/a In the latest 3.4.*, 4.4.* and 5.0.* branches the `script-src-elem` and `style-src-elem` directives are added to the Content-Security-Policy header if they don't exist by copying the `default-src`. This causes browsers to ignore the `script-src` and `style-src` directives which likely contain scripts and styles the developer wanted to allow. As mentioned in the fixed ticket, we shouldn't be adding these directives if they don't exist because the browser will automatically fallback to `script-src` and `style-src` which we have already added `unsafe-inlen` and the `nonce-*` to. This will need to be merged into 3.4, 4.4 and 5.0, but I was unsure which branch I am meant to base it off to start with. I've put it on 4.4 but can move it to another if required. Commits ------- d9c4708 [WebProfiler] Do not add src-elem CSP directives if they do not exist
diff --git a/src/Symfony/Bundle/WebProfilerBundle/Csp/ContentSecurityPolicyHandler.php b/src/Symfony/Bundle/WebProfilerBundle/Csp/ContentSecurityPolicyHandler.php
@@ -133,12 +133,11 @@ private function updateCspHeaders(Response $response, array $nonces = [])
                     continue;
                 }
                 if (!isset($headers[$header][$type])) {
-                    if (isset($headers[$header]['default-src'])) {
-                        $headers[$header][$type] = $headers[$header]['default-src'];
-                    } else {
-                        // If there is no script-src/style-src and no default-src, no additional rules required.
+                    if (null === $fallback = $this->getDirectiveFallback($directives, $type)) {
                         continue;
                     }
+
+                    $headers[$header][$type] = $fallback;
                 }
                 $ruleIsSet = true;
                 if (!\in_array('\'unsafe-inline\'', $headers[$header][$type], true)) {
@@ -218,9 +217,7 @@ private function authorizesInline(array $directivesSet, $type)
     {
         if (isset($directivesSet[$type])) {
             $directives = $directivesSet[$type];
-        } elseif (isset($directivesSet['default-src'])) {
-            $directives = $directivesSet['default-src'];
-        } else {
+        } elseif (null === $directives = $this->getDirectiveFallback($directivesSet, $type)) {
             return false;
         }
 
@@ -244,6 +241,16 @@ private function hasHashOrNonce(array $directives)
         return false;
     }
 
+    private function getDirectiveFallback(array $directiveSet, $type)
+    {
+        if (\in_array($type, ['script-src-elem', 'style-src-elem'], true) || !isset($directiveSet['default-src'])) {
+            // Let the browser fallback on it's own
+            return null;
+        }
+
+        return $directiveSet['default-src'];
+    }
+
     /**
      * Retrieves the Content-Security-Policy headers (either X-Content-Security-Policy or Content-Security-Policy) from
      * a response.
diff --git a/src/Symfony/Bundle/WebProfilerBundle/Tests/Csp/ContentSecurityPolicyHandlerTest.php b/src/Symfony/Bundle/WebProfilerBundle/Tests/Csp/ContentSecurityPolicyHandlerTest.php
@@ -131,7 +131,14 @@ public function provideRequestAndResponsesForOnKernelResponse()
                 ['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
                 $this->createRequest(),
                 $this->createResponse(['Content-Security-Policy' => 'default-src \'self\' domain.com; script-src \'self\' \'unsafe-inline\'', 'Content-Security-Policy-Report-Only' => 'default-src \'self\' domain-report-only.com; script-src \'self\' \'unsafe-inline\'']),
-                ['Content-Security-Policy' => 'default-src \'self\' domain.com; script-src \'self\' \'unsafe-inline\'; script-src-elem \'self\' domain.com \'unsafe-inline\' \'nonce-'.$nonce.'\'; style-src \'self\' domain.com \'unsafe-inline\' \'nonce-'.$nonce.'\'; style-src-elem \'self\' domain.com \'unsafe-inline\' \'nonce-'.$nonce.'\'', 'Content-Security-Policy-Report-Only' => 'default-src \'self\' domain-report-only.com; script-src \'self\' \'unsafe-inline\'; script-src-elem \'self\' domain-report-only.com \'unsafe-inline\' \'nonce-'.$nonce.'\'; style-src \'self\' domain-report-only.com \'unsafe-inline\' \'nonce-'.$nonce.'\'; style-src-elem \'self\' domain-report-only.com \'unsafe-inline\' \'nonce-'.$nonce.'\'', 'X-Content-Security-Policy' => null],
+                ['Content-Security-Policy' => 'default-src \'self\' domain.com; script-src \'self\' \'unsafe-inline\'; style-src \'self\' domain.com \'unsafe-inline\' \'nonce-'.$nonce.'\'', 'Content-Security-Policy-Report-Only' => 'default-src \'self\' domain-report-only.com; script-src \'self\' \'unsafe-inline\'; style-src \'self\' domain-report-only.com \'unsafe-inline\' \'nonce-'.$nonce.'\'', 'X-Content-Security-Policy' => null],
+            ],
+            [
+                $nonce,
+                ['csp_script_nonce' => $nonce, 'csp_style_nonce' => $nonce],
+                $this->createRequest(),
+                $this->createResponse(['Content-Security-Policy' => 'default-src \'self\' domain.com; script-src \'self\' \'unsafe-inline\'; script-src-elem \'self\'; style-src \'self\' \'unsafe-inline\'; style-src-elem \'self\'', 'Content-Security-Policy-Report-Only' => 'default-src \'self\' domain-report-only.com; script-src \'self\' \'unsafe-inline\'; script-src-elem \'self\'; style-src \'self\' \'unsafe-inline\'; style-src-elem \'self\'']),
+                ['Content-Security-Policy' => 'default-src \'self\' domain.com; script-src \'self\' \'unsafe-inline\'; script-src-elem \'self\' \'unsafe-inline\' \'nonce-'.$nonce.'\'; style-src \'self\' \'unsafe-inline\'; style-src-elem \'self\' \'unsafe-inline\' \'nonce-'.$nonce.'\'', 'Content-Security-Policy-Report-Only' => 'default-src \'self\' domain-report-only.com; script-src \'self\' \'unsafe-inline\'; script-src-elem \'self\' \'unsafe-inline\' \'nonce-'.$nonce.'\'; style-src \'self\' \'unsafe-inline\'; style-src-elem \'self\' \'unsafe-inline\' \'nonce-'.$nonce.'\'', 'X-Content-Security-Policy' => null],
             ],
             [
                 $nonce,
