From 1a1a85fcf115400d7753af842403ec6e846319de Mon Sep 17 00:00:00 2001 From: "Paragon Initiative Enterprises, LLC" Date: Wed, 14 Dec 2022 22:49:58 -0500 Subject: [PATCH] Don't allow Semicolon or CRLF injection CSP-Builder is a developer tool. It is not meant to be used with user input. However, the ability to inject CSP directives or additional headers violates the principle of least astonishment. This was reported via user demonia on HackerOne. --- composer.json | 5 +++++ src/CSPBuilder.php | 30 ++++++++++++++++++++++++++---- test/InvalidInputTest.php | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 test/InvalidInputTest.php diff --git a/composer.json b/composer.json index d6de6cd..088072b 100644 --- a/composer.json +++ b/composer.json @@ -29,6 +29,11 @@ "ParagonIE\\CSPBuilder\\": "src" } }, + "autoload-dev": { + "psr-4": { + "ParagonIE\\CSPBuilderTest\\": "test" + } + }, "require": { "php": "^7.1|^8", "ext-json": "*", diff --git a/src/CSPBuilder.php b/src/CSPBuilder.php index 01e23aa..c8b3b2d 100644 --- a/src/CSPBuilder.php +++ b/src/CSPBuilder.php @@ -137,7 +137,7 @@ public function compile(): string if (!is_string($this->policies['report-uri'])) { throw new TypeError('report-uri policy somehow not a string'); } - $compiled [] = 'report-uri ' . $this->policies['report-uri'] . '; '; + $compiled [] = 'report-uri ' . $this->enc($this->policies['report-uri']) . '; '; } if (!empty($this->policies['report-to'])) { if (!is_string($this->policies['report-to'])) { @@ -863,16 +863,16 @@ protected function compileSubgroup(string $directive, $policies = []): string if ($directive === 'plugin-types') { return ''; } elseif ($directive === 'sandbox') { - return $directive.'; '; + return $this->enc($directive) . '; '; } return $directive." 'none'; "; } /** @var array $policies */ - $ret = $directive.' '; + $ret = $this->enc($directive) . ' '; if ($directive === 'plugin-types') { // Expects MIME types, not URLs - return $ret . implode(' ', $policies['allow']).'; '; + return $ret . $this->enc(implode(' ', $policies['allow']), 'mime').'; '; } if (!empty($policies['self'])) { $ret .= "'self' "; @@ -1009,6 +1009,28 @@ protected function getHeaderKeys(bool $legacy = true): array return $return; } + /** + * @param string $piece + * @param string $type + * @return string + */ + protected function enc(string $piece, string $type = 'default'): string + { + switch ($type) { + case 'mime': + return preg_replace('#^[a-z0-9\-/]+#', '', strtolower($piece)); + case 'url': + return urlencode($piece); + default: + // Don't inject + return str_replace( + [';', "\r", "\n", ':'], + ['%3B', '%0D', '%0A', '%3A'], + $piece + ); + } + } + /** * Is this user currently connected over HTTPS? * diff --git a/test/InvalidInputTest.php b/test/InvalidInputTest.php new file mode 100644 index 0000000..0f35546 --- /dev/null +++ b/test/InvalidInputTest.php @@ -0,0 +1,36 @@ +setReportUri('https://example.com/csp_report.php; hello world') + ->compile(); + + $this->assertStringNotContainsString( + $csp, + 'report-uri https://example.com/csp_report.php; hello world', + 'Semicolon injection is possible' + ); + } + + public function testRejectCrLf() + { + $csp = (new CSPBuilder([])) + ->setReportUri("https://example.com/csp_report.php;\r\nContent-Type:text/plain") + ->compile(); + + $this->assertStringNotContainsString( + $csp, + "\r\nContent-Type:", + "CRLF Injection is possible" + ); + } +}