diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

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
-------

d9c47087c9 [WebProfiler] Do not add src-elem CSP directives if they do not exist
This commit is contained in:
Nicolas Grekas 2020-05-04 15:33:45 +02:00
parent 78a7f4682f d9c47087c9
commit cf0d086d14
2 changed files with 22 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/Symfony/Bundle/WebProfilerBundle/Csp/ContentSecurityPolicyHandler.php
View File

@ -133,12 +133,11 @@ class ContentSecurityPolicyHandler
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 @@ class ContentSecurityPolicyHandler
{
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 @@ class ContentSecurityPolicyHandler
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.

9
src/Symfony/Bundle/WebProfilerBundle/Tests/Csp/ContentSecurityPolicyHandlerTest.php
View File

@ -131,7 +131,14 @@ class ContentSecurityPolicyHandlerTest extends TestCase
['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,