From 7a33a2c9f7f6c00307ddf71f24238429019f6dad Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 8 Apr 2026 08:22:57 +0800 Subject: [PATCH 1/4] Fix open redirect vulnerability in ValidateTokenMiddleware Validate that returnUrl is a local path before redirecting. Reject absolute URLs, protocol-relative URLs (//), and backslash escape URLs (/\) to prevent open redirect attacks. Add unit tests covering malicious returnUrl values. --- .../Model/ValidateTokenMiddleware.cs | 12 ++++++++- .../ValidateTokenMiddlewareTests.cs | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs index e741099f7b1..80d311b1587 100644 --- a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs +++ b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs @@ -68,7 +68,8 @@ public async Task InvokeAsync(HttpContext context) private static void RedirectAfterValidation(HttpContext context) { - if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl)) + if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl) + && IsLocalUrl(returnUrl.ToString())) { context.Response.Redirect(returnUrl.ToString()); } @@ -78,6 +79,15 @@ private static void RedirectAfterValidation(HttpContext context) } } + private static bool IsLocalUrl(string url) + { + // Only allow local URLs that start with "/" (but not "//" or "/\" which browsers treat as absolute). + // This mirrors the logic in ASP.NET Core's IUrlHelper.IsLocalUrl. + return url.Length > 0 + && url[0] == '/' + && (url.Length == 1 || (url[1] != '/' && url[1] != '\\')); + } + public static async Task TryAuthenticateAsync(string incomingBrowserToken, HttpContext httpContext, IOptionsMonitor dashboardOptions) { if (string.IsNullOrEmpty(incomingBrowserToken) || dashboardOptions.CurrentValue.Frontend.GetBrowserTokenBytes() is not { } expectedBrowserTokenBytes) diff --git a/tests/Aspire.Dashboard.Tests/Middleware/ValidateTokenMiddlewareTests.cs b/tests/Aspire.Dashboard.Tests/Middleware/ValidateTokenMiddlewareTests.cs index e887dba46e4..12841fe60ff 100644 --- a/tests/Aspire.Dashboard.Tests/Middleware/ValidateTokenMiddlewareTests.cs +++ b/tests/Aspire.Dashboard.Tests/Middleware/ValidateTokenMiddlewareTests.cs @@ -63,6 +63,32 @@ public async Task ValidateToken_BrowserTokenAuth_RightToken_RedirectsToReturnUrl Assert.Equal("/test", response.Headers.Location?.OriginalString); } + [Theory] + [InlineData("https://evil.com")] + [InlineData("http://evil.com")] + [InlineData("//evil.com")] + [InlineData("//evil.com/path")] + [InlineData("/\\evil.com")] + public async Task ValidateToken_NotBrowserTokenAuth_AbsoluteReturnUrl_RedirectsToHomepage(string returnUrl) + { + using var host = await SetUpHostAsync(FrontendAuthMode.Unsecured, string.Empty).DefaultTimeout(); + var response = await host.GetTestClient().GetAsync($"/login?t=test&returnUrl={Uri.EscapeDataString(returnUrl)}").DefaultTimeout(); + Assert.Equal("/", response.Headers.Location?.OriginalString); + } + + [Theory] + [InlineData("https://evil.com")] + [InlineData("http://evil.com")] + [InlineData("//evil.com")] + [InlineData("//evil.com/path")] + [InlineData("/\\evil.com")] + public async Task ValidateToken_BrowserTokenAuth_RightToken_AbsoluteReturnUrl_RedirectsToHomepage(string returnUrl) + { + using var host = await SetUpHostAsync(FrontendAuthMode.BrowserToken, "token").DefaultTimeout(); + var response = await host.GetTestClient().GetAsync($"/login?t=token&returnUrl={Uri.EscapeDataString(returnUrl)}").DefaultTimeout(); + Assert.Equal("/", response.Headers.Location?.OriginalString); + } + private static async Task SetUpHostAsync(FrontendAuthMode authMode, string expectedToken) { return await new HostBuilder() From 8010ef764cb88bfad267c5a1cc57d817ea773ca7 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 8 Apr 2026 08:33:26 +0800 Subject: [PATCH 2/4] Use full ASP.NET Core IsLocalUrl implementation for returnUrl validation --- .../Model/ValidateTokenMiddleware.cs | 66 +++++++++++++++++-- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs index 80d311b1587..0c038d6a22c 100644 --- a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs +++ b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.Extensions.Options; using System.Web; +using System.Diagnostics.CodeAnalysis; namespace Aspire.Dashboard.Model; @@ -79,13 +80,66 @@ private static void RedirectAfterValidation(HttpContext context) } } - private static bool IsLocalUrl(string url) + // Copied from ASP.NET Core's IsLocalUrl implementation: + // https://github.com/dotnet/aspnetcore/blob/7cbda0e023075490b4365a0754ca410ce6eff59a/src/Shared/ResultsHelpers/SharedUrlHelper.cs#L33 + internal static bool IsLocalUrl([NotNullWhen(true)] string? url) { - // Only allow local URLs that start with "/" (but not "//" or "/\" which browsers treat as absolute). - // This mirrors the logic in ASP.NET Core's IUrlHelper.IsLocalUrl. - return url.Length > 0 - && url[0] == '/' - && (url.Length == 1 || (url[1] != '/' && url[1] != '\\')); + 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; + } } public static async Task TryAuthenticateAsync(string incomingBrowserToken, HttpContext httpContext, IOptionsMonitor dashboardOptions) From ad14a0e1d1091595c12d4edafcca3e7939a16813 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 8 Apr 2026 09:01:48 +0800 Subject: [PATCH 3/4] Apply suggestion from @JamesNK --- src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs index 0c038d6a22c..0e6e009120c 100644 --- a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs +++ b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs @@ -69,6 +69,8 @@ public async Task InvokeAsync(HttpContext context) private static void RedirectAfterValidation(HttpContext context) { + // An optional returnurl value must be a valid URL and it must be local. + // We don't want an open redirect (the potential to create a URL that redirects to a third-party site). if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl) && IsLocalUrl(returnUrl.ToString())) { From 67b01ab61df8b403ba1c6ccc55227713cd287c2a Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 8 Apr 2026 11:00:56 +0800 Subject: [PATCH 4/4] Move IsLocalUrl to shared UrlValidationHelper and fix Login page open redirect - Extract IsLocalUrl into UrlValidationHelper.cs shared utility - Add IsSafeRedirectUrl that validates URI format and locality - Fix Login.razor.cs GetRedirectUrl() to validate ReturnUrl - Remove inlined IsLocalUrl from ValidateTokenMiddleware --- .../Components/Pages/Login.razor.cs | 2 +- .../Model/ValidateTokenMiddleware.cs | 65 +-------------- .../Utils/UrlValidationHelper.cs | 83 +++++++++++++++++++ 3 files changed, 85 insertions(+), 65 deletions(-) create mode 100644 src/Aspire.Dashboard/Utils/UrlValidationHelper.cs diff --git a/src/Aspire.Dashboard/Components/Pages/Login.razor.cs b/src/Aspire.Dashboard/Components/Pages/Login.razor.cs index b55bac08ced..7be7ac38ffb 100644 --- a/src/Aspire.Dashboard/Components/Pages/Login.razor.cs +++ b/src/Aspire.Dashboard/Components/Pages/Login.razor.cs @@ -108,7 +108,7 @@ private async Task SubmitAsync() private string GetRedirectUrl() { - return ReturnUrl ?? DashboardUrls.ResourcesUrl(); + return UrlValidationHelper.IsSafeRedirectUrl(ReturnUrl) ? ReturnUrl : DashboardUrls.ResourcesUrl(); } public async ValueTask DisposeAsync() diff --git a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs index 0e6e009120c..29b4fd0a13e 100644 --- a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs +++ b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs @@ -8,7 +8,6 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.Extensions.Options; using System.Web; -using System.Diagnostics.CodeAnalysis; namespace Aspire.Dashboard.Model; @@ -72,7 +71,7 @@ private static void RedirectAfterValidation(HttpContext context) // An optional returnurl value must be a valid URL and it must be local. // We don't want an open redirect (the potential to create a URL that redirects to a third-party site). if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl) - && IsLocalUrl(returnUrl.ToString())) + && UrlValidationHelper.IsSafeRedirectUrl(returnUrl.ToString())) { context.Response.Redirect(returnUrl.ToString()); } @@ -82,68 +81,6 @@ private static void RedirectAfterValidation(HttpContext context) } } - // Copied from ASP.NET Core's IsLocalUrl implementation: - // https://github.com/dotnet/aspnetcore/blob/7cbda0e023075490b4365a0754ca410ce6eff59a/src/Shared/ResultsHelpers/SharedUrlHelper.cs#L33 - internal static bool IsLocalUrl([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; - } - } - public static async Task TryAuthenticateAsync(string incomingBrowserToken, HttpContext httpContext, IOptionsMonitor dashboardOptions) { if (string.IsNullOrEmpty(incomingBrowserToken) || dashboardOptions.CurrentValue.Frontend.GetBrowserTokenBytes() is not { } expectedBrowserTokenBytes) diff --git a/src/Aspire.Dashboard/Utils/UrlValidationHelper.cs b/src/Aspire.Dashboard/Utils/UrlValidationHelper.cs new file mode 100644 index 00000000000..84207d205a8 --- /dev/null +++ b/src/Aspire.Dashboard/Utils/UrlValidationHelper.cs @@ -0,0 +1,83 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; + +namespace Aspire.Dashboard.Utils; + +/// +/// Helpers for validating redirect URLs. +/// +internal static class UrlValidationHelper +{ + /// + /// Checks whether a URL is safe to use as a redirect target. + /// The URL must be a valid URI and a local path (not absolute or protocol-relative). + /// + internal static bool IsSafeRedirectUrl([NotNullWhen(true)] string? url) + { + return Uri.TryCreate(url, UriKind.Relative, out _) && IsLocalUrl(url); + } + + // Copied from ASP.NET Core's IsLocalUrl implementation: + // https://github.com/dotnet/aspnetcore/blob/7cbda0e023075490b4365a0754ca410ce6eff59a/src/Shared/ResultsHelpers/SharedUrlHelper.cs#L33 + internal static bool IsLocalUrl([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; + } + } +}