Merged PR 1480: [2.1] HttpSys path normalization

This takes PathNormalizer from Kestrel to normalize the request path and prevent traversals. (e.g. "/./" and "/../").

In 2.1 only HttpSys was affected. In 2.2 HttpSys and IIS-in-proc will share this code (with additional tests). In 3.0 we'll refactor it to use more shared source across all three servers.

eng/PatchConfig.props

@@ -37,6 +37,7 @@ Later on, this will be checked using this condition:
   </PropertyGroup>
   <PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.12' ">
     <PackagesInPatch>
+      Microsoft.AspNetCore.Server.HttpSys;
     </PackagesInPatch>
   </PropertyGroup>
 </Project>

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.

src/Shared/HttpSys/RequestProcessing/PathNormalizer.cs

@@ -0,0 +1,208 @@
+// 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;
+using System.Runtime.InteropServices;
+
+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 = &MemoryMarshal.GetReference(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;
+        }
+    }
+}

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>

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.1;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>

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));
+        }
+    }
+}