diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs index df750a344a..c9f77bc273 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs @@ -807,10 +807,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http } method = begin.GetAsciiString(scan); + if (method == null) { RejectRequest("Missing method."); } + + // Note: We're not in the fast path any more (GetKnownMethod should have handled any HTTP Method we're aware of) + // So we can be a tiny bit slower and more careful here. + for (int i = 0; i < method.Length; i++) + { + if (!IsValidTokenChar(method[i])) + { + RejectRequest("Invalid method."); + } + } } else { @@ -938,6 +949,31 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http } } + private static bool IsValidTokenChar(char c) + { + // Determines if a character is valid as a 'token' as defined in the + // HTTP spec: https://tools.ietf.org/html/rfc7230#section-3.2.6 + return + (c >= '0' && c <= '9') || + (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || + c == '!' || + c == '#' || + c == '$' || + c == '%' || + c == '&' || + c == '\'' || + c == '*' || + c == '+' || + c == '-' || + c == '.' || + c == '^' || + c == '_' || + c == '`' || + c == '|' || + c == '~'; + } + private bool RequestUrlStartsWithPathBase(string requestUrl, out bool caseMatches) { caseMatches = true; diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs index 7b750066ac..e8baf36f10 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests // Missing method [InlineData(" ")] // Missing second space - [InlineData("/ HTTP/1.1\r\n\r\n")] + [InlineData("/ ")] // This fails trying to read the '/' because that's invalid for an HTTP method [InlineData("GET /\r\n")] // Missing target [InlineData("GET ")] @@ -67,13 +67,33 @@ namespace Microsoft.AspNetCore.Server.KestrelTests [InlineData("GET / 8charact\r")] // Missing LF [InlineData("GET / HTTP/1.0\rA")] + // Bad HTTP Methods (invalid according to RFC) + [InlineData("( / HTTP/1.0\r\n")] + [InlineData(") / HTTP/1.0\r\n")] + [InlineData("< / HTTP/1.0\r\n")] + [InlineData("> / HTTP/1.0\r\n")] + [InlineData("@ / HTTP/1.0\r\n")] + [InlineData(", / HTTP/1.0\r\n")] + [InlineData("; / HTTP/1.0\r\n")] + [InlineData(": / HTTP/1.0\r\n")] + [InlineData("\\ / HTTP/1.0\r\n")] + [InlineData("\" / HTTP/1.0\r\n")] + [InlineData("/ / HTTP/1.0\r\n")] + [InlineData("[ / HTTP/1.0\r\n")] + [InlineData("] / HTTP/1.0\r\n")] + [InlineData("? / HTTP/1.0\r\n")] + [InlineData("= / HTTP/1.0\r\n")] + [InlineData("{ / HTTP/1.0\r\n")] + [InlineData("} / HTTP/1.0\r\n")] + [InlineData("get@ / HTTP/1.0\r\n")] + [InlineData("post= / HTTP/1.0\r\n")] public async Task TestBadRequestLines(string request) { using (var server = new TestServer(context => TaskUtilities.CompletedTask)) { using (var connection = server.CreateConnection()) { - await connection.SendEnd(request); + await connection.SendAllEnd(request); await ReceiveBadRequestResponse(connection); } } @@ -84,6 +104,26 @@ namespace Microsoft.AspNetCore.Server.KestrelTests [InlineData("GET ")] [InlineData("GET / HTTP/1.2\r")] [InlineData("GET / HTTP/1.0\rA")] + // Bad HTTP Methods (invalid according to RFC) + [InlineData("( ")] + [InlineData(") ")] + [InlineData("< ")] + [InlineData("> ")] + [InlineData("@ ")] + [InlineData(", ")] + [InlineData("; ")] + [InlineData(": ")] + [InlineData("\\ ")] + [InlineData("\" ")] + [InlineData("/ ")] + [InlineData("[ ")] + [InlineData("] ")] + [InlineData("? ")] + [InlineData("= ")] + [InlineData("{ ")] + [InlineData("} ")] + [InlineData("get@ ")] + [InlineData("post= ")] public async Task ServerClosesConnectionAsSoonAsBadRequestLineIsDetected(string request) { using (var server = new TestServer(context => TaskUtilities.CompletedTask)) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/TestConnection.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/TestConnection.cs index f9974bef04..39cefb305f 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/TestConnection.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/TestConnection.cs @@ -43,6 +43,21 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _socket.Dispose(); } + public async Task SendAll(params string[] lines) + { + var text = String.Join("\r\n", lines); + var writer = new StreamWriter(_stream, Encoding.ASCII); + await writer.WriteAsync(text); + writer.Flush(); + _stream.Flush(); + } + + public async Task SendAllEnd(params string[] lines) + { + await SendAll(lines); + _socket.Shutdown(SocketShutdown.Send); + } + public async Task Send(params string[] lines) { var text = String.Join("\r\n", lines);