diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index 8342295e7b..6c515f6dac 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -59,6 +59,8 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.Mvc.Api.Analyzers; + Microsoft.AspNetCore.Server.HttpSys; + Microsoft.AspNetCore.Server.IIS; diff --git a/src/Servers/HttpSys/test/FunctionalTests/RequestTests.cs b/src/Servers/HttpSys/test/FunctionalTests/RequestTests.cs index 837c53340a..f987ffe8b1 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/RequestTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/RequestTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -338,6 +338,134 @@ namespace Microsoft.AspNetCore.Server.HttpSys } } + [ConditionalFact] + public async Task Request_UrlUnescaping() + { + // Must start with '/' + var stringBuilder = new StringBuilder("/"); + for (var i = 32; i < 127; i++) + { + stringBuilder.Append("%"); + stringBuilder.Append(i.ToString("X2")); + } + var rawPath = stringBuilder.ToString(); + string root; + using (var server = Utilities.CreateHttpServerReturnRoot("/", out root, httpContext => + { + var requestInfo = httpContext.Features.Get(); + Assert.Equal(rawPath, requestInfo.RawTarget); + // '/' %2F is an exception, un-escaping it would change the structure of the path + Assert.Equal("/ !\"#$%&'()*+,-.%2F0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", requestInfo.Path); + return Task.FromResult(0); + })) + { + var response = await SendSocketRequestAsync(root, rawPath); + var responseStatusCode = response.Substring(9); // Skip "HTTP/1.1 " + Assert.Equal("200", responseStatusCode); + } + } + + [ConditionalFact] + public async Task Request_WithDoubleSlashes_LeftAlone() + { + var rawPath = "//a/b//c"; + string root; + using (var server = Utilities.CreateHttpServerReturnRoot("/", out root, httpContext => + { + var requestInfo = httpContext.Features.Get(); + Assert.Equal(rawPath, requestInfo.RawTarget); + Assert.Equal(rawPath, requestInfo.Path); + return Task.FromResult(0); + })) + { + var response = await SendSocketRequestAsync(root, rawPath); + var responseStatusCode = response.Substring(9); // Skip "HTTP/1.1 " + Assert.Equal("200", responseStatusCode); + } + } + + [ConditionalTheory] + [InlineData("/", "/a/b/../c", "", "/a/c")] + [InlineData("/", "/a/b/./c", "", "/a/b/c")] + [InlineData("/a", "/a/./c", "/a", "/c")] + [InlineData("/a", "/a/d/../b/c", "/a", "/b/c")] + [InlineData("/a/b", "/a/d/../b/c", "/a/b", "/c")] // Http.Sys uses the cooked URL when routing. + [InlineData("/a/b", "/a/./b/c", "/a/b", "/c")] // Http.Sys uses the cooked URL when routing. + public async Task Request_WithNavigation_Removed(string basePath, string input, string expectedPathBase, string expectedPath) + { + string root; + using (var server = Utilities.CreateHttpServerReturnRoot(basePath, out root, httpContext => + { + var requestInfo = httpContext.Features.Get(); + Assert.Equal(input, requestInfo.RawTarget); + Assert.Equal(expectedPathBase, requestInfo.PathBase); + Assert.Equal(expectedPath, requestInfo.Path); + return Task.FromResult(0); + })) + { + var response = await SendSocketRequestAsync(root, input); + var responseStatusCode = response.Substring(9); // Skip "HTTP/1.1 " + Assert.Equal("200", responseStatusCode); + } + } + + [ConditionalTheory] + [InlineData("/a/b/%2E%2E/c", "/a/c")] + [InlineData("/a/b/%2E/c", "/a/b/c")] + public async Task Request_WithEscapedNavigation_Removed(string input, string expected) + { + string root; + using (var server = Utilities.CreateHttpServerReturnRoot("/", out root, httpContext => + { + var requestInfo = httpContext.Features.Get(); + Assert.Equal(input, requestInfo.RawTarget); + Assert.Equal(expected, requestInfo.Path); + return Task.FromResult(0); + })) + { + var response = await SendSocketRequestAsync(root, input); + var responseStatusCode = response.Substring(9); // Skip "HTTP/1.1 " + Assert.Equal("200", responseStatusCode); + } + } + + [ConditionalFact] + public async Task Request_ControlCharacters_400() + { + string root; + using (var server = Utilities.CreateHttpServerReturnRoot("/", out root, httpContext => + { + throw new NotImplementedException(); + })) + { + for (var i = 0; i < 32; i++) + { + if (i == 9 || i == 10) continue; // \t and \r are allowed by Http.Sys. + var response = await SendSocketRequestAsync(root, "/" + (char)i); + var responseStatusCode = response.Substring(9); // Skip "HTTP/1.1 " + Assert.True(string.Equals("400", responseStatusCode), i.ToString("X2")); + } + } + } + + [ConditionalFact] + public async Task Request_EscapedControlCharacters_400() + { + string root; + using (var server = Utilities.CreateHttpServerReturnRoot("/", out root, httpContext => + { + throw new NotImplementedException(); + })) + { + for (var i = 0; i < 32; i++) + { + var response = await SendSocketRequestAsync(root, "/%" + i.ToString("X2")); + var responseStatusCode = response.Substring(9); // Skip "HTTP/1.1 " + Assert.True(string.Equals("400", responseStatusCode), i.ToString("X2")); + } + } + } + private IServer CreateServer(out string root, RequestDelegate app) { // TODO: We're just doing this to get a dynamic port. This can be removed later when we add support for hot-adding prefixes. diff --git a/src/Servers/IIS/IIS/test/Common.FunctionalTests/Inprocess/RequestTests.cs b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Inprocess/RequestTests.cs new file mode 100644 index 0000000000..ab199580e0 --- /dev/null +++ b/src/Servers/IIS/IIS/test/Common.FunctionalTests/Inprocess/RequestTests.cs @@ -0,0 +1,115 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Linq; +using System.Net.Sockets; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Testing.xunit; +using Xunit; + +namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests.InProcess +{ + [Collection(IISTestSiteCollection.Name)] + public class RequestInProcessTests + { + private readonly IISTestSiteFixture _fixture; + + public RequestInProcessTests(IISTestSiteFixture fixture) + { + _fixture = fixture; + } + + [ConditionalFact] + public async Task RequestPath_UrlUnescaping() + { + // Must start with '/' + var stringBuilder = new StringBuilder("/RequestPath/"); + for (var i = 32; i < 127; i++) + { + if (i == 43) continue; // %2B "+" gives a 404.11 (URL_DOUBLE_ESCAPED) + stringBuilder.Append("%"); + stringBuilder.Append(i.ToString("X2")); + } + var rawPath = stringBuilder.ToString(); + var response = await SendSocketRequestAsync(rawPath); + Assert.Equal(200, response.Status); + // '/' %2F is an exception, un-escaping it would change the structure of the path + Assert.Equal("/ !\"#$%&'()*,-.%2F0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", response.Body); + } + + [ConditionalFact] + public async Task Request_WithDoubleSlashes_LeftAlone() + { + var rawPath = "/RequestPath//a/b//c"; + var response = await SendSocketRequestAsync(rawPath); + Assert.Equal(200, response.Status); + Assert.Equal("//a/b//c", response.Body); + } + + [ConditionalTheory] + [InlineData("/RequestPath/a/b/../c", "/a/c")] + [InlineData("/RequestPath/a/b/./c", "/a/b/c")] + public async Task Request_WithNavigation_Removed(string input, string expectedPath) + { + var response = await SendSocketRequestAsync(input); + Assert.Equal(200, response.Status); + Assert.Equal(expectedPath, response.Body); + } + + [ConditionalTheory] + [InlineData("/RequestPath/a/b/%2E%2E/c", "/a/c")] + [InlineData("/RequestPath/a/b/%2E/c", "/a/b/c")] + public async Task Request_WithEscapedNavigation_Removed(string input, string expectedPath) + { + var response = await SendSocketRequestAsync(input); + Assert.Equal(200, response.Status); + Assert.Equal(expectedPath, response.Body); + } + + [ConditionalFact] + public async Task Request_ControlCharacters_400() + { + for (var i = 0; i < 32; i++) + { + if (i == 9 || i == 10) continue; // \t and \r are allowed by Http.Sys. + var response = await SendSocketRequestAsync("/" + (char)i); + Assert.True(string.Equals(400, response.Status), i.ToString("X2") + ";" + response); + } + } + + [ConditionalFact] + public async Task Request_EscapedControlCharacters_400() + { + for (var i = 0; i < 32; i++) + { + var response = await SendSocketRequestAsync("/%" + i.ToString("X2")); + Assert.True(string.Equals(400, response.Status), i.ToString("X2") + ";" + response); + } + } + + private async Task<(int Status, string Body)> SendSocketRequestAsync(string path) + { + using (var connection = _fixture.CreateTestConnection()) + { + await connection.Send( + "GET " + path + " HTTP/1.1", + "Host: " + _fixture.Client.BaseAddress.Authority, + "", + ""); + var headers = await connection.ReceiveHeaders(); + var status = int.Parse(headers[0].Substring(9, 3)); + if (headers.Contains("Transfer-Encoding: chunked")) + { + var bytes0 = await connection.ReceiveChunk(); + Assert.False(bytes0.IsEmpty); + return (status, Encoding.UTF8.GetString(bytes0.Span)); + } + var length = int.Parse(headers.Single(h => h.StartsWith("Content-Length: ")).Substring("Content-Length: ".Length)); + var bytes1 = await connection.Receive(length); + return (status, Encoding.ASCII.GetString(bytes1.Span)); + } + } + } +} diff --git a/src/Servers/IIS/IIS/test/testassets/IIS.Common.TestLib/TestConnections.cs b/src/Servers/IIS/IIS/test/testassets/IIS.Common.TestLib/TestConnections.cs index 3b7a870cf3..d54e06cb4d 100644 --- a/src/Servers/IIS/IIS/test/testassets/IIS.Common.TestLib/TestConnections.cs +++ b/src/Servers/IIS/IIS/test/testassets/IIS.Common.TestLib/TestConnections.cs @@ -119,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting Assert.Equal(expected, Encoding.ASCII.GetString(actual.Span)); } - private async Task> Receive(int length) + public async Task> Receive(int length) { var actual = new byte[length]; int offset = 0; diff --git a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs index 9b2011346e..35559ddb70 100644 --- a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs +++ b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs @@ -808,6 +808,12 @@ namespace TestSite await ctx.Response.WriteAsync(AppDomain.CurrentDomain.BaseDirectory); } + private Task RequestPath(HttpContext ctx) + { + ctx.Request.Headers.ContentLength = ctx.Request.Path.Value.Length; + return ctx.Response.WriteAsync(ctx.Request.Path.Value); + } + private async Task Shutdown(HttpContext ctx) { await ctx.Response.WriteAsync("Shutting down"); diff --git a/src/Shared/HttpSys/RequestProcessing/PathNormalizer.cs b/src/Shared/HttpSys/RequestProcessing/PathNormalizer.cs new file mode 100644 index 0000000000..ca9545dc6c --- /dev/null +++ b/src/Shared/HttpSys/RequestProcessing/PathNormalizer.cs @@ -0,0 +1,207 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Diagnostics; + +namespace Microsoft.AspNetCore.HttpSys.Internal +{ + internal static class PathNormalizer + { + private const byte ByteSlash = (byte)'/'; + private const byte ByteDot = (byte)'.'; + + // In-place implementation of the algorithm from https://tools.ietf.org/html/rfc3986#section-5.2.4 + public static unsafe int RemoveDotSegments(Span input) + { + fixed (byte* start = input) + { + var end = start + input.Length; + return RemoveDotSegments(start, end); + } + } + + public static unsafe int RemoveDotSegments(byte* start, byte* end) + { + if (!ContainsDotSegments(start, end)) + { + return (int)(end - start); + } + + var src = start; + var dst = start; + + while (src < end) + { + var ch1 = *src; + Debug.Assert(ch1 == '/', "Path segment must always start with a '/'"); + + byte ch2, ch3, ch4; + + switch (end - src) + { + case 1: + break; + case 2: + ch2 = *(src + 1); + + if (ch2 == ByteDot) + { + // B. if the input buffer begins with a prefix of "/./" or "/.", + // where "." is a complete path segment, then replace that + // prefix with "/" in the input buffer; otherwise, + src += 1; + *src = ByteSlash; + continue; + } + + break; + case 3: + ch2 = *(src + 1); + ch3 = *(src + 2); + + if (ch2 == ByteDot && ch3 == ByteDot) + { + // C. if the input buffer begins with a prefix of "/../" or "/..", + // where ".." is a complete path segment, then replace that + // prefix with "/" in the input buffer and remove the last + // segment and its preceding "/" (if any) from the output + // buffer; otherwise, + src += 2; + *src = ByteSlash; + + if (dst > start) + { + do + { + dst--; + } while (dst > start && *dst != ByteSlash); + } + + continue; + } + else if (ch2 == ByteDot && ch3 == ByteSlash) + { + // B. if the input buffer begins with a prefix of "/./" or "/.", + // where "." is a complete path segment, then replace that + // prefix with "/" in the input buffer; otherwise, + src += 2; + continue; + } + + break; + default: + ch2 = *(src + 1); + ch3 = *(src + 2); + ch4 = *(src + 3); + + if (ch2 == ByteDot && ch3 == ByteDot && ch4 == ByteSlash) + { + // C. if the input buffer begins with a prefix of "/../" or "/..", + // where ".." is a complete path segment, then replace that + // prefix with "/" in the input buffer and remove the last + // segment and its preceding "/" (if any) from the output + // buffer; otherwise, + src += 3; + + if (dst > start) + { + do + { + dst--; + } while (dst > start && *dst != ByteSlash); + } + + continue; + } + else if (ch2 == ByteDot && ch3 == ByteSlash) + { + // B. if the input buffer begins with a prefix of "/./" or "/.", + // where "." is a complete path segment, then replace that + // prefix with "/" in the input buffer; otherwise, + src += 2; + continue; + } + + break; + } + + // E. move the first path segment in the input buffer to the end of + // the output buffer, including the initial "/" character (if + // any) and any subsequent characters up to, but not including, + // the next "/" character or the end of the input buffer. + do + { + *dst++ = ch1; + ch1 = *++src; + } while (src < end && ch1 != ByteSlash); + } + + if (dst == start) + { + *dst++ = ByteSlash; + } + + return (int)(dst - start); + } + + public static unsafe bool ContainsDotSegments(byte* start, byte* end) + { + var src = start; + var dst = start; + + while (src < end) + { + var ch1 = *src; + Debug.Assert(ch1 == '/', "Path segment must always start with a '/'"); + + byte ch2, ch3, ch4; + + switch (end - src) + { + case 1: + break; + case 2: + ch2 = *(src + 1); + + if (ch2 == ByteDot) + { + return true; + } + + break; + case 3: + ch2 = *(src + 1); + ch3 = *(src + 2); + + if ((ch2 == ByteDot && ch3 == ByteDot) || + (ch2 == ByteDot && ch3 == ByteSlash)) + { + return true; + } + + break; + default: + ch2 = *(src + 1); + ch3 = *(src + 2); + ch4 = *(src + 3); + + if ((ch2 == ByteDot && ch3 == ByteDot && ch4 == ByteSlash) || + (ch2 == ByteDot && ch3 == ByteSlash)) + { + return true; + } + + break; + } + + do + { + ch1 = *++src; + } while (src < end && ch1 != ByteSlash); + } + + return false; + } + } +} diff --git a/src/Shared/HttpSys/RequestProcessing/RequestUriBuilder.cs b/src/Shared/HttpSys/RequestProcessing/RequestUriBuilder.cs index 81e7c437a6..2f058e8ab0 100644 --- a/src/Shared/HttpSys/RequestProcessing/RequestUriBuilder.cs +++ b/src/Shared/HttpSys/RequestProcessing/RequestUriBuilder.cs @@ -30,7 +30,9 @@ namespace Microsoft.AspNetCore.HttpSys.Internal var unescapedPath = Unescape(rawPath); - return UTF8.GetString(unescapedPath); + var length = PathNormalizer.RemoveDotSegments(unescapedPath); + + return UTF8.GetString(unescapedPath.Slice(0, length)); } /// @@ -38,7 +40,7 @@ namespace Microsoft.AspNetCore.HttpSys.Internal /// /// The raw path string to be unescaped /// The unescaped path string - private static ReadOnlySpan Unescape(Span rawPath) + private static Span Unescape(Span rawPath) { // the slot to read the input var reader = 0; @@ -63,10 +65,10 @@ namespace Microsoft.AspNetCore.HttpSys.Internal // If decoding process succeeds, the writer iterator will be moved // to the next write-ready location. On the other hand if the scanned // percent-encodings cannot be interpreted as sequence of UTF-8 octets, - // these bytes should be copied to output as is. - // The decodeReader iterator is always moved to the first byte not yet + // these bytes should be copied to output as is. + // The decodeReader iterator is always moved to the first byte not yet // be scanned after the process. A failed decoding means the chars - // between the reader and decodeReader can be copied to output untouched. + // between the reader and decodeReader can be copied to output untouched. if (!DecodeCore(ref decodeReader, ref writer, end, rawPath)) { Copy(reader, decodeReader, ref writer, rawPath); @@ -236,18 +238,18 @@ namespace Microsoft.AspNetCore.HttpSys.Internal /// /// Read the percent-encoding and try unescape it. - /// - /// The operation first peek at the character the - /// iterator points at. If it is % the is then - /// moved on to scan the following to characters. If the two following - /// characters are hexadecimal literals they will be unescaped and the + /// + /// The operation first peek at the character the + /// iterator points at. If it is % the is then + /// moved on to scan the following to characters. If the two following + /// characters are hexadecimal literals they will be unescaped and the /// value will be returned. - /// - /// If the first character is not % the iterator + /// + /// If the first character is not % the iterator /// will be removed beyond the location of % and -1 will be returned. - /// - /// If the following two characters can't be successfully unescaped the - /// iterator will be move behind the % and -1 + /// + /// If the following two characters can't be successfully unescaped the + /// iterator will be move behind the % and -1 /// will be returned. /// /// The value to read @@ -286,7 +288,7 @@ namespace Microsoft.AspNetCore.HttpSys.Internal /// /// Read the next char and convert it into hexadecimal value. - /// + /// /// The iterator will be moved to the next /// byte no matter no matter whether the operation successes. /// diff --git a/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj b/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj index 9d3b2bfa57..620aff13d7 100644 --- a/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj +++ b/src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj @@ -1,13 +1,15 @@ - + - netcoreapp3.0;net461 + netcoreapp3.0 portable + true + @@ -19,8 +21,11 @@ + + + diff --git a/src/Shared/test/Shared.Tests/PathNormalizerTests.cs b/src/Shared/test/Shared.Tests/PathNormalizerTests.cs new file mode 100644 index 0000000000..7d9425a864 --- /dev/null +++ b/src/Shared/test/Shared.Tests/PathNormalizerTests.cs @@ -0,0 +1,64 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Text; +using Xunit; + +namespace Microsoft.AspNetCore.HttpSys.Internal +{ + public class PathNormalizerTests + { + [Theory] + [InlineData("/a", "/a")] + [InlineData("/a/", "/a/")] + [InlineData("/a/b", "/a/b")] + [InlineData("/a/b/", "/a/b/")] + [InlineData("/./a", "/a")] + [InlineData("/././a", "/a")] + [InlineData("/../a", "/a")] + [InlineData("/../../a", "/a")] + [InlineData("/a/./b", "/a/b")] + [InlineData("/a/../b", "/b")] + [InlineData("/a/./", "/a/")] + [InlineData("/a/.", "/a/")] + [InlineData("/a/../", "/")] + [InlineData("/a/..", "/")] + [InlineData("/a/../b/../", "/")] + [InlineData("/a/../b/..", "/")] + [InlineData("/a/../../b", "/b")] + [InlineData("/a/../../b/", "/b/")] + [InlineData("/a/.././../b", "/b")] + [InlineData("/a/.././../b/", "/b/")] + [InlineData("/a/b/c/./../../d", "/a/d")] + [InlineData("/./a/b/c/./../../d", "/a/d")] + [InlineData("/../a/b/c/./../../d", "/a/d")] + [InlineData("/./../a/b/c/./../../d", "/a/d")] + [InlineData("/.././a/b/c/./../../d", "/a/d")] + [InlineData("/.a", "/.a")] + [InlineData("/..a", "/..a")] + [InlineData("/...", "/...")] + [InlineData("/a/.../b", "/a/.../b")] + [InlineData("/a/../.../../b", "/b")] + [InlineData("/a/.b", "/a/.b")] + [InlineData("/a/..b", "/a/..b")] + [InlineData("/a/b.", "/a/b.")] + [InlineData("/a/b..", "/a/b..")] + [InlineData("/longlong/../short", "/short")] + [InlineData("/short/../longlong", "/longlong")] + [InlineData("/longlong/../short/..", "/")] + [InlineData("/short/../longlong/..", "/")] + [InlineData("/longlong/../short/../", "/")] + [InlineData("/short/../longlong/../", "/")] + [InlineData("/", "/")] + [InlineData("/no/segments", "/no/segments")] + [InlineData("/no/segments/", "/no/segments/")] + public void RemovesDotSegments(string input, string expected) + { + var data = Encoding.ASCII.GetBytes(input); + var length = PathNormalizer.RemoveDotSegments(new Span(data)); + Assert.True(length >= 1); + Assert.Equal(expected, Encoding.ASCII.GetString(data, 0, length)); + } + } +}