diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs index a77cd5882a..a52bed2e63 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[] TokensRequiringNormalization = 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) { @@ -42,18 +59,18 @@ public static string CombinePath(string first, string second) result = first.Substring(0, index + 1) + second; } - return ResolvePath(result); + return NormalizePath(result); } - public static string ResolvePath(string path) + public static string NormalizePath(string path) { - if (!RequiresPathResolution(path)) + if (!RequiresPathNormalization(path)) { return 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) @@ -93,10 +110,17 @@ public static string ResolvePath(string path) return builder.ToString(); } - private static bool RequiresPathResolution(string path) + private static bool RequiresPathNormalization(string path) { - return path.IndexOf(ParentDirectoryToken, StringComparison.Ordinal) != -1 || - path.IndexOf(CurrentDirectoryToken, StringComparison.Ordinal) != -1; + for (var i = 0; i < TokensRequiringNormalization.Length; i++) + { + if (path.IndexOf(TokensRequiringNormalization[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..f4a13cceae 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); @@ -316,7 +315,7 @@ public string GetAbsolutePath(string executingFilePath, string pagePath) // path relative to currently-executing view, if any. // Not yet executing a view. Start in app root. var absolutePath = "/" + pagePath; - return ViewEnginePath.ResolvePath(absolutePath); + return ViewEnginePath.NormalizePath(absolutePath); } return ViewEnginePath.CombinePath(executingFilePath, pagePath); @@ -369,6 +368,8 @@ private ViewLocationCacheResult OnCacheMiss( expanderContext.ControllerName, expanderContext.AreaName); + path = ViewEnginePath.NormalizePath(path); + cacheResult = CreateCacheResult(expirationTokens, path, expanderContext.IsMainPage); if (cacheResult != null) { 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());