From 556a611076b12786792278a1ec6be753f625321b Mon Sep 17 00:00:00 2001
From: "John Paul E. Balandan, CPA" <paulbalandan@gmail.com>
Date: Tue, 17 Nov 2020 01:00:00 +0800
Subject: [PATCH 1/2] Update Content Security Policy

---
 app/Config/ContentSecurityPolicy.php  | 157 ++++++++++++++++++++++----
 system/HTTP/ContentSecurityPolicy.php | 118 +++++++------------
 2 files changed, 176 insertions(+), 99 deletions(-)

diff --git a/app/Config/ContentSecurityPolicy.php b/app/Config/ContentSecurityPolicy.php
index 0fc8348421f7..997d054a9d69 100644
--- a/app/Config/ContentSecurityPolicy.php
+++ b/app/Config/ContentSecurityPolicy.php
@@ -5,44 +5,155 @@
 use CodeIgniter\Config\BaseConfig;
 
 /**
- * Class ContentSecurityPolicyConfig
- *
  * Stores the default settings for the ContentSecurityPolicy, if you
  * choose to use it. The values here will be read in and set as defaults
  * for the site. If needed, they can be overridden on a page-by-page basis.
  *
  * Suggested reference for explanations:
- *    https://www.html5rocks.com/en/tutorials/security/content-security-policy/
+ *
+ * @see https://www.html5rocks.com/en/tutorials/security/content-security-policy/
  */
 class ContentSecurityPolicy extends BaseConfig
 {
-	// broadbrush CSP management
+	//-------------------------------------------------------------------------
+	// Broadbrush CSP management
+	//-------------------------------------------------------------------------
+
+	/**
+	 * Default CSP report context
+	 *
+	 * @var boolean
+	 */
+	public $reportOnly = false;
 
-	public $reportOnly              = false; // default CSP report context
-	public $reportURI               = null; // URL to send violation reports to
-	public $upgradeInsecureRequests = false; // toggle for forcing https
+	/**
+	 * Specifies a URL where a browser will send reports
+	 * when a content security policy is violated.
+	 *
+	 * @var string|null
+	 */
+	public $reportURI = null;
 
-	// sources allowed; string or array of strings
+	/**
+	 * Instructs user agents to rewrite URL schemes, changing
+	 * HTTP to HTTPS. This directive is for websites with
+	 * large numbers of old URLs that need to be rewritten.
+	 *
+	 * @var boolean
+	 */
+	public $upgradeInsecureRequests = false;
+
+	//-------------------------------------------------------------------------
+	// Sources allowed
 	// Note: once you set a policy to 'none', it cannot be further restricted
+	//-------------------------------------------------------------------------
+
+	/**
+	 * Will default to self if not overridden
+	 *
+	 * @var string|string[]|null
+	 */
+	public $defaultSrc = null;
+
+	/**
+	 * Lists allowed scripts' URLs.
+	 *
+	 * @var string|string[]
+	 */
+	public $scriptSrc = 'self';
+
+	/**
+	 * Lists allowed stylesheets' URLs.
+	 *
+	 * @var string|string[]
+	 */
+	public $styleSrc = 'self';
+
+	/**
+	 * Defines the origins from which images can be loaded.
+	 *
+	 * @var string|string[]
+	 */
+	public $imageSrc = 'self';
+
+	/**
+	 * Restricts the URLs that can appear in a page's `<base>` element.
+	 *
+	 * Will default to self if not overridden
+	 *
+	 * @var string|string[]|null
+	 */
+	public $baseURI = null;
 
-	public $defaultSrc     = null; // will default to self if not over-ridden
-	public $scriptSrc      = 'self';
-	public $styleSrc       = 'self';
-	public $imageSrc       = 'self';
-	public $baseURI        = null;    // will default to self if not over-ridden
-	public $childSrc       = 'self';
-	public $connectSrc     = 'self';
-	public $fontSrc        = null;
-	public $formAction     = 'self';
+	/**
+	 * Lists the URLs for workers and embedded frame contents
+	 *
+	 * @var string|string[]
+	 */
+	public $childSrc = 'self';
+
+	/**
+	 * Limits the origins that you can connect to (via XHR,
+	 * WebSockets, and EventSource).
+	 *
+	 * @var string|string[]
+	 */
+	public $connectSrc = 'self';
+
+	/**
+	 * Specifies the origins that can serve web fonts.
+	 *
+	 * @var string|string[]
+	 */
+	public $fontSrc = null;
+
+	/**
+	 * Lists valid endpoints for submission from `<form>` tags.
+	 *
+	 * @var string|string[]
+	 */
+	public $formAction = 'self';
+
+	/**
+	 * Specifies the sources that can embed the current page.
+	 * This directive applies to `<frame>`, `<iframe>`, `<embed>`,
+	 * and `<applet>` tags. This directive can't be used in
+	 * `<meta>` tags and applies only to non-HTML resources.
+	 *
+	 * @var string|string[]
+	 */
 	public $frameAncestors = null;
-	public $mediaSrc       = null;
-	public $objectSrc      = 'self';
-	public $manifestSrc    = null;
 
-	// mime types allowed; string or array of strings
+	/**
+	 * Restricts the origins allowed to deliver video and audio.
+	 *
+	 * @var string|string[]|null
+	 */
+	public $mediaSrc = null;
+
+	/**
+	 * Allows control over Flash and other plugins.
+	 *
+	 * @var string|string[]
+	 */
+	public $objectSrc = 'self';
+
+	/**
+	 * @var string|string[]|null
+	 */
+	public $manifestSrc = null;
+
+	/**
+	 * Limits the kinds of plugins a page may invoke.
+	 *
+	 * @var string|string[]|null
+	 */
 	public $pluginTypes = null;
 
-	// list of actions allowed; string or array of strings
+	/**
+	 * List of actions allowed.
+	 *
+	 * @var string|string[]|null
+	 */
 	public $sandbox = null;
-
 }
diff --git a/system/HTTP/ContentSecurityPolicy.php b/system/HTTP/ContentSecurityPolicy.php
index 730a3cb5f72f..8aa11fa62ee9 100644
--- a/system/HTTP/ContentSecurityPolicy.php
+++ b/system/HTTP/ContentSecurityPolicy.php
@@ -14,8 +14,6 @@
 use Config\ContentSecurityPolicy as ContentSecurityPolicyConfig;
 
 /**
- * Class ContentSecurityPolicy
- *
  * Provides tools for working with the Content-Security-Policy header
  * to help defeat XSS attacks.
  *
@@ -26,7 +24,6 @@
  */
 class ContentSecurityPolicy
 {
-
 	/**
 	 * Used for security enforcement
 	 *
@@ -188,10 +185,8 @@ class ContentSecurityPolicy
 	 */
 	protected $reportOnlyHeaders = [];
 
-	//--------------------------------------------------------------------
-
 	/**
-	 * ContentSecurityPolicy constructor.
+	 * Constructor.
 	 *
 	 * Stores our default values from the Config file.
 	 *
@@ -199,33 +194,30 @@ class ContentSecurityPolicy
 	 */
 	public function __construct(ContentSecurityPolicyConfig $config)
 	{
-		foreach ($config as $setting => $value) // @phpstan-ignore-line
+		foreach (get_object_vars($config) as $setting => $value)
 		{
-			if (isset($this->{$setting}))
+			if (property_exists($this, $setting))
 			{
 				$this->{$setting} = $value;
 			}
 		}
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Compiles and sets the appropriate headers in the request.
 	 *
 	 * Should be called just prior to sending the response to the user agent.
 	 *
 	 * @param ResponseInterface $response
+	 *
+	 * @return void
 	 */
 	public function finalize(ResponseInterface &$response)
 	{
 		$this->generateNonces($response);
-
 		$this->buildHeaders($response);
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * If TRUE, nothing will be restricted. Instead all violations will
 	 * be reported to the reportURI for monitoring. This is useful when
@@ -233,7 +225,7 @@ public function finalize(ResponseInterface &$response)
 	 * determine what errors need to be addressed before you turn on
 	 * all filtering.
 	 *
-	 * @param boolean|true $value
+	 * @param boolean $value
 	 *
 	 * @return $this
 	 */
@@ -244,8 +236,6 @@ public function reportOnly(bool $value = true)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new base_uri value. Can be either a URI class or a simple string.
 	 *
@@ -265,8 +255,6 @@ public function addBaseURI($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for a form's action. Can be either
 	 * a URI class or a simple string.
@@ -289,8 +277,6 @@ public function addChildSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for a form's action. Can be either
 	 * a URI class or a simple string.
@@ -312,8 +298,6 @@ public function addConnectSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for a form's action. Can be either
 	 * a URI class or a simple string.
@@ -335,8 +319,6 @@ public function setDefaultSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for a form's action. Can be either
 	 * a URI class or a simple string.
@@ -357,8 +339,6 @@ public function addFontSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for a form's action. Can be either
 	 * a URI class or a simple string.
@@ -377,8 +357,6 @@ public function addFormAction($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new resource that should allow embedding the resource using
 	 * <frame>, <iframe>, <object>, <embed>, or <applet>
@@ -397,8 +375,6 @@ public function addFrameAncestor($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for valid image sources. Can be either
 	 * a URI class or a simple string.
@@ -417,8 +393,6 @@ public function addImageSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for valid video and audio. Can be either
 	 * a URI class or a simple string.
@@ -437,8 +411,6 @@ public function addMediaSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for manifest sources. Can be either
 	 * a URI class or simple string.
@@ -457,8 +429,6 @@ public function addManifestSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for Flash and other plugin sources. Can be either
 	 * a URI class or a simple string.
@@ -477,8 +447,6 @@ public function addObjectSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Limits the types of plugins that can be used. Can be either
 	 * a URI class or a simple string.
@@ -497,8 +465,6 @@ public function addPluginType($mime, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Specifies a URL where a browser will send reports when a content
 	 * security policy is violated. Can be either a URI class or a simple string.
@@ -516,8 +482,6 @@ public function setReportURI(string $uri)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * specifies an HTML sandbox policy that the user agent applies to
 	 * the protected resource.
@@ -535,8 +499,6 @@ public function addSandbox($flags, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for javascript file sources. Can be either
 	 * a URI class or a simple string.
@@ -555,8 +517,6 @@ public function addScriptSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a new valid endpoint for CSS file sources. Can be either
 	 * a URI class or a simple string.
@@ -575,8 +535,6 @@ public function addStyleSrc($uri, bool $explicitReporting = null)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Sets whether the user agents should rewrite URL schemes, changing
 	 * HTTP to HTTPS.
@@ -592,9 +550,9 @@ public function upgradeInsecureRequests(bool $value = true)
 		return $this;
 	}
 
-	//--------------------------------------------------------------------
+	//-------------------------------------------------------------------------
 	// Utility
-	//--------------------------------------------------------------------
+	//-------------------------------------------------------------------------
 
 	/**
 	 * DRY method to add an string or array to a class property.
@@ -602,6 +560,8 @@ public function upgradeInsecureRequests(bool $value = true)
 	 * @param string|array $options
 	 * @param string       $target
 	 * @param boolean|null $explicitReporting
+	 *
+	 * @return void
 	 */
 	protected function addOption($options, string $target, bool $explicitReporting = null)
 	{
@@ -624,17 +584,20 @@ protected function addOption($options, string $target, bool $explicitReporting =
 		}
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Scans the body of the request message and replaces any nonce
 	 * placeholders with actual nonces, that we'll then add to our
 	 * headers.
 	 *
-	 * @param ResponseInterface|Response $response
+	 * @param ResponseInterface $response
+	 *
+	 * @return void
 	 */
 	protected function generateNonces(ResponseInterface &$response)
 	{
+		/**
+		 * @var Response $response
+		 */
 		$body = $response->getBody();
 
 		if (empty($body))
@@ -646,48 +609,49 @@ protected function generateNonces(ResponseInterface &$response)
 		{
 			$this->styleSrc = [$this->styleSrc];
 		}
+
 		if (! is_array($this->scriptSrc))
 		{
 			$this->scriptSrc = [$this->scriptSrc];
 		}
 
 		// Replace style placeholders with nonces
-		$body = preg_replace_callback(
-				'/{csp-style-nonce}/', function ($matches) {
-					$nonce = bin2hex(random_bytes(12));
+		$body = preg_replace_callback('/{csp-style-nonce}/', function ($matches) {
+			$nonce = bin2hex(random_bytes(12));
 
-					$this->styleSrc[] = 'nonce-' . $nonce;
+			$this->styleSrc[] = 'nonce-' . $nonce;
 
-					return "nonce=\"{$nonce}\"";
-				}, $body
-		);
+			return "nonce=\"{$nonce}\"";
+		}, $body);
 
 		// Replace script placeholders with nonces
-		$body = preg_replace_callback(
-				'/{csp-script-nonce}/', function ($matches) {
-					$nonce = bin2hex(random_bytes(12));
+		$body = preg_replace_callback('/{csp-script-nonce}/', function ($matches) {
+			$nonce = bin2hex(random_bytes(12));
 
-					$this->scriptSrc[] = 'nonce-' . $nonce;
+			$this->scriptSrc[] = 'nonce-' . $nonce;
 
-					return "nonce=\"{$nonce}\"";
-				}, $body
-		);
+			return "nonce=\"{$nonce}\"";
+		}, $body);
 
 		$response->setBody($body);
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Based on the current state of the elements, will add the appropriate
 	 * Content-Security-Policy and Content-Security-Policy-Report-Only headers
 	 * with their values to the response object.
 	 *
-	 * @param ResponseInterface|Response $response
+	 * @param ResponseInterface $response
+	 *
+	 * @return void
 	 */
 	protected function buildHeaders(ResponseInterface &$response)
 	{
-		// Ensure both headers are available and arrays...
+		/**
+		 * Ensure both headers are available and arrays...
+		 *
+		 * @var Response $response
+		 */
 		$response->setHeader('Content-Security-Policy', []);
 		$response->setHeader('Content-Security-Policy-Report-Only', []);
 
@@ -715,6 +679,7 @@ protected function buildHeaders(ResponseInterface &$response)
 		{
 			$this->baseURI = 'self';
 		}
+
 		if (empty($this->defaultSrc))
 		{
 			$this->defaultSrc = 'self';
@@ -722,7 +687,6 @@ protected function buildHeaders(ResponseInterface &$response)
 
 		foreach ($directives as $name => $property)
 		{
-			// base_uri
 			if (! empty($this->{$property}))
 			{
 				$this->addToHeader($name, $this->{$property});
@@ -735,10 +699,12 @@ protected function buildHeaders(ResponseInterface &$response)
 		if (! empty($this->tempHeaders))
 		{
 			$header = '';
+
 			foreach ($this->tempHeaders as $name => $value)
 			{
 				$header .= " {$name} {$value};";
 			}
+
 			// add token only if needed
 			if ($this->upgradeInsecureRequests)
 			{
@@ -751,10 +717,12 @@ protected function buildHeaders(ResponseInterface &$response)
 		if (! empty($this->reportOnlyHeaders))
 		{
 			$header = '';
+
 			foreach ($this->reportOnlyHeaders as $name => $value)
 			{
 				$header .= " {$name} {$value};";
 			}
+
 			$response->appendHeader('Content-Security-Policy-Report-Only', $header);
 		}
 
@@ -762,8 +730,6 @@ protected function buildHeaders(ResponseInterface &$response)
 		$this->reportOnlyHeaders = [];
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Adds a directive and it's options to the appropriate header. The $values
 	 * array might have options that are geared toward either the regular or the
@@ -771,6 +737,8 @@ protected function buildHeaders(ResponseInterface &$response)
 	 *
 	 * @param string            $name
 	 * @param array|string|null $values
+	 *
+	 * @return void
 	 */
 	protected function addToHeader(string $name, $values = null)
 	{
@@ -817,6 +785,4 @@ protected function addToHeader(string $name, $values = null)
 			$this->reportOnlyHeaders[$name] = implode(' ', $reportSources);
 		}
 	}
-
-	//--------------------------------------------------------------------
 }

From 791a5e9776d40e73c68b389eca80aa63e6b996a9 Mon Sep 17 00:00:00 2001
From: "John Paul E. Balandan, CPA" <paulbalandan@gmail.com>
Date: Thu, 26 Nov 2020 20:50:17 +0800
Subject: [PATCH 2/2] Implement code review suggestions

---
 app/Config/ContentSecurityPolicy.php  | 2 +-
 system/HTTP/ContentSecurityPolicy.php | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/app/Config/ContentSecurityPolicy.php b/app/Config/ContentSecurityPolicy.php
index 997d054a9d69..e357cba6a7ca 100644
--- a/app/Config/ContentSecurityPolicy.php
+++ b/app/Config/ContentSecurityPolicy.php
@@ -120,7 +120,7 @@ class ContentSecurityPolicy extends BaseConfig
 	 * and `<applet>` tags. This directive can't be used in
 	 * `<meta>` tags and applies only to non-HTML resources.
 	 *
-	 * @var string|string[]
+	 * @var string|string[]|null
 	 */
 	public $frameAncestors = null;
 
diff --git a/system/HTTP/ContentSecurityPolicy.php b/system/HTTP/ContentSecurityPolicy.php
index 8aa11fa62ee9..9ebc08d62fd8 100644
--- a/system/HTTP/ContentSecurityPolicy.php
+++ b/system/HTTP/ContentSecurityPolicy.php
@@ -595,9 +595,6 @@ protected function addOption($options, string $target, bool $explicitReporting =
 	 */
 	protected function generateNonces(ResponseInterface &$response)
 	{
-		/**
-		 * @var Response $response
-		 */
 		$body = $response->getBody();
 
 		if (empty($body))