fix(cadente): reject invalid request methods and targets, answer 431 on oversized headers

cadente/Sisk.Cadente/HttpConnection.cs

@@ -62,7 +62,12 @@ public async Task<HttpConnectionState> HandleConnectionEventsAsync ( Cancellatio
 
         while (!disposedValue) {
 
-            (HttpRequestBase? nextRequest, bufferedRequestLength) = await HttpRequestReader.TryReadHttpRequestAsync ( requestPool.Memory, bufferedRequestLength, networkStream, shutdownToken, headerParsingTimeout, headerReadTimeoutSource ).ConfigureAwait ( false );
+            (HttpRequestBase? nextRequest, bufferedRequestLength, bool headersTooLarge) = await HttpRequestReader.TryReadHttpRequestAsync ( requestPool.Memory, bufferedRequestLength, networkStream, shutdownToken, headerParsingTimeout, headerReadTimeoutSource ).ConfigureAwait ( false );
+
+            if (headersTooLarge) {
+                await networkStream.WriteAsync ( HttpResponseSerializer.GetRawMessage ( "Request header fields too large.", 431, "Request Header Fields Too Large" ), shutdownToken ).ConfigureAwait ( false );
+                await networkStream.FlushAsync ( shutdownToken ).ConfigureAwait ( false );
+            }
 
             if (nextRequest is null) {
                 Logger.LogInformation ( $"TryReadHttpRequestAsync returned no request" );

cadente/Sisk.Cadente/HttpHost.cs

@@ -466,7 +466,15 @@ private static async Task WritePlainHttpRedirectAsync ( Stream stream ) {
 
         using var memoryPin = MemoryPool<byte>.Shared.Rent ( HttpConnection.RESERVED_BUFFER_SIZE );
         using var shutdownToken = new CancellationTokenSource ( TimeSpan.FromSeconds ( 15 ) );
-        (HttpRequestBase? rawRequest, _) = await HttpRequestReader.TryReadHttpRequestAsync ( memoryPin.Memory, bufferedLength: 0, stream, shutdownToken.Token, headerReadTimeoutMs: 5_000 ).ConfigureAwait ( false );
+        (HttpRequestBase? rawRequest, _, bool headersTooLarge) = await HttpRequestReader.TryReadHttpRequestAsync ( memoryPin.Memory, bufferedLength: 0, stream, shutdownToken.Token, headerReadTimeoutMs: 5_000 ).ConfigureAwait ( false );
+
+        if (headersTooLarge) {
+            try {
+                await stream.WriteAsync ( HttpResponseSerializer.GetRawMessage ( "Request header fields too large.", 431, "Request Header Fields Too Large" ), shutdownToken.Token ).ConfigureAwait ( false );
+            }
+            catch { }
+            return;
+        }
 
         if (rawRequest is null) {
 

cadente/Sisk.Cadente/HttpSerializer/HttpRequestReader.cs

@@ -35,9 +35,11 @@ static class HttpRequestReader {
 
     private static ReadOnlySpan<byte> TrimChars => " \t\r\n"u8;
 
+    private static readonly SearchValues<byte> MethodTokenChars = SearchValues.Create ( "!#$%&'*+-.^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"u8 );
+
     [SkipLocalsInit]
     [MethodImpl ( MethodImplOptions.AggressiveOptimization )]
-    public static async ValueTask<(HttpRequestBase? Request, int BufferedLength)> TryReadHttpRequestAsync ( Memory<byte> sharedBuffer, int bufferedLength, Stream stream, CancellationToken cancellationToken = default, int headerReadTimeoutMs = DefaultHeaderReadTimeoutMs, CancellationTokenSource? timeoutSource = null ) {
+    public static async ValueTask<(HttpRequestBase? Request, int BufferedLength, bool HeadersTooLarge)> TryReadHttpRequestAsync ( Memory<byte> sharedBuffer, int bufferedLength, Stream stream, CancellationToken cancellationToken = default, int headerReadTimeoutMs = DefaultHeaderReadTimeoutMs, CancellationTokenSource? timeoutSource = null ) {
 
         int bufferLength = sharedBuffer.Length;
         int totalRead = bufferedLength;
@@ -50,18 +52,18 @@ static class HttpRequestReader {
                 ReadOnlySpan<byte> searchRegion = sharedBuffer.Span.Slice ( effectiveSearchStart, totalRead - effectiveSearchStart );
 
                 if (searchRegion.IndexOf ( HeaderTerminator ) >= 0) {
-                    return (ParseHttpRequest ( sharedBuffer.Slice ( 0, totalRead ) ), totalRead);
+                    return (ParseHttpRequest ( sharedBuffer.Slice ( 0, totalRead ) ), totalRead, false);
                 }
 
                 if (totalRead >= bufferLength) {
                     Logger.LogInformation ( $"failed to parse HTTP request: headers too large" );
-                    return (null, totalRead);
+                    return (null, totalRead, true);
                 }
 
                 long currentTicks = Environment.TickCount64;
                 if (currentTicks >= deadlineTicks) {
                     Logger.LogInformation ( $"failed to parse HTTP request: header read timeout" );
-                    return (null, totalRead);
+                    return (null, totalRead, false);
                 }
 
                 int remainingMs = (int) (deadlineTicks - currentTicks);
@@ -78,27 +80,27 @@ static class HttpRequestReader {
 
                 if (bytesRead == 0) {
                     Logger.LogInformation ( $"failed to parse HTTP request: connection closed" );
-                    return (null, totalRead);
+                    return (null, totalRead, false);
                 }
 
                 totalRead += bytesRead;
             }
         }
         catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) {
             Logger.LogInformation ( $"failed to parse HTTP request: header read timeout" );
-            return (null, totalRead);
+            return (null, totalRead, false);
         }
         catch (OperationCanceledException) {
             Logger.LogInformation ( $"failed to parse HTTP request: operation cancelled" );
-            return (null, totalRead);
+            return (null, totalRead, false);
         }
         catch (SocketException sex) {
             Logger.LogInformation ( $"failed to parse HTTP request: {sex.Message}" );
-            return (null, totalRead);
+            return (null, totalRead, false);
         }
         catch (Exception ex) {
             Logger.LogInformation ( $"failed to parse HTTP request (exception): {ex.Message}" );
-            return (null, totalRead);
+            return (null, totalRead, false);
         }
     }
 
@@ -146,6 +148,10 @@ private static async ValueTask<int> ReadWithTimeoutAsync ( Stream stream, Memory
         }
 
         ReadOnlyMemory<byte> method = buffer.Slice ( 0, methodEnd );
+        if (method.Span.ContainsAnyExcept ( MethodTokenChars )) {
+            Logger.LogInformation ( $"failed to parse HTTP request: request method is not a valid token" );
+            return null;
+        }
 
         int pathStart = methodEnd + 1;
         int pathEndRel = span.Slice ( pathStart ).IndexOf ( Space );
@@ -164,6 +170,10 @@ private static async ValueTask<int> ReadWithTimeoutAsync ( Stream stream, Memory
             Logger.LogInformation ( $"failed to parse HTTP request: fragment is not allowed in request target" );
             return null;
         }
+        if (path.Span.IndexOfAnyExceptInRange ( (byte) 0x21, (byte) 0x7E ) >= 0) {
+            Logger.LogInformation ( $"failed to parse HTTP request: request target contains invalid bytes" );
+            return null;
+        }
 
         int protocolStart = pathEnd + 1;
         int protocolLineEndRel = span.Slice ( protocolStart ).IndexOf ( LineFeed );

tests/Sisk.Core/Tests/CadenteSecurityTests.cs

@@ -131,6 +131,47 @@ public async Task HttpsListener_RedirectsPlainHttpRequest () {
         }
     }
 
+    [TestMethod]
+    public async Task HttpsListener_OversizedPlainHttpRequest_RespondsWithSingle431 () {
+        int port = GetFreePort ();
+        using var certificate = CertificateHelper.CreateDevelopmentCertificate ( "localhost" );
+        using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) {
+            Handler = new EmptyHandler (),
+            HttpsOptions = new HttpsOptions ( certificate )
+        };
+        host.TimeoutManager.SslHandshakeTimeout = CloseWaitTimeout;
+        host.Start ();
+
+        using var client = new TcpClient ();
+        using var ioCts = new CancellationTokenSource ( CloseWaitTimeout );
+        await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token );
+
+        await using var stream = client.GetStream ();
+        byte [] requestBytes = Encoding.ASCII.GetBytes (
+            "GET / HTTP/1.1\r\n" +
+            $"Host: localhost:{port}\r\n" +
+            $"X-Large: {new string ( 'a', 9000 )}\r\n" +
+            "\r\n" );
+        await stream.WriteAsync ( requestBytes, ioCts.Token );
+        await stream.FlushAsync ( ioCts.Token );
+
+        using var responseBuffer = new MemoryStream ();
+        byte [] buffer = new byte [ 1024 ];
+        try {
+            int read;
+            while ((read = await stream.ReadAsync ( buffer, ioCts.Token )) > 0) {
+                responseBuffer.Write ( buffer, 0, read );
+            }
+        }
+        catch (IOException) {
+        }
+
+        string response = Encoding.ASCII.GetString ( responseBuffer.ToArray () );
+
+        StringAssert.StartsWith ( response, "HTTP/1.1 431" );
+        Assert.AreEqual ( -1, response.IndexOf ( "HTTP/1.1 ", 1, StringComparison.Ordinal ), "Only one response must be written." );
+    }
+
     [TestMethod]
     public async Task FixedLengthResponseStream_WithoutWrite_FlushesHeadersBeforeKeepAliveRead () {
         int port = GetFreePort ();
@@ -233,6 +274,101 @@ public async Task TransferEncoding_RejectsUnsupportedCodingsBeforeHandler ( stri
             $"Unsupported Transfer-Encoding values must not produce a successful response. Response: {response}" );
     }
 
+    [DataTestMethod]
+    [DataRow ( "GET", "/admin\u0080x" )]
+    [DataRow ( "GET", "/admin\u001bx" )]
+    [DataRow ( "GET", "/admin\nx" )]
+    [DataRow ( "GET", "/admin\0x" )]
+    [DataRow ( "GET", "/admin\u007fx" )]
+    [DataRow ( "G\u0001T", "/" )]
+    [DataRow ( "G@T", "/" )]
+    [DataRow ( "GET\u0080", "/" )]
+    public async Task InvalidRequestLineBytes_AreRejectedBeforeHandler ( string method, string target ) {
+        int port = GetFreePort ();
+        var handler = new ContextInvocationHandler ();
+        using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) {
+            Handler = handler
+        };
+        host.Start ();
+
+        using var client = new TcpClient ();
+        using var ioCts = new CancellationTokenSource ( CloseWaitTimeout );
+        await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token );
+
+        await using var stream = client.GetStream ();
+        byte [] requestBytes = Encoding.Latin1.GetBytes (
+            $"{method} {target} HTTP/1.1\r\n" +
+            $"Host: localhost:{port}\r\n" +
+            "Connection: close\r\n" +
+            "\r\n" );
+
+        await stream.WriteAsync ( requestBytes, ioCts.Token );
+        await stream.FlushAsync ( ioCts.Token );
+
+        string response = await ReadHeadersAsync ( stream, ioCts.Token );
+
+        Assert.IsFalse ( handler.ContextCreated.Task.IsCompleted, "Invalid request lines must be rejected before user handlers run." );
+        Assert.IsFalse ( response.StartsWith ( "HTTP/1.1 2", StringComparison.Ordinal ), $"Invalid request lines must not produce a successful response. Response: {response}" );
+    }
+
+    [TestMethod]
+    public async Task OversizedHeaderBlock_RespondsWith431 () {
+        int port = GetFreePort ();
+        var handler = new ContextInvocationHandler ();
+        using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) {
+            Handler = handler
+        };
+        host.Start ();
+
+        using var client = new TcpClient ();
+        using var ioCts = new CancellationTokenSource ( CloseWaitTimeout );
+        await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token );
+
+        await using var stream = client.GetStream ();
+        byte [] requestBytes = Encoding.ASCII.GetBytes (
+            "GET / HTTP/1.1\r\n" +
+            $"Host: localhost:{port}\r\n" +
+            $"X-Large: {new string ( 'a', 9000 )}\r\n" +
+            "\r\n" );
+
+        await stream.WriteAsync ( requestBytes, ioCts.Token );
+        await stream.FlushAsync ( ioCts.Token );
+
+        string response = await ReadHeadersAsync ( stream, ioCts.Token );
+
+        Assert.IsFalse ( handler.ContextCreated.Task.IsCompleted, "Oversized header blocks must be rejected before user handlers run." );
+        StringAssert.StartsWith ( response, "HTTP/1.1 431" );
+    }
+
+    [DataTestMethod]
+    [DataRow ( "GET", "/a?b=c%20d" )]
+    [DataRow ( "get", "/" )]
+    [DataRow ( "M-SEARCH", "/" )]
+    public async Task ValidRequestLine_StillReachesHandler ( string method, string target ) {
+        int port = GetFreePort ();
+        var handler = new ContextInvocationHandler ();
+        using var host = new HttpHost ( new IPEndPoint ( IPAddress.Loopback, port ) ) {
+            Handler = handler
+        };
+        host.Start ();
+
+        using var client = new TcpClient ();
+        using var ioCts = new CancellationTokenSource ( CloseWaitTimeout );
+        await client.ConnectAsync ( IPAddress.Loopback, port, ioCts.Token );
+
+        await using var stream = client.GetStream ();
+        byte [] requestBytes = Encoding.ASCII.GetBytes (
+            $"{method} {target} HTTP/1.1\r\n" +
+            $"Host: localhost:{port}\r\n" +
+            "Connection: close\r\n" +
+            "\r\n" );
+
+        await stream.WriteAsync ( requestBytes, ioCts.Token );
+        await stream.FlushAsync ( ioCts.Token );
+
+        await handler.ContextCreated.Task.WaitAsync ( CloseWaitTimeout );
+    }
+
     [TestMethod]
     public async Task ContentEncoding_DoesNotAffectRequestFraming () {
         int port = GetFreePort ();