Skip to content

Commit

Permalink
Don't allow Semicolon or CRLF injection
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
paragonie-security committed Dec 15, 2022
1 parent 613c0d2 commit 1a1a85f
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
5 changes: 5 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
"ParagonIE\\CSPBuilder\\": "src"
}
},
"autoload-dev": {
"psr-4": {
"ParagonIE\\CSPBuilderTest\\": "test"
}
},
"require": {
"php": "^7.1|^8",
"ext-json": "*",
Expand Down
30 changes: 26 additions & 4 deletions src/CSPBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand Down Expand Up @@ -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<array-key, mixed> $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' ";
Expand Down Expand Up @@ -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?
*
Expand Down
36 changes: 36 additions & 0 deletions test/InvalidInputTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
declare(strict_types=1);

namespace ParagonIE\CSPBuilderTest;

use PHPUnit\Framework\TestCase;
use ParagonIE\CSPBuilder\CSPBuilder;

class InvalidInputTest extends TestCase
{
public function testRejectSemicolon()
{
$csp = (new CSPBuilder([]))
->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"
);
}
}

0 comments on commit 1a1a85f

Please sign in to comment.