diff --git a/src/Components/test/E2ETest/Tests/FormsInputDateTest.cs b/src/Components/test/E2ETest/Tests/FormsInputDateTest.cs index 0e034d019e6f..2cf0898a5a2a 100644 --- a/src/Components/test/E2ETest/Tests/FormsInputDateTest.cs +++ b/src/Components/test/E2ETest/Tests/FormsInputDateTest.cs @@ -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(); @@ -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(); @@ -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(); diff --git a/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs b/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs index 2b0fa0b1a8f2..c1598ac349ec 100644 --- a/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs +++ b/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs @@ -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; @@ -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 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) { diff --git a/src/Security/Authentication/Cookies/src/CookieAuthenticationHandler.cs b/src/Security/Authentication/Cookies/src/CookieAuthenticationHandler.cs index 0e9ad0883808..bce83818b29b 100644 --- a/src/Security/Authentication/Cookies/src/CookieAuthenticationHandler.cs +++ b/src/Security/Authentication/Cookies/src/CookieAuthenticationHandler.cs @@ -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; @@ -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); /// protected override async Task HandleForbiddenAsync(AuthenticationProperties properties) diff --git a/src/Security/Authentication/Cookies/src/Microsoft.AspNetCore.Authentication.Cookies.csproj b/src/Security/Authentication/Cookies/src/Microsoft.AspNetCore.Authentication.Cookies.csproj index 6a3843e97683..254c5623a081 100644 --- a/src/Security/Authentication/Cookies/src/Microsoft.AspNetCore.Authentication.Cookies.csproj +++ b/src/Security/Authentication/Cookies/src/Microsoft.AspNetCore.Authentication.Cookies.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Security/Authentication/test/CookieTests.cs b/src/Security/Authentication/test/CookieTests.cs index bc9ff451415d..41ad1c911b5e 100644 --- a/src/Security/Authentication/test/CookieTests.cs +++ b/src/Security/Authentication/test/CookieTests.cs @@ -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() { diff --git a/src/Shared/ResultsHelpers/SharedUrlHelper.cs b/src/Shared/ResultsHelpers/SharedUrlHelper.cs index aa64d071bfef..e1c301378f6e 100644 --- a/src/Shared/ResultsHelpers/SharedUrlHelper.cs +++ b/src/Shared/ResultsHelpers/SharedUrlHelper.cs @@ -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))