Resolve path traversals in RazorViewEngine

This change moves the onus of path resolution from individual IFileProvider instances to RazorViewEngine.

Fixes #5574
Fixes https://github.com/aspnet/MvcPrecompilation/issues/33

src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs

@@ -6,6 +6,7 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.Globalization;
 using System.Linq;
+using System.Text;
 using System.Text.Encodings.Web;
 using Microsoft.AspNetCore.Mvc.Internal;
 using Microsoft.AspNetCore.Mvc.Razor.Internal;
@@ -32,7 +33,9 @@ namespace Microsoft.AspNetCore.Mvc.Razor
 
         private const string ControllerKey = "controller";
         private const string AreaKey = "area";
+        private const string ParentDirectoryToken = "..";
         private static readonly TimeSpan _cacheExpirationDuration = TimeSpan.FromMinutes(20);
+        private static readonly char[] _pathSeparators = new[] { '/', '\\' };
 
         private readonly IRazorPageFactoryProvider _pageFactory;
         private readonly IRazorPageActivator _pageActivator;
@@ -329,19 +332,66 @@ namespace Microsoft.AspNetCore.Mvc.Razor
                 return pagePath;
             }
 
-            // Given a relative path i.e. not yet application-relative (starting with "~/" or "/"), interpret
+            string absolutePath;
-            // path relative to currently-executing view, if any.
             if (string.IsNullOrEmpty(executingFilePath))
             {
+                // Given a relative path i.e. not yet application-relative (starting with "~/" or "/"), interpret
+                // path relative to currently-executing view, if any.
                 // Not yet executing a view. Start in app root.
-                return "/" + pagePath;
+                absolutePath = "/" + pagePath;
+            }
+            else
+            {
+                // Get directory name (including final slash) but do not use Path.GetDirectoryName() to preserve path
+                // normalization.
+                var index = executingFilePath.LastIndexOf('/');
+                Debug.Assert(index >= 0);
+                absolutePath = executingFilePath.Substring(0, index + 1) + pagePath;
+                if (!RequiresPathResolution(pagePath))
+                {
+                    return absolutePath;
+                }
             }
 
-            // Get directory name (including final slash) but do not use Path.GetDirectoryName() to preserve path
+            if (!RequiresPathResolution(pagePath))
-            // normalization.
+            {
-            var index = executingFilePath.LastIndexOf('/');
+                return absolutePath;
-            Debug.Assert(index >= 0);
+            }
-            return executingFilePath.Substring(0, index + 1) + pagePath;
+
+            var pathSegments = new List<StringSegment>();
+            var tokenizer = new StringTokenizer(absolutePath, _pathSeparators);
+            foreach (var segment in tokenizer)
+            {
+                if (segment.Length == 0)
+                {
+                    // Ignore multiple directory separators
+                    continue;
+                }
+                if (segment.Equals(ParentDirectoryToken, StringComparison.Ordinal))
+                {
+                    if (pathSegments.Count == 0)
+                    {
+                        // Don't resolve the path if we ever escape the file system root. We can't reason about it in a
+                        // consistent way.
+                        return absolutePath;
+                    }
+                    pathSegments.RemoveAt(pathSegments.Count - 1);
+                }
+                else
+                {
+                    pathSegments.Add(segment);
+                }
+            }
+
+            var builder = new StringBuilder();
+            for (var i = 0; i < pathSegments.Count; i++)
+            {
+                var segment = pathSegments[i];
+                builder.Append('/');
+                builder.Append(segment.Buffer, segment.Offset, segment.Length);
+            }
+
+            return builder.ToString();
         }
 
         private ViewLocationCacheResult OnCacheMiss(
@@ -490,5 +540,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor
             // Though ./ViewName looks like a relative path, framework searches for that view using view locations.
             return name.EndsWith(ViewExtension, StringComparison.OrdinalIgnoreCase);
         }
+
+        private static bool RequiresPathResolution(string path)
+        {
+            return path.IndexOf(ParentDirectoryToken, StringComparison.Ordinal) != -1;
+        }
     }
 }

test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs

@@ -246,6 +246,44 @@ ViewWithNestedLayout-Content
             Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true);
         }
 
+        [Fact]
+        public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider()
+        {
+            // Arrange
+            var expected =
+@"<embdedded-layout>Hello from EmbeddedShared/_Partial
+Hello from Shared/_EmbeddedPartial
+</embdedded-layout>";
+
+            // Act
+            var body = await Client.GetStringAsync("/EmbeddedViews");
+
+            // Assert
+            Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true);
+        }
+
+        [Fact]
+        public async Task RazorViewEngine_UpdatesViewsReferencedViaRelativePathsOnChange()
+        {
+            // Arrange
+            var expected1 = "Original content";
+            var expected2 = "New content";
+
+            // Act - 1
+            var body = await Client.GetStringAsync("/UpdateableFileProvider");
+
+            // Assert - 1
+            Assert.Equal(expected1, body.Trim(), ignoreLineEndingDifferences: true);
+
+            // Act - 2
+            var response = await Client.PostAsync("/UpdateableFileProvider/Update", new StringContent(string.Empty));
+            response.EnsureSuccessStatusCode();
+            body = await Client.GetStringAsync("/UpdateableFileProvider");
+
+            // Assert - 1
+            Assert.Equal(expected2, body.Trim(), ignoreLineEndingDifferences: true);
+        }
+
         [Fact]
         public async Task LayoutValueIsPassedBetweenNestedViewStarts()
         {

test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs

@@ -1419,6 +1419,39 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test
             Assert.Same(pagePath, result);
         }
 
+        [Theory]
+        [InlineData("/Views/Home/Index.cshtml", "../Shared/_Partial.cshtml")]
+        [InlineData("/Views/Home/Index.cshtml", "..\\Shared\\_Partial.cshtml")]
+        [InlineData("/Areas/MyArea/Views/Home/Index.cshtml", "../../../../Views/Shared/_Partial.cshtml")]
+        [InlineData("/Views/Accounts/Users.cshtml", "../Test/../Shared/_Partial.cshtml")]
+        public void GetAbsolutePath_ResolvesPathTraversals(string executingFilePath, string pagePath)
+        {
+            // Arrange
+            var viewEngine = CreateViewEngine();
+
+            // Act
+            var result = viewEngine.GetAbsolutePath(executingFilePath, pagePath);
+
+            // Assert
+            Assert.Equal("/Views/Shared/_Partial.cshtml", result);
+        }
+
+        [Theory]
+        [InlineData("../Shared/_Layout.cshtml")]
+        [InlineData("Folder1/../Folder2/../../File.cshtml")]
+        public void GetAbsolutePath_DoesNotResolvePathIfTraversalsEscapeTheRoot(string pagePath)
+        {
+            // Arrange
+            var expected = '/' + pagePath;
+            var viewEngine = CreateViewEngine();
+
+            // Act
+            var result = viewEngine.GetAbsolutePath("/Index.cshtml", pagePath);
+
+            // Assert
+            Assert.Equal(expected, result);
+        }
+
         [Theory]
         [InlineData(null, "/Page")]
         [InlineData(null, "~/Folder/Page.cshtml")]

test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs

@@ -0,0 +1,12 @@
+// 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 Microsoft.AspNetCore.Mvc;
+
+namespace RazorWebSite.Controllers
+{
+    public class EmbeddedViewsController : Controller
+    {
+        public IActionResult Index() => View("/Views/EmbeddedHome/Index.cshtml");
+    }
+}

test/WebSites/RazorWebSite/Controllers/UpdateableFileProviderController.cs

@@ -0,0 +1,19 @@
+// 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 Microsoft.AspNetCore.Mvc;
+
+namespace RazorWebSite
+{
+    public class UpdateableFileProviderController : Controller
+    {
+        public IActionResult Index() => View("/Views/UpdateableIndex/Index.cshtml");
+
+        [HttpPost]
+        public IActionResult Update([FromServices] UpdateableFileProvider fileProvider)
+        {
+            fileProvider.UpdateContent("/Views/UpdateableShared/_Partial.cshtml", "New content");
+            return Ok();
+        }
+    }
+}

test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/Index.cshtml

@@ -0,0 +1,3 @@
+@{ Layout = "/Views/EmbeddedShared/_Layout.cshtml"; }
+@Html.Partial("../EmbeddedShared/_Partial.cshtml")
+@Html.Partial("_EmbeddedPartial")

test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Layout.cshtml

@@ -0,0 +1 @@
+<embdedded-layout>@RenderBody()</embdedded-layout>

test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Partial.cshtml

@@ -0,0 +1 @@
+Hello from EmbeddedShared/_Partial

test/WebSites/RazorWebSite/EmbeddedViews/Views/Shared/_EmbeddedPartial.cshtml

@@ -0,0 +1 @@
+Hello from Shared/_EmbeddedPartial

test/WebSites/RazorWebSite/Services/UpdateableFileProvider.cs

@@ -0,0 +1,88 @@
+// 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.Collections.Generic;
+using System.IO;
+using System.Text;
+using System.Threading;
+using Microsoft.Extensions.FileProviders;
+using Microsoft.Extensions.Primitives;
+
+namespace RazorWebSite
+{
+    public class UpdateableFileProvider : IFileProvider
+    {
+        private readonly Dictionary<string, TestFileInfo> _content = new Dictionary<string, TestFileInfo>()
+        {
+            {
+                "/Views/UpdateableIndex/Index.cshtml",
+                new TestFileInfo(@"@Html.Partial(""../UpdateableShared/_Partial.cshtml"")")
+            },
+            {
+                "/Views/UpdateableShared/_Partial.cshtml",
+                new TestFileInfo("Original content")
+            },
+        };
+
+        public IDirectoryContents GetDirectoryContents(string subpath)
+        {
+            throw new NotImplementedException();
+        }
+
+        public void UpdateContent(string subpath, string content)
+        {
+            var old = _content[subpath];
+            old.TokenSource.Cancel();
+            _content[subpath] = new TestFileInfo(content);
+        }
+
+        public IFileInfo GetFileInfo(string subpath)
+        {
+            TestFileInfo fileInfo;
+            if (!_content.TryGetValue(subpath, out fileInfo))
+            {
+                fileInfo = new TestFileInfo(null);
+            }
+
+            return fileInfo;
+        }
+
+        public IChangeToken Watch(string filter)
+        {
+            TestFileInfo fileInfo;
+            if (_content.TryGetValue(filter, out fileInfo))
+            {
+                return fileInfo.ChangeToken;
+            }
+
+            return NullChangeToken.Singleton;
+        }
+
+        private class TestFileInfo : IFileInfo
+        {
+            private readonly string _content;
+
+            public TestFileInfo(string content)
+            {
+                _content = content;
+                ChangeToken = new CancellationChangeToken(TokenSource.Token);
+                Exists = _content != null;
+            }
+
+            public bool Exists { get; }
+            public bool IsDirectory => false;
+            public DateTimeOffset LastModified => DateTimeOffset.MinValue;
+            public long Length => -1;
+            public string Name => null;
+            public string PhysicalPath => null;
+            public CancellationTokenSource TokenSource { get; } = new CancellationTokenSource();
+            public CancellationChangeToken ChangeToken { get; }
+
+            public Stream CreateReadStream()
+            {
+                return new MemoryStream(Encoding.UTF8.GetBytes(_content));
+            }
+        }
+    }
+}

test/WebSites/RazorWebSite/Startup.cs

@@ -4,11 +4,13 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Reflection;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Localization;
 using Microsoft.AspNetCore.Mvc.Razor;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.FileProviders;
 
 namespace RazorWebSite
 {
@@ -16,10 +18,16 @@ namespace RazorWebSite
     {
         public void ConfigureServices(IServiceCollection services)
         {
+            var updateableFileProvider = new UpdateableFileProvider();
+            services.AddSingleton(updateableFileProvider);
             services
                 .AddMvc()
                 .AddRazorOptions(options =>
                 {
+                    options.FileProviders.Add(new EmbeddedFileProvider(
+                        typeof(Startup).GetTypeInfo().Assembly,
+                        $"{nameof(RazorWebSite)}.EmbeddedViews"));
+                    options.FileProviders.Add(updateableFileProvider);
                     options.ViewLocationExpanders.Add(new NonMainPageViewLocationExpander());
 #if NET451
                     options.ParseOptions = options.ParseOptions.WithPreprocessorSymbols("DNX451", "NET451_CUSTOM_DEFINE");

test/WebSites/RazorWebSite/project.json

@@ -1,7 +1,10 @@
 {
   "buildOptions": {
     "emitEntryPoint": true,
-    "preserveCompilationContext": true
+    "preserveCompilationContext": true,
+    "embed": {
+      "include": "EmbeddedViews/**"
+    }
   },
   "publishOptions": {
     "include": [
@@ -17,7 +20,8 @@
     },
     "Microsoft.AspNetCore.Server.IISIntegration": "1.2.0-*",
     "Microsoft.AspNetCore.Server.Kestrel": "1.2.0-*",
-    "Microsoft.AspNetCore.StaticFiles": "1.2.0-*"
+    "Microsoft.AspNetCore.StaticFiles": "1.2.0-*",
+    "Microsoft.Extensions.FileProviders.Embedded": "1.2.0-*"
   },
   "frameworks": {
     "net451": {