From 435c66b14bfba187f8865f2d6d80af73524918a8 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Thu, 2 Mar 2023 14:39:35 +0100
Subject: [PATCH] [BUGFIX] Adjust Content-Security-Policy reports check for SVG
files
Issue #93884 provided and adjusted CSP header for SVG inline styles:
| default-src 'self'; script-src 'none';
| style-src 'unsafe-inline'; object-src 'none';
For SVG files, having 'unsafe-inline' for style-src is fine, since
it only applies to the very same file and cannot include other local
or remote resources.
Resolves: #100041
Releases: main, 12.4, 11.5
Change-Id: I198e0a8225ef6c0e729a3ae78981581b2d2b2205
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79033
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
.../ContentSecurityPolicyHeader.php | 4 ++-
.../ServerResponse/FileDeclaration.php | 2 +-
.../ServerResponse/ServerResponseCheck.php | 4 +--
.../ContentSecurityPolicyHeaderTest.php | 25 +++++++++++++++++--
4 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeader.php b/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeader.php
index 3919ab4f8dbd..e86de5f5cf5e 100644
--- a/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeader.php
+++ b/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeader.php
@@ -48,8 +48,9 @@ class ContentSecurityPolicyHeader
return empty($this->directives);
}
- public function mitigatesCrossSiteScripting(): bool
+ public function mitigatesCrossSiteScripting(string $fileName = null): bool
{
+ $isSvg = str_ends_with($fileName ?? '', '.svg');
$defaultSrc = isset($this->directives['default-src'])
? $this->directiveMitigatesCrossSiteScripting($this->directives['default-src'])
: null;
@@ -58,6 +59,7 @@ class ContentSecurityPolicyHeader
: null;
$styleSrc = isset($this->directives['style-src'])
? $this->directiveMitigatesCrossSiteScripting($this->directives['style-src'])
+ || ($isSvg && $this->directives['style-src']->hasInstructions('unsafe-inline'))
: null;
$objectSrc = isset($this->directives['object-src'])
? $this->directiveMitigatesCrossSiteScripting($this->directives['object-src'])
diff --git a/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/FileDeclaration.php b/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/FileDeclaration.php
index 205312369685..b085f1c4a337 100644
--- a/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/FileDeclaration.php
+++ b/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/FileDeclaration.php
@@ -122,7 +122,7 @@ class FileDeclaration
{
$mismatches = [];
if ($this->handler instanceof \Closure) {
- $result = $this->handler->call($this, $response);
+ $result = $this->handler->call($this, $this, $response);
if ($result !== null) {
$mismatches[] = $result;
}
diff --git a/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ServerResponseCheck.php b/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ServerResponseCheck.php
index d252a420a582..b686e9d2459c 100644
--- a/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ServerResponseCheck.php
+++ b/typo3/sysext/install/Classes/SystemEnvironment/ServerResponse/ServerResponseCheck.php
@@ -137,7 +137,7 @@ class ServerResponseCheck implements CheckInterface
protected function initializeFileDeclarations(string $fileName): array
{
- $cspClosure = function (ResponseInterface $response): ?StatusMessage {
+ $cspClosure = function (FileDeclaration $fileDeclaration, ResponseInterface $response): ?StatusMessage {
$cspHeader = new ContentSecurityPolicyHeader(
$response->getHeaderLine('content-security-policy')
);
@@ -147,7 +147,7 @@ class ServerResponseCheck implements CheckInterface
'missing Content-Security-Policy for this location'
);
}
- if (!$cspHeader->mitigatesCrossSiteScripting()) {
+ if (!$cspHeader->mitigatesCrossSiteScripting($fileDeclaration->getFileName())) {
return new StatusMessage(
'weak Content-Security-Policy for this location "%s"',
$response->getHeaderLine('content-security-policy')
diff --git a/typo3/sysext/install/Tests/Unit/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeaderTest.php b/typo3/sysext/install/Tests/Unit/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeaderTest.php
index df9342be5853..ac437e2c2fc7 100644
--- a/typo3/sysext/install/Tests/Unit/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeaderTest.php
+++ b/typo3/sysext/install/Tests/Unit/SystemEnvironment/ServerResponse/ContentSecurityPolicyHeaderTest.php
@@ -27,48 +27,69 @@ class ContentSecurityPolicyHeaderTest extends TestCase
return [
'#1' => [
'',
+ null,
false,
],
'#2' => [
"default-src 'none'",
+ null,
true,
],
'#3' => [
"script-src 'none'",
+ null,
false,
],
'#4' => [
"style-src 'none'",
+ null,
false,
],
'#5' => [
"default-src 'none'; script-src 'none'",
+ null,
true,
],
'#6' => [
"default-src 'none'; style-src 'none'",
+ null,
true,
],
'#7' => [
"default-src 'none'; object-src 'none'",
+ null,
true,
],
'#8' => [
"default-src 'none'; script-src 'self'; style-src 'self'; object-src 'self'",
+ null,
false,
],
'#9' => [
"script-src 'none'; style-src 'none'; object-src 'none'",
+ null,
true,
],
'#10' => [
"default-src 'none'; script-src 'unsafe-eval'; style-src 'none'; object-src 'none'",
+ null,
false,
],
'#11' => [
"default-src 'none'; script-src 'unsafe-inline'; style-src 'none'; object-src 'none'",
+ null,
false,
],
+ '#12' => [
+ "default-src 'self'; script-src 'none'; style-src 'unsafe-inline'; object-src 'none'",
+ null,
+ false,
+ ],
+ '#13' => [
+ "default-src 'self'; script-src 'none'; style-src 'unsafe-inline'; object-src 'none'",
+ 'file.svg',
+ true,
+ ],
];
}
@@ -79,9 +100,9 @@ class ContentSecurityPolicyHeaderTest extends TestCase
* @test
* @dataProvider mitigatesCrossSiteScriptingDataProvider
*/
- public function mitigatesCrossSiteScripting(string $header, bool $expectation): void
+ public function mitigatesCrossSiteScripting(string $header, ?string $fileName, $expectation): void
{
$subject = new ContentSecurityPolicyHeader($header);
- self::assertSame($expectation, $subject->mitigatesCrossSiteScripting());
+ self::assertSame($expectation, $subject->mitigatesCrossSiteScripting($fileName));
}
}
--
GitLab