improve validation of HTTP methods

This commit is contained in:
Andrew Stanton-Nurse 2016-05-27 10:44:02 -07:00
parent 306084356e
commit 290e1e3f3f
3 changed files with 93 additions and 2 deletions

View File

@ -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;

View File

@ -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))

View File

@ -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);