Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Components/test/E2ETest/Tests/FormsInputDateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ protected override void InitializeAsyncCore()
}

[Fact]
[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/31734")]
public void InputDateInteractsWithEditContext_NonNullableDateTime()
{
var appElement = Browser.MountTestComponent<TypicalValidationComponent>();
Expand Down Expand Up @@ -66,6 +67,7 @@ public void InputDateInteractsWithEditContext_NonNullableDateTime()
}

[Fact]
[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/67243")]
public void InputDateInteractsWithEditContext_NullableDateTimeOffset()
{
var appElement = Browser.MountTestComponent<TypicalValidationComponent>();
Expand All @@ -89,6 +91,7 @@ public void InputDateInteractsWithEditContext_NullableDateTimeOffset()
}

[Fact]
[QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/35018")]
public void InputDateInteractsWithEditContext_TimeInput()
{
var appElement = Browser.MountTestComponent<TypicalValidationComponent>();
Expand Down
60 changes: 2 additions & 58 deletions src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Globalization;
using System.Text;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Internal;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Routing;

Expand Down Expand Up @@ -313,64 +314,7 @@ internal static void NormalizeRouteValuesForPage(
}

internal static bool CheckIsLocalUrl([NotNullWhen(true)] string? url)
{
if (string.IsNullOrEmpty(url))
{
return false;
}

// Allows "/" or "/foo" but not "//" or "/\".
if (url[0] == '/')
{
// url is exactly "/"
if (url.Length == 1)
{
return true;
}

// url doesn't start with "//" or "/\"
if (url[1] != '/' && url[1] != '\\')
{
return !HasControlCharacter(url.AsSpan(1));
}

return false;
}

// Allows "~/" or "~/foo" but not "~//" or "~/\".
if (url[0] == '~' && url.Length > 1 && url[1] == '/')
{
// url is exactly "~/"
if (url.Length == 2)
{
return true;
}

// url doesn't start with "~//" or "~/\"
if (url[2] != '/' && url[2] != '\\')
{
return !HasControlCharacter(url.AsSpan(2));
}

return false;
}

return false;

static bool HasControlCharacter(ReadOnlySpan<char> readOnlySpan)
{
// URLs may not contain ASCII control characters.
for (var i = 0; i < readOnlySpan.Length; i++)
{
if (char.IsControl(readOnlySpan[i]))
{
return true;
}
}

return false;
}
}
=> SharedUrlHelper.IsLocalUrl(url);

private static object CalculatePageName(ActionContext? context, RouteValueDictionary? ambientValues, string pageName)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Text.Encodings.Web;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Internal;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

Expand Down Expand Up @@ -446,17 +447,9 @@ await Events.RedirectToReturnUrl(
}

private static bool IsHostRelative(string path)
{
if (string.IsNullOrEmpty(path))
{
return false;
}
if (path.Length == 1)
{
return path[0] == '/';
}
return path[0] == '/' && path[1] != '/' && path[1] != '\\';
}
=> !string.IsNullOrEmpty(path)
&& path[0] == '/' // Suppresses the "~/..." branch of SharedUrlHelper.IsLocalUrl so only true host-relative paths are accepted.
&& SharedUrlHelper.IsLocalUrl(path);

/// <inheritdoc />
protected override async Task HandleForbiddenAsync(AuthenticationProperties properties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

<ItemGroup>
<Compile Include="$(SharedSourceRoot)ChunkingCookieManager\**\*.cs" />
<Compile Include="$(SharedSourceRoot)ResultsHelpers\SharedUrlHelper.cs" LinkBase="Shared" />
</ItemGroup>

<ItemGroup>
Expand Down
79 changes: 79 additions & 0 deletions src/Security/Authentication/test/CookieTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,85 @@ public async Task AjaxLogoutRedirectToReturnUrlTurnsInto200WithLocationHeader()
Assert.StartsWith("/", responded.Single());
}

[Theory]
[InlineData("/\tfoo")]
[InlineData("/\rfoo")]
[InlineData("/\nfoo")]
[InlineData("/\r\nfoo")]
[InlineData("/\0foo")]
[InlineData("/\u0001foo")]
[InlineData("/\u001Ffoo")]
[InlineData("/\u007Ffoo")]
[InlineData("/foo\r\nLocation:%20evil")]
[InlineData("/foo\tbar")]
public async Task LogoutReturnUrlWithControlCharacterIsRejected(string returnUrl)
{
using var host = await CreateHost(o => o.LogoutPath = "/signout");
using var server = host.GetTestServer();
var transaction = await SendAsync(
server,
"http://example.com/signout?ReturnUrl=" + Uri.EscapeDataString(returnUrl));

Assert.False(transaction.Response.Headers.Contains("Location"));
}

[Theory]
[InlineData("/")]
[InlineData("/foo")]
[InlineData("/foo/bar")]
[InlineData("/foo?x=1#y")]
public async Task LogoutReturnUrlWithoutControlCharacterIsHonored(string returnUrl)
{
using var host = await CreateHost(o => o.LogoutPath = "/signout");
using var server = host.GetTestServer();
var transaction = await SendAsync(
server,
"http://example.com/signout?ReturnUrl=" + Uri.EscapeDataString(returnUrl));

var location = Assert.Single(transaction.Response.Headers.GetValues("Location"));
Assert.Equal(returnUrl, location);
}

[Theory]
// Protocol-relative URLs (path[1] == '/').
[InlineData("//www.example.com")]
[InlineData("//www.example.com/foobar.html")]
[InlineData("///www.example.com")]
[InlineData("//////www.example.com")]
// Forward-then-backslash (path[1] == '\\').
[InlineData("/\\")]
[InlineData("/\\foo")]
[InlineData("/\\evil.com")]
// Application-relative (~) is rejected — cookie auth never resolves PathBase like MVC's IUrlHelper.Content does.
[InlineData("~/")]
[InlineData("~/foo")]
[InlineData("~/foo.html")]
// Absolute URLs (path[0] != '/').
[InlineData("http://www.example.com")]
[InlineData("https://www.example.com")]
[InlineData("HtTpS://www.example.com")]
[InlineData("http://localhost/foobar.html")]
[InlineData("javascript:alert(1)")]
// Relative URLs (path[0] != '/').
[InlineData("foo.html")]
[InlineData("../foo.html")]
[InlineData("fold/foo.html")]
// Single-slash scheme confusion.
[InlineData("http:/foo.html")]
[InlineData("hTtP:foo.html")]
// Whitespace-only (length 1, path[0] != '/').
[InlineData(" ")]
public async Task LogoutReturnUrlNonHostRelativeIsRejected(string returnUrl)
{
using var host = await CreateHost(o => o.LogoutPath = "/signout");
using var server = host.GetTestServer();
var transaction = await SendAsync(
server,
"http://example.com/signout?ReturnUrl=" + Uri.EscapeDataString(returnUrl));

Assert.False(transaction.Response.Headers.Contains("Location"));
}

[Fact]
public async Task AjaxChallengeRedirectTurnsInto200WithLocationHeader()
{
Expand Down
4 changes: 4 additions & 0 deletions src/Shared/ResultsHelpers/SharedUrlHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ internal static class SharedUrlHelper
return contentPath;
}

// SECURITY: This is the open-redirect guard used by MVC URL helpers, Results.LocalRedirect,
// and CookieAuthenticationHandler. Changes to the accepted/rejected shapes (control characters,
// "//", "/\", "~/", etc.) must be reviewed against all call sites and the existing test corpus.
// Do not relax any check without security review.
internal static bool IsLocalUrl([NotNullWhen(true)] string? url)
{
if (string.IsNullOrEmpty(url))
Expand Down
Loading