From 41337345954c01cba7a11ebc8e78faaf1fcfce3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Tue, 6 Feb 2024 02:19:01 +0100 Subject: [PATCH 1/2] Support multiple cookie of the same name in the Response mock --- site/app/Test/Http/Response.php | 11 +++++++---- site/tests/Application/ThemeTest.phpt | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/site/app/Test/Http/Response.php b/site/app/Test/Http/Response.php index 55c8e9810..a541130b4 100644 --- a/site/app/Test/Http/Response.php +++ b/site/app/Test/Http/Response.php @@ -17,7 +17,7 @@ class Response implements IResponse /** @var array> */ private array $allHeaders = []; - /** @var array */ + /** @var array> */ private array $cookies = []; public string $cookieDomain = ''; @@ -135,7 +135,7 @@ public function deleteHeader(string $name): self #[Override] public function setCookie(string $name, string $value, $expire, string $path = null, string $domain = null, bool $secure = null, bool $httpOnly = null, string $sameSite = null): self { - $this->cookies[$name] = new Cookie( + $this->cookies[$name][] = new Cookie( $name, $value, $expire, @@ -154,9 +154,12 @@ public function deleteCookie(string $name, string $path = null, string $domain = } - public function getCookie(string $name): ?Cookie + /** + * @return list + */ + public function getCookie(string $name): array { - return $this->cookies[$name] ?? null; + return $this->cookies[$name] ?? []; } diff --git a/site/tests/Application/ThemeTest.phpt b/site/tests/Application/ThemeTest.phpt index 3d14fbacf..585093986 100644 --- a/site/tests/Application/ThemeTest.phpt +++ b/site/tests/Application/ThemeTest.phpt @@ -29,14 +29,14 @@ class ThemeTest extends TestCase public function testSetDarkMode(): void { $this->theme->setDarkMode(); - Assert::same('dark', $this->response->getCookie('future')?->getValue()); + Assert::same('dark', $this->response->getCookie('future')[0]->getValue()); } public function testSetLightMode(): void { $this->theme->setLightMode(); - Assert::same('bright', $this->response->getCookie('future')?->getValue()); + Assert::same('bright', $this->response->getCookie('future')[0]->getValue()); } From ed464750f5d3fabc390423a95b07f3360ea0539d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Tue, 6 Feb 2024 02:34:40 +0100 Subject: [PATCH 2/2] Detect CRLF URL attempts and fight back with maybe humor URLs like `/%0DSet-Cookie:...` would throw "PHP Warning: Header may not contain more than a single header, new line detected in [...]Http/Response.php:98" because of the `IResponse::redirect()` call in `WebApplication::redirectToSecure()`. Let's detect such attempts and stop them in their tracks. --- site/app/Application/WebApplication.php | 11 +++ site/app/EasterEgg/CrLfUrlInjections.php | 45 +++++++++++ site/app/Test/Http/Response.php | 19 ++++- site/config/services.neon | 1 + site/disallowed-calls.neon | 1 + .../EasterEgg/CrLfUrlInjectionsTest.phpt | 80 +++++++++++++++++++ 6 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 site/app/EasterEgg/CrLfUrlInjections.php create mode 100644 site/tests/EasterEgg/CrLfUrlInjectionsTest.phpt diff --git a/site/app/Application/WebApplication.php b/site/app/Application/WebApplication.php index 06614e2e3..144d25542 100644 --- a/site/app/Application/WebApplication.php +++ b/site/app/Application/WebApplication.php @@ -3,6 +3,7 @@ namespace MichalSpacekCz\Application; +use MichalSpacekCz\EasterEgg\CrLfUrlInjections; use MichalSpacekCz\Http\ContentSecurityPolicy\CspValues; use MichalSpacekCz\Http\SecurityHeaders; use Nette\Application\Application; @@ -17,6 +18,7 @@ public function __construct( private IResponse $httpResponse, private SecurityHeaders $securityHeaders, private Application $application, + private CrLfUrlInjections $crLfUrlInjections, private string $fqdn, ) { } @@ -24,6 +26,7 @@ public function __construct( public function run(): void { + $this->detectCrLfUrlInjectionAttempt(); $this->redirectToSecure(); $this->application->onResponse[] = function (): void { $this->securityHeaders->sendHeaders(); @@ -42,4 +45,12 @@ private function redirectToSecure(): void } } + + private function detectCrLfUrlInjectionAttempt(): void + { + if ($this->crLfUrlInjections->detectAttempt()) { + exit(); + } + } + } diff --git a/site/app/EasterEgg/CrLfUrlInjections.php b/site/app/EasterEgg/CrLfUrlInjections.php new file mode 100644 index 000000000..6d89e04d9 --- /dev/null +++ b/site/app/EasterEgg/CrLfUrlInjections.php @@ -0,0 +1,45 @@ +httpRequest->getUrl()->getAbsoluteUrl(); + if (!str_contains($url, "\r") && !str_contains($url, "\n")) { + return false; + } + $matches = Strings::matchAll($url, sprintf('/Set\-Cookie:%s=([a-z0-9]+)/i', self::COOKIE_NAME)); + foreach ($matches as $match) { + // Don't use any cookie name from the request to avoid e.g. session fixation + $this->httpResponse->setCookie( + self::COOKIE_NAME, + $match[1], + new DateTimeImmutable('-3 years 1 month 3 days 3 hours 7 minutes'), + '/expired=3years/1month/3days/3hours/7minutes/ago', + ); + } + $this->httpResponse->setCode(IResponse::S204_NoContent, 'U WOT M8'); + $this->httpResponse->setHeader('Hack-the-Planet', 'https://youtu.be/u3CKgkyc7Qo?t=20'); + return true; + } + +} diff --git a/site/app/Test/Http/Response.php b/site/app/Test/Http/Response.php index a541130b4..1f640187f 100644 --- a/site/app/Test/Http/Response.php +++ b/site/app/Test/Http/Response.php @@ -11,6 +11,8 @@ class Response implements IResponse private int $code = IResponse::S200_OK; + private ?string $reason = null; + /** @var array */ private array $headers = []; @@ -40,9 +42,10 @@ class Response implements IResponse #[Override] - public function setCode(int $code, string $reason = null): self + public function setCode(int $code, ?string $reason = null): self { $this->code = $code; + $this->reason = $reason; return $this; } @@ -54,6 +57,12 @@ public function getCode(): int } + public function getReason(): ?string + { + return $this->reason; + } + + #[Override] public function setHeader(string $name, string $value): self { @@ -207,4 +216,12 @@ public function sent(bool $isSent): void $this->isSent = $isSent; } + + public function reset(): void + { + $this->headers = []; + $this->allHeaders = []; + $this->cookies = []; + } + } diff --git a/site/config/services.neon b/site/config/services.neon index 5987850c7..fe7819f55 100644 --- a/site/config/services.neon +++ b/site/config/services.neon @@ -29,6 +29,7 @@ services: - MichalSpacekCz\DateTime\DateTimeFactory - MichalSpacekCz\DateTime\DateTimeFormatter(@translation.translator::getDefaultLocale()) - MichalSpacekCz\DateTime\DateTimeZoneFactory + - MichalSpacekCz\EasterEgg\CrLfUrlInjections - MichalSpacekCz\EasterEgg\FourOhFourButFound - MichalSpacekCz\EasterEgg\NetteCve202015227 - MichalSpacekCz\EasterEgg\WinterIsComing diff --git a/site/disallowed-calls.neon b/site/disallowed-calls.neon index d3ce6fa1a..7cd37a286 100644 --- a/site/disallowed-calls.neon +++ b/site/disallowed-calls.neon @@ -32,6 +32,7 @@ parameters: - 'MichalSpacekCz\Http\Cookies\Cookies::getString()' - 'MichalSpacekCz\Http\Cookies\Cookies::set()' - 'MichalSpacekCz\Http\Cookies\Cookies::deleteCookie()' + - 'MichalSpacekCz\EasterEgg\CrLfUrlInjections::detectAttempt()' # Bot trolling, not for humans, the cookie is always expired - method: - 'Nette\Application\Request::getPost()' diff --git a/site/tests/EasterEgg/CrLfUrlInjectionsTest.phpt b/site/tests/EasterEgg/CrLfUrlInjectionsTest.phpt new file mode 100644 index 000000000..2a6b7e74c --- /dev/null +++ b/site/tests/EasterEgg/CrLfUrlInjectionsTest.phpt @@ -0,0 +1,80 @@ +response->reset(); + } + + + /** + * @return non-empty-list + */ + public function getUrls(): array + { + return [ + ['/foo', false, 0], + ['/foo/Set-Cookie:crlfinjection=1337', false, 0], + ['/foo%0A', true, 0], + ['/foo%0ASetCookie:crlfinjection=1337', true, 0], + ['/foo%0ASet-Cookie:crlfinjection=1337', true, 1], + ['/foo%0D', true, 0], + ['/foo%0DSetCookie:crlfinjection=1337', true, 0], + ['/foo%0DSet-Cookie:crlfinjection=1337', true, 1], + ['/foo%0D%0A', true, 0], + ['/foo%0D%0ASetCookie:crlfinjection=1337', true, 0], + ['/foo%0D%0ASet-Cookie:crlfinjection=1337', true, 1], + ['/foo%0D%0ASet-Cookie:PHPSESSID=1338', true, 0], + ]; + } + + + /** @dataProvider getUrls */ + public function testDetectAttempt(string $path, bool $attempt, int $cookies): void + { + $this->request->setUrl((new UrlScript())->withPath(urldecode($path))); + if ($attempt) { + Assert::same($attempt, $this->crLfUrlInjections->detectAttempt()); + Assert::same(IResponse::S204_NoContent, $this->response->getCode()); + Assert::same('U WOT M8', $this->response->getReason()); + Assert::count($cookies, $this->response->getCookie('crlfinjection')); + if ($cookies > 1) { + Assert::same('1337', $this->response->getCookie('crlfinjection')[0]->getValue()); + } + } else { + Assert::false($this->crLfUrlInjections->detectAttempt()); + Assert::same(IResponse::S200_OK, $this->response->getCode()); + Assert::null($this->response->getReason()); + Assert::same([], $this->response->getCookie('crlfinjection')); + } + } + +} + +TestCaseRunner::run(CrLfUrlInjectionsTest::class);