From 947d2aaa16254bbedb012b759e2e8a28ab51c2f3 Mon Sep 17 00:00:00 2001
From: Najdanovic Ivan <najdanovicivan@gmail.com>
Date: Wed, 22 Jan 2020 22:08:01 +0100
Subject: [PATCH] Proposal: HTTP Response - Fix CSP non existing object when
 CSP is disabled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If CSP is disabled property $CSP in HTTP/Response is not initialized.

If we try to access the CSP methods on the request object anywhere in code with CSP disabled it will crash the framework with "Call to a member function …. on null "

In order to avoid this CSP object can be initiated regardless of CSP config.

I’m aware that this is not the most efficient way to bypass the issue but some mechanism for disabling CSP should exist without having to do modifications everywhere in code.

Maybe better idea will be to create mock class to be loaded instead which will respond with catchall  magic methods like __call __set __get ….. But I don’t know if it is worth doing it as it will require adding additional class in framework.

Ref #2456

Added unit test testCSPDisabled()
---
 system/HTTP/Response.php                        |  9 +++------
 tests/system/HTTP/ContentSecurityPolicyTest.php | 17 +++++++++++++++--
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/system/HTTP/Response.php b/system/HTTP/Response.php
index 866106ae6bb7..5d535c5d0c63 100644
--- a/system/HTTP/Response.php
+++ b/system/HTTP/Response.php
@@ -239,13 +239,10 @@ public function __construct($config)
 		// Also ensures that a Cache-control header exists.
 		$this->noCache();
 
-		// Are we enforcing a Content Security Policy?
-		if ($config->CSPEnabled === true)
-		{
-			$this->CSP        = new ContentSecurityPolicy(new \Config\ContentSecurityPolicy());
-			$this->CSPEnabled = true;
-		}
+		// We need CSP object even if not enabled to avoid calls to non existing methods
+		$this->CSP = new ContentSecurityPolicy(new \Config\ContentSecurityPolicy());
 
+		$this->CSPEnabled     = $config->CSPEnabled;
 		$this->cookiePrefix   = $config->cookiePrefix;
 		$this->cookieDomain   = $config->cookieDomain;
 		$this->cookiePath     = $config->cookiePath;
diff --git a/tests/system/HTTP/ContentSecurityPolicyTest.php b/tests/system/HTTP/ContentSecurityPolicyTest.php
index 3e93b491725a..6993c5bb00cd 100644
--- a/tests/system/HTTP/ContentSecurityPolicyTest.php
+++ b/tests/system/HTTP/ContentSecurityPolicyTest.php
@@ -13,10 +13,10 @@ class ContentSecurityPolicyTest extends \CIUnitTestCase
 {
 
 	// Having this method as setUp() doesn't work - can't find Config\App !?
-	protected function prepare()
+	protected function prepare(bool $CSPEnabled = true)
 	{
 		$config             = new App();
-		$config->CSPEnabled = true;
+		$config->CSPEnabled = $CSPEnabled;
 		$this->response     = new Response($config);
 		$this->response->pretend(false);
 		$this->csp = $this->response->CSP;
@@ -490,4 +490,17 @@ public function testHeaderIgnoreCase()
 		$this->assertContains("base-uri 'self';", $result);
 	}
 
+	/**
+	 * @runInSeparateProcess
+	 * @preserveGlobalState  disabled
+	 */
+	public function testCSPDisabled()
+	{
+		$this->prepare(false);
+		$result = $this->work();
+		$this->response->CSP->addStyleSrc('https://example.com');
+
+		$this->assertHeaderNotEmitted('content-security-policy', true);
+	}
+
 }