diff options
9 files changed, 533 insertions, 5 deletions
diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index e10a485b99..3daf32fba4 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -69,6 +69,8 @@ Later on, this will be checked using this condition: <PropertyGroup Condition=" '$(VersionPrefix)' == '2.2.6' "> <PackagesInPatch> Microsoft.AspNetCore.Mvc.Api.Analyzers; + Microsoft.AspNetCore.Server.HttpSys; + Microsoft.AspNetCore.Server.IIS; </PackagesInPatch> </PropertyGroup> </Project> 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<IHttpRequestFeature>(); + 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<IHttpRequestFeature>(); + 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<IHttpRequestFeature>(); + 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<IHttpRequestFeature>(); + 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..f4ae7dbc02 --- /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.IISIntegration.FunctionalTests +{ + [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/Common.Tests/Utilities/TestConnections.cs b/src/Servers/IIS/IIS/test/Common.Tests/Utilities/TestConnections.cs index 3b7a870cf3..d54e06cb4d 100644 --- a/src/Servers/IIS/IIS/test/Common.Tests/Utilities/TestConnections.cs +++ b/src/Servers/IIS/IIS/test/Common.Tests/Utilities/TestConnections.cs @@ -119,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting Assert.Equal(expected, Encoding.ASCII.GetString(actual.Span)); } - private async Task<Memory<byte>> Receive(int length) + public async Task<Memory<byte>> 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 c19b4e80db..a4296faa9e 100644 --- a/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs +++ b/src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs @@ -673,6 +673,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..f92066f310 --- /dev/null +++ b/src/Shared/HttpSys/RequestProcessing/PathNormalizer.cs @@ -0,0 +1,206 @@ +// 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<byte> 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 6308d6d8ea..fbc1101436 100644 --- a/src/Shared/HttpSys/RequestProcessing/RequestUriBuilder.cs +++ b/src/Shared/HttpSys/RequestProcessing/RequestUriBuilder.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; @@ -33,7 +33,9 @@ namespace Microsoft.AspNetCore.HttpSys.Internal var unescapedPath = Unescape(rawPath); - return UTF8.GetString(unescapedPath.Array, unescapedPath.Offset, unescapedPath.Count); + var length = PathNormalizer.RemoveDotSegments(unescapedPath); + + return UTF8.GetString(unescapedPath.Array, unescapedPath.Offset, length); } /// <summary> 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 cc3f3d588c..bde7735594 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 @@ -<Project Sdk="Microsoft.NET.Sdk"> +<Project Sdk="Microsoft.NET.Sdk"> <PropertyGroup> <TargetFrameworks>netcoreapp2.2;net461</TargetFrameworks> <DebugType>portable</DebugType> + <AllowUnsafeBlocks>true</AllowUnsafeBlocks> </PropertyGroup> <ItemGroup> <Compile Include="$(SharedSourceRoot)ClosedGenericMatcher\*.cs" /> <Compile Include="$(SharedSourceRoot)CopyOnWriteDictionary\*.cs" /> + <Compile Include="$(SharedSourceRoot)HttpSys\**\*.cs" /> <Compile Include="$(SharedSourceRoot)ObjectMethodExecutor\*.cs" /> <Compile Include="$(SharedSourceRoot)PropertyActivator\*.cs" /> <Compile Include="$(SharedSourceRoot)PropertyHelper\*.cs" /> @@ -19,8 +21,11 @@ <ItemGroup> <Reference Include="FSharp.Core" /> <Reference Include="System.Reflection.Metadata" /> + <Reference Include="System.Security.Principal.Windows" /> <Reference Include="System.Threading.Tasks.Extensions" /> + <Reference Include="Microsoft.AspNetCore.Http.Features" /> <Reference Include="Microsoft.Extensions.TypeNameHelper.Sources" PrivateAssets="All" /> + <Reference Include="Microsoft.Net.Http.Headers" /> </ItemGroup> <ItemGroup> 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<byte>(data)); + Assert.True(length >= 1); + Assert.Equal(expected, Encoding.ASCII.GetString(data, 0, length)); + } + } +} |