From 58e4f98fc5f97c5380bec39c739753205b005dfe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <duesterhus@woltlab.com>
Date: Fri, 1 Jul 2022 15:32:03 +0200
Subject: [PATCH 1/2] Improve typing in
 ServerRequestFactory::getHeaderFromArray()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Tim Düsterhus <duesterhus@woltlab.com>
---
 src/ServerRequestFactory.php | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/ServerRequestFactory.php b/src/ServerRequestFactory.php
index 30eb71cb..6f8dec0a 100644
--- a/src/ServerRequestFactory.php
+++ b/src/ServerRequestFactory.php
@@ -289,9 +289,10 @@ private static function marshalHostAndPortFromHeader($host): array
     /**
      * Retrieve a header value from an array of headers using a case-insensitive lookup.
      *
+     * @template T
      * @param array<string, string|list<string>> $headers Key/value header pairs
-     * @param mixed $default Default value to return if header not found
-     * @return mixed
+     * @param T $default Default value to return if header not found
+     * @return string|T
      */
     private static function getHeaderFromArray(string $name, array $headers, $default = null)
     {

From d2f285953d24ff170ee8c2e614675dd0c993b8d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <duesterhus@woltlab.com>
Date: Fri, 1 Jul 2022 15:23:32 +0200
Subject: [PATCH 2/2] Ignore obviously malformed `host` headers when
 constructing a ServerRequest
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I opted to ignore the `host` header instead of throwing an Exception to not
introduce a remotely triggerable exception.

Signed-off-by: Tim Düsterhus <duesterhus@woltlab.com>
---
 src/ServerRequestFactory.php      | 10 ++++++++-
 test/ServerRequestFactoryTest.php | 36 +++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/ServerRequestFactory.php b/src/ServerRequestFactory.php
index 6f8dec0a..506970d9 100644
--- a/src/ServerRequestFactory.php
+++ b/src/ServerRequestFactory.php
@@ -168,7 +168,15 @@ private static function marshalHostAndPort(array $server, array $headers) : arra
 
         $host = self::getHeaderFromArray('host', $headers, false);
         if ($host !== false) {
-            return self::marshalHostAndPortFromHeader($host);
+            // Ignore obviously malformed host headers:
+            // - Whitespace is invalid within a hostname and break the URI representation within HTTP.
+            //   non-printable characters other than SPACE and TAB are already rejected by HeaderSecurity.
+            // - A comma indicates that multiple host headers have been sent which is not legal
+            //   and might be used in an attack where a load balancer sees a different host header
+            //   than Diactoros.
+            if (! \preg_match('/[\\t ,]/', $host)) {
+                return self::marshalHostAndPortFromHeader($host);
+            }
         }
 
         if (! isset($server['SERVER_NAME'])) {
diff --git a/test/ServerRequestFactoryTest.php b/test/ServerRequestFactoryTest.php
index eeda1682..e52c0aad 100644
--- a/test/ServerRequestFactoryTest.php
+++ b/test/ServerRequestFactoryTest.php
@@ -786,4 +786,40 @@ public function testHonorsHostHeaderOverServerNameWhenMarshalingUrl(): void
         $uri = $request->getUri();
         $this->assertSame('example.com', $uri->getHost());
     }
+
+    /**
+     * @psalm-return iterable<string, array{
+     *     0: string
+     * }>
+     */
+    public function invalidHostHeaders(): iterable
+    {
+        return [
+            'comma' => ['example.com,example.net'],
+            'space' => ['example com'],
+            'tab' => ["example\tcom"],
+        ];
+    }
+
+    /**
+     * @dataProvider invalidHostHeaders
+     */
+    public function testRejectsDuplicatedHostHeader(string $host): void
+    {
+        $server = [
+            'HTTP_HOST' => $host,
+        ];
+
+        $request = ServerRequestFactory::fromGlobals(
+            $server,
+            null,
+            null,
+            null,
+            null,
+            new DoNotFilter()
+        );
+
+        $uri = $request->getUri();
+        $this->assertSame('', $uri->getHost());
+    }
 }