Merge pull request #32113 from nextcloud/bugfix/noid/fix-csp-merging-bools

Add CSP policy merge priority for booleans

4 files changed, 16 insertions, 3 deletions

diff --git a/lib/private/Security/CSP/ContentSecurityPolicy.php b/lib/private/Security/CSP/ContentSecurityPolicy.php
index 8a72934d4c9..8d9551c8978 100644
--- a/lib/private/Security/CSP/ContentSecurityPolicy.php
+++ b/lib/private/Security/CSP/ContentSecurityPolicy.php
@@ -246,6 +246,13 @@ class ContentSecurityPolicy extends \OCP\AppFramework\Http\ContentSecurityPolicy
 	}
 
 	/**
+	 * @return boolean
+	 */
+	public function isStrictDynamicAllowed(): bool {
+		return $this->strictDynamicAllowed;
+	}
+
+	/**
 	 * @param boolean $strictDynamicAllowed
 	 */
 	public function setStrictDynamicAllowed(bool $strictDynamicAllowed) {
diff --git a/lib/private/Security/CSP/ContentSecurityPolicyManager.php b/lib/private/Security/CSP/ContentSecurityPolicyManager.php
index ff770435eda..4930dcb759c 100644
--- a/lib/private/Security/CSP/ContentSecurityPolicyManager.php
+++ b/lib/private/Security/CSP/ContentSecurityPolicyManager.php
@@ -82,7 +82,12 @@ class ContentSecurityPolicyManager implements IContentSecurityPolicyManager {
 				$currentValues = \is_array($defaultPolicy->$getter()) ? $defaultPolicy->$getter() : [];
 				$defaultPolicy->$setter(array_values(array_unique(array_merge($currentValues, $value))));
 			} elseif (\is_bool($value)) {
-				$defaultPolicy->$setter($value);
+				$getter = 'is'.ucfirst($name);
+				$currentValue = $defaultPolicy->$getter();
+				// true wins over false
+				if ($value > $currentValue) {
+					$defaultPolicy->$setter($value);
+				}
 			}
 		}
 
diff --git a/lib/public/AppFramework/Http/ContentSecurityPolicy.php b/lib/public/AppFramework/Http/ContentSecurityPolicy.php
index 3a91e3dc2a7..0e3a6a705d5 100644
--- a/lib/public/AppFramework/Http/ContentSecurityPolicy.php
+++ b/lib/public/AppFramework/Http/ContentSecurityPolicy.php
@@ -45,7 +45,7 @@ class ContentSecurityPolicy extends EmptyContentSecurityPolicy {
 	/** @var bool Whether eval in JS scripts is allowed */
 	protected $evalScriptAllowed = false;
 	/** @var bool Whether strict-dynamic should be set */
-	protected $strictDynamicAllowed = null;
+	protected $strictDynamicAllowed = false;
 	/** @var array Domains from which scripts can get loaded */
 	protected $allowedScriptDomains = [
 		'\'self\'',
diff --git a/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php b/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php
index e5cf9ea9e1f..082ef46330d 100644
--- a/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php
+++ b/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php
@@ -87,6 +87,7 @@ class ContentSecurityPolicyManagerTest extends TestCase {
 			$policy->addAllowedFontDomain('mydomain.com');
 			$policy->addAllowedImageDomain('anotherdomain.de');
 			$policy->useStrictDynamic(true);
+			$policy->allowEvalScript(true);
 
 			$e->addPolicy($policy);
 		});
@@ -96,7 +97,7 @@ class ContentSecurityPolicyManagerTest extends TestCase {
 			$policy->addAllowedFontDomain('example.com');
 			$policy->addAllowedImageDomain('example.org');
 			$policy->allowInlineScript(true);
-			$policy->allowEvalScript(true);
+			$policy->allowEvalScript(false);
 			$e->addPolicy($policy);
 		});