Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Commit

Permalink
Normalize paths in RazorViewEngine prior to invoking page factory
Browse files Browse the repository at this point in the history
Fixes #6672
  • Loading branch information
pranavkm committed Sep 7, 2017
1 parent a7cc243 commit b4048c3
Show file tree
Hide file tree
Showing 16 changed files with 198 additions and 12 deletions.
32 changes: 28 additions & 4 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ViewEnginePath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -53,7 +70,7 @@ public static string ResolvePath(string path)
}

var pathSegments = new List<StringSegment>();
var tokenizer = new StringTokenizer(path, _pathSeparators);
var tokenizer = new StringTokenizer(path, PathSeparators);
foreach (var segment in tokenizer)
{
if (segment.Length == 0)
Expand Down Expand Up @@ -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;
}
}
}
5 changes: 3 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<IChangeToken>();
cacheResult = CreateCacheResult(expirationTokens, applicationRelativePath, isMainPage);
Expand Down Expand Up @@ -369,6 +368,8 @@ private ViewLocationCacheResult OnCacheMiss(
expanderContext.ControllerName,
expanderContext.AreaName);

path = ViewEnginePath.ResolvePath(path);

cacheResult = CreateCacheResult(expirationTokens, path, expanderContext.IsMainPage);
if (cacheResult != null)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
34 changes: 32 additions & 2 deletions test/Microsoft.AspNetCore.Mvc.FunctionalTests/ViewEngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -257,7 +257,24 @@ Hello from Shared/_EmbeddedPartial
</embdedded-layout>";

// 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 =
@"<embdedded-layout>Hello from EmbeddedShared/_Partial
Hello from Shared/_EmbeddedPartial
<a href=""/EmbeddedViews"">Tag Helper Link</a>
</embdedded-layout>";

// Act
var body = await Client.GetStringAsync("/EmbeddedViews/LookupByPath");

// Assert
Assert.Equal(expected, body.Trim(), ignoreLineEndingDifferences: true);
Expand Down Expand Up @@ -493,5 +510,18 @@ public async Task ViewEngine_NormalizesPathsReturnedByViewLocationExpanders()
// Assert
Assert.Equal(expected, responseContent.Trim());
}

[Fact]
public async Task ViewEngine_ResolvesPathsWithSlashesThatDoNotHaveExtensions()
{
// Arrange
var expected = @"<embdedded-layout>Hello from EmbeddedHome\EmbeddedPartial</embdedded-layout>";

// Act
var responseContent = await Client.GetStringAsync("/EmbeddedViews/RelativeNonPath");

// Assert
Assert.Equal(expected, responseContent.Trim());
}
}
}
77 changes: 76 additions & 1 deletion test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
using Moq;
using Xunit;

namespace Microsoft.AspNetCore.Mvc.Razor.Test
namespace Microsoft.AspNetCore.Mvc.Razor
{
public class RazorViewEngineTest
{
Expand Down Expand Up @@ -1864,6 +1864,81 @@ public void ViewEngine_SetsPageValue_IfItIsSpecifiedInRouteValues()
expander.Verify();
}

[Fact]
public void FindView_ResolvesDirectoryTraversalsPriorToInvokingPageFactory()
{
// Arrange
var pageFactory = new Mock<IRazorPageFactoryProvider>();
pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml"))
.Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
.Verifiable();
var viewEngine = CreateViewEngine(pageFactory.Object);
var context = GetActionContext(new Dictionary<string, object>());

// Act
var result = viewEngine.FindView(context, "../Shared/_Partial", isMainPage: false);

// Assert
pageFactory.Verify();
}

[Fact]
public void FindPage_ResolvesDirectoryTraversalsPriorToInvokingPageFactory()
{
// Arrange
var pageFactory = new Mock<IRazorPageFactoryProvider>();
pageFactory.Setup(p => p.CreateFactory("/Views/Shared/_Partial.cshtml"))
.Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
.Verifiable();
var viewEngine = CreateViewEngine(pageFactory.Object);
var context = GetActionContext(new Dictionary<string, object>());

// 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<IRazorPageFactoryProvider>();
pageFactory.Setup(p => p.CreateFactory("/Views/MyView.cshtml"))
.Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
.Verifiable();
var viewEngine = CreateViewEngine(pageFactory.Object);
var context = GetActionContext(new Dictionary<string, object>());

// Act
var result = viewEngine.FindView(context, "MyView", isMainPage: true);

// Assert
pageFactory.Verify();
}

[Fact]
public void FindPage_ResolvesNormalizesSlashesPriorToInvokingPageFactory()
{
// Arrange
var pageFactory = new Mock<IRazorPageFactoryProvider>();
pageFactory.Setup(p => p.CreateFactory("/Views/MyPage.cshtml"))
.Returns(GetPageFactoryResult(() => Mock.Of<IRazorPage>()))
.Verifiable();
var viewEngine = CreateViewEngine(pageFactory.Object);
var context = GetActionContext(new Dictionary<string, object>());

// Act
var result = viewEngine.FindPage(context, "MyPage");

// Assert
pageFactory.Verify();
}

// Return RazorViewEngine with a page factory provider that is always successful.
private RazorViewEngine CreateSuccessfulViewEngine()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hello from EmbeddedHome\EmbeddedPartial
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@{ Layout = "../EmbeddedShared/_Layout"; }
@Html.Partial("./EmbeddedPartial")
2 changes: 1 addition & 1 deletion test/WebSites/RazorWebSite/RazorWebSite.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</PropertyGroup>

<ItemGroup>
<EmbeddedResource Include="EmbeddedViews\**" />
<EmbeddedResource Include="EmbeddedResources\**" />
</ItemGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion test/WebSites/RazorWebSite/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit b4048c3

Please sign in to comment.