Issue #8716 - Handle bad host/authority headers better

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
jetty · Oct 14, 2022 · d4a7439 · d4a7439
1 parent 1c242a6
commit d4a7439
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 6 deletions.
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
@@ -1028,6 +1028,8 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
                         break;
 
                     case HOST:
+                        if (_host)
+                            throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Host: multiple headers");
                         _host = true;
                         if (!(_field instanceof HostPortHttpField) && _valueString != null && !_valueString.isEmpty())
                         {

diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
@@ -2040,12 +2040,20 @@ public void testHostPort()
         assertEquals(8888, _port);
     }
 
-    @Test
-    public void testHostBadPort()
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "Host: whatever.com:xxxx\r\n",
+        "Host: myhost:testBadPort\r\n",
+        "Host: a b c d\r\n",
+        "Host: hosta, hostb, hostc\r\n",
+        "Host: hosta,hostb,hostc\r\n",
+        "Host: hosta\r\nHost: hostb\r\nHost: hostc\r\n"
+    })
+    public void testBadHost(String hostline)
     {
         ByteBuffer buffer = BufferUtil.toBuffer(
             "GET / HTTP/1.1\r\n" +
-                "Host: myhost:testBadPort\r\n" +
+                hostline +
                 "Connection: close\r\n" +
                 "\r\n");
 

diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
@@ -87,6 +87,8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -783,8 +785,16 @@ public void testEncodedNotParams() throws Exception
         assertThat(responses, startsWith("HTTP/1.1 200"));
     }
 
-    @Test
-    public void testInvalidHostHeader() throws Exception
+    @ParameterizedTest
+    @ValueSource(strings = {
+        "Host: whatever.com:xxxx\r\n",
+        "Host: myhost:testBadPort\r\n",
+        "Host: a b c d\r\n",
+        "Host: hosta, hostb, hostc\r\n",
+        "Host: hosta,hostb,hostc\r\n",
+        "Host: hosta\r\nHost: hostb\r\nHost: hostc\r\n"
+    })
+    public void testInvalidHostHeader(String hostline) throws Exception
     {
         // Use a contextHandler with vhosts to force call to Request.getServerName()
         ContextHandler context = new ContextHandler();
@@ -795,7 +805,7 @@ public void testInvalidHostHeader() throws Exception
 
         // Request with illegal Host header
         String request = "GET / HTTP/1.1\n" +
-            "Host: whatever.com:xxxx\n" +
+            hostline +
             "Content-Type: text/html;charset=utf8\n" +
             "Connection: close\n" +
             "\n";

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java b/jetty-util/src/main/java/org/eclipse/jetty/util/HostPort.java
@@ -81,6 +81,10 @@ else if (authority.charAt(0) == '[')
                 }
                 else
                 {
+                    if (!isValidAuthority(authority))
+                    {
+                        throw new IllegalArgumentException("Bad Authority");
+                    }
                     _host = authority;
                     _port = 0;
                 }
@@ -96,6 +100,29 @@ else if (authority.charAt(0) == '[')
         }
     }
 
+    /**
+     * Performs some safety checks on the authority.
+     *
+     * @param authority the authority to test
+     * @return true if the authority passes as valid
+     */
+    private boolean isValidAuthority(String authority)
+    {
+        if (authority == null)
+            return false;
+        for (int i = 0; i < authority.length(); i++)
+        {
+            int codepoint = authority.codePointAt(i);
+            if (codepoint == ',')
+                return false;
+            if (Character.isISOControl(codepoint))
+                return false;
+            if (Character.isWhitespace(codepoint))
+                return false;
+        }
+        return true;
+    }
+
     /**
      * Get the host.
      *