From b4048c3cde567a4293b7ffa092640eb35f3df49c Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 5 Sep 2017 16:07:17 -0700 Subject: [PATCH] Normalize paths in RazorViewEngine prior to invoking page factory Fixes #6672 --- .../Internal/ViewEnginePath.cs | 32 +++++++- .../RazorViewEngine.cs | 5 +- .../Internal/ViewEnginePathTest.cs | 49 ++++++++++++ .../ViewEngineTests.cs | 34 +++++++- .../RazorViewEngineTest.cs | 77 ++++++++++++++++++- .../Controllers/EmbeddedViewsController.cs | 6 +- .../Views/EmbeddedShared/_Layout.cshtml | 0 .../Views/EmbeddedShared/_Partial.cshtml | 0 .../EmbeddedViews/EmbeddedPartial.cshtml | 1 + .../Views/EmbeddedViews}/Index.cshtml | 0 .../EmbeddedViews/RelativeNonPath.cshtml | 2 + .../Views/EmbeddedViews}/_ViewImports.cshtml | 0 .../Views/EmbeddedViews}/_ViewStart.cshtml | 0 .../Views/Shared/_EmbeddedPartial.cshtml | 0 .../WebSites/RazorWebSite/RazorWebSite.csproj | 2 +- test/WebSites/RazorWebSite/Startup.cs | 2 +- 16 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs rename test/WebSites/RazorWebSite/{EmbeddedViews => EmbeddedResources}/Views/EmbeddedShared/_Layout.cshtml (100%) rename test/WebSites/RazorWebSite/{EmbeddedViews => EmbeddedResources}/Views/EmbeddedShared/_Partial.cshtml (100%) create mode 100644 test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml rename test/WebSites/RazorWebSite/{EmbeddedViews/Views/EmbeddedHome => EmbeddedResources/Views/EmbeddedViews}/Index.cshtml (100%) create mode 100644 test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml rename test/WebSites/RazorWebSite/{EmbeddedViews/Views/EmbeddedHome => EmbeddedResources/Views/EmbeddedViews}/_ViewImports.cshtml (100%) rename test/WebSites/RazorWebSite/{EmbeddedViews/Views/EmbeddedHome => EmbeddedResources/Views/EmbeddedViews}/_ViewStart.cshtml (100%) rename test/WebSites/RazorWebSite/{EmbeddedViews => EmbeddedResources}/Views/Shared/_EmbeddedPartial.cshtml (100%) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs index a77cd5882a..5fc9b3da3e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs @@ -11,9 +11,26 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public static class ViewEnginePath { + public static readonly char[] PathSeparators = new[] { '/', '\\' }; private const string CurrentDirectoryToken = "."; private const string ParentDirectoryToken = ".."; - private static readonly char[] _pathSeparators = new[] { '/', '\\' }; + // Tokens that suggest directory traversal such as ./, \.. etc. All parent directory traversals (such as "..\") are covered + // by current directory tokens (".\"). + private static readonly string[] TokensRequiringResolution = new string[] + { + // ./ + CurrentDirectoryToken + PathSeparators[0], + // .\ + CurrentDirectoryToken + PathSeparators[1], + // /. + PathSeparators[0] + CurrentDirectoryToken, + // \. + PathSeparators[1] + CurrentDirectoryToken, + // // + string.Empty + PathSeparators[0] + PathSeparators[0], + // \\ + string.Empty + PathSeparators[1] + PathSeparators[1], + }; public static string CombinePath(string first, string second) { @@ -53,7 +70,7 @@ public static string ResolvePath(string path) } var pathSegments = new List(); - var tokenizer = new StringTokenizer(path, _pathSeparators); + var tokenizer = new StringTokenizer(path, PathSeparators); foreach (var segment in tokenizer) { if (segment.Length == 0) @@ -95,8 +112,15 @@ public static string ResolvePath(string path) private static bool RequiresPathResolution(string path) { - return path.IndexOf(ParentDirectoryToken, StringComparison.Ordinal) != -1 || - path.IndexOf(CurrentDirectoryToken, StringComparison.Ordinal) != -1; + for (var i = 0; i < TokensRequiringResolution.Length; i++) + { + if (path.IndexOf(TokensRequiringResolution[i]) != -1) + { + return true; + } + } + + return false; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs index ad0ec5e1cf..d51099471d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs @@ -205,8 +205,7 @@ private ViewLocationCacheResult LocatePageFromPath(string executingFilePath, str { var applicationRelativePath = GetAbsolutePath(executingFilePath, pagePath); var cacheKey = new ViewLocationCacheKey(applicationRelativePath, isMainPage); - ViewLocationCacheResult cacheResult; - if (!ViewLookupCache.TryGetValue(cacheKey, out cacheResult)) + if (!ViewLookupCache.TryGetValue(cacheKey, out ViewLocationCacheResult cacheResult)) { var expirationTokens = new HashSet(); cacheResult = CreateCacheResult(expirationTokens, applicationRelativePath, isMainPage); @@ -369,6 +368,8 @@ private ViewLocationCacheResult OnCacheMiss( expanderContext.ControllerName, expanderContext.AreaName); + path = ViewEnginePath.ResolvePath(path); + cacheResult = CreateCacheResult(expirationTokens, path, expanderContext.IsMainPage); if (cacheResult != null) { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs new file mode 100644 index 0000000000..4201d029cf --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ViewEnginePathTest.cs @@ -0,0 +1,49 @@ +// 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 Xunit; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ViewEnginePathTest + { + [Theory] + [InlineData("Views/../Home/Index.cshtml", "/Home/Index.cshtml")] + [InlineData("/Views/Home/../Shared/Partial.cshtml", "/Views/Shared/Partial.cshtml")] + [InlineData("/Views/Shared/./Partial.cshtml", "/Views/Shared/Partial.cshtml")] + public void ResolvePath_ResolvesPathTraversals(string input, string expected) + { + // Arrange & Act + var result = ViewEnginePath.ResolvePath(input); + + // Assert + Assert.Equal(expected, result); + } + + [Theory] + [InlineData("../Index.cshtml")] + [InlineData("Views/../../Index.cshtml")] + [InlineData("Views/../Shared/../../Index.cshtml")] + public void ResolvePath_DoesNotTraversePastApplicationRoot(string input) + { + // Arrange + var result = ViewEnginePath.ResolvePath(input); + + // Assert + Assert.Same(input, result); + } + + [Theory] + [InlineData("/Views/Index.cshtml")] + [InlineData(@"/Views\Index.cshtml")] + [InlineData(@"Index..cshtml")] + public void ResolvePath_DoesNotModifyPathsWithoutTraversals(string input) + { + // Arrange & Act + var result = ViewEnginePath.ResolvePath(input); + + // Assert + Assert.Same(input, result); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs index d8c8ac6328..ffa82916f6 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs @@ -247,7 +247,7 @@ public async Task RazorViewEngine_RendersPartialViews(string actionName, string } [Fact] - public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider() + public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider_WhenLookedupByName() { // Arrange var expected = @@ -257,7 +257,24 @@ Hello from Shared/_EmbeddedPartial "; // Act - var body = await Client.GetStringAsync("/EmbeddedViews"); + var body = await Client.GetStringAsync("/EmbeddedViews/LookupByName"); + + // Assert + Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true); + } + + [Fact] + public async Task RazorViewEngine_RendersViewsFromEmbeddedFileProvider_WhenLookedupByPath() + { + // Arrange + var expected = +@"Hello from EmbeddedShared/_Partial +Hello from Shared/_EmbeddedPartial +Tag Helper Link +"; + + // Act + var body = await Client.GetStringAsync("/EmbeddedViews/LookupByPath"); // Assert Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true); @@ -493,5 +510,18 @@ public async Task ViewEngine_NormalizesPathsReturnedByViewLocationExpanders() // Assert Assert.Equal(expected, responseContent.Trim()); } + + [Fact] + public async Task ViewEngine_ResolvesPathsWithSlashesThatDoNotHaveExtensions() + { + // Arrange + var expected = @"Hello from EmbeddedHome\EmbeddedPartial"; + + // Act + var responseContent = await Client.GetStringAsync("/EmbeddedViews/RelativeNonPath"); + + // Assert + Assert.Equal(expected, responseContent.Trim()); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs index 3c8d4350ec..b4f8c3ee72 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs @@ -24,7 +24,7 @@ using Moq; using Xunit; -namespace Microsoft.AspNetCore.Mvc.Razor.Test +namespace Microsoft.AspNetCore.Mvc.Razor { public class RazorViewEngineTest { @@ -1864,6 +1864,81 @@ public void ViewEngine_SetsPageValue_IfItIsSpecifiedInRouteValues() expander.Verify(); } + [Fact] + public void FindView_ResolvesDirectoryTraversalsPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindView(context, "../Shared/_Partial", isMainPage: false); + + // Assert + pageFactory.Verify(); + } + + [Fact] + public void FindPage_ResolvesDirectoryTraversalsPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindPage(context, "../Shared/_Partial"); + + // Assert + pageFactory.Verify(); + } + + // Prior to normalizing the path, the view engine would have attempted to lookup "/Views//MyView.cshtml" + // which works for PhysicalFileProvider but fails for exact lookups performed during precompilation. + // We normalize it to "/Views/MyView.cshtml" to avoid this discrepancy. + [Fact] + public void FindView_ResolvesNormalizesSlashesPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/MyView.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindView(context, "MyView", isMainPage: true); + + // Assert + pageFactory.Verify(); + } + + [Fact] + public void FindPage_ResolvesNormalizesSlashesPriorToInvokingPageFactory() + { + // Arrange + var pageFactory = new Mock(); + pageFactory.Setup(p => p.CreateFactory("/Views/MyPage.cshtml")) + .Returns(GetPageFactoryResult(() => Mock.Of())) + .Verifiable(); + var viewEngine = CreateViewEngine(pageFactory.Object); + var context = GetActionContext(new Dictionary()); + + // Act + var result = viewEngine.FindPage(context, "MyPage"); + + // Assert + pageFactory.Verify(); + } + // Return RazorViewEngine with a page factory provider that is always successful. private RazorViewEngine CreateSuccessfulViewEngine() { diff --git a/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs b/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs index 33837ea026..ca653f83a0 100644 --- a/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs +++ b/test/WebSites/RazorWebSite/Controllers/EmbeddedViewsController.cs @@ -7,6 +7,10 @@ namespace RazorWebSite.Controllers { public class EmbeddedViewsController : Controller { - public IActionResult Index() => View("/Views/EmbeddedHome/Index.cshtml"); + public IActionResult LookupByName() => View("Index"); + + public IActionResult LookupByPath() => View("/Views/EmbeddedViews/Index.cshtml"); + + public IActionResult RelativeNonPath() => View(); } } diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Layout.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Layout.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Layout.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Layout.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Partial.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Partial.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedShared/_Partial.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedShared/_Partial.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml new file mode 100644 index 0000000000..cebe57816a --- /dev/null +++ b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/EmbeddedPartial.cshtml @@ -0,0 +1 @@ +Hello from EmbeddedHome\EmbeddedPartial \ No newline at end of file diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/Index.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/Index.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/Index.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/Index.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml new file mode 100644 index 0000000000..1aca712209 --- /dev/null +++ b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/RelativeNonPath.cshtml @@ -0,0 +1,2 @@ +@{ Layout = "../EmbeddedShared/_Layout"; } +@Html.Partial("./EmbeddedPartial") \ No newline at end of file diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewImports.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewImports.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewImports.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewImports.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewStart.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewStart.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/EmbeddedHome/_ViewStart.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/EmbeddedViews/_ViewStart.cshtml diff --git a/test/WebSites/RazorWebSite/EmbeddedViews/Views/Shared/_EmbeddedPartial.cshtml b/test/WebSites/RazorWebSite/EmbeddedResources/Views/Shared/_EmbeddedPartial.cshtml similarity index 100% rename from test/WebSites/RazorWebSite/EmbeddedViews/Views/Shared/_EmbeddedPartial.cshtml rename to test/WebSites/RazorWebSite/EmbeddedResources/Views/Shared/_EmbeddedPartial.cshtml diff --git a/test/WebSites/RazorWebSite/RazorWebSite.csproj b/test/WebSites/RazorWebSite/RazorWebSite.csproj index 5ac6a97546..dac6890ecb 100644 --- a/test/WebSites/RazorWebSite/RazorWebSite.csproj +++ b/test/WebSites/RazorWebSite/RazorWebSite.csproj @@ -7,7 +7,7 @@ - + diff --git a/test/WebSites/RazorWebSite/Startup.cs b/test/WebSites/RazorWebSite/Startup.cs index 4bb646bc5c..7fb43f75e8 100644 --- a/test/WebSites/RazorWebSite/Startup.cs +++ b/test/WebSites/RazorWebSite/Startup.cs @@ -30,7 +30,7 @@ public void ConfigureServices(IServiceCollection services) { options.FileProviders.Add(new EmbeddedFileProvider( typeof(Startup).GetTypeInfo().Assembly, - $"{nameof(RazorWebSite)}.EmbeddedViews")); + $"{nameof(RazorWebSite)}.EmbeddedResources")); options.FileProviders.Add(updateableFileProvider); options.ViewLocationExpanders.Add(new NonMainPageViewLocationExpander()); options.ViewLocationExpanders.Add(new ForwardSlashExpander());