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 e741099f7b1..29b4fd0a13e 100644 --- a/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs +++ b/src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs @@ -68,7 +68,10 @@ public async Task InvokeAsync(HttpContext context) private static void RedirectAfterValidation(HttpContext context) { - if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl)) + // 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) + && UrlValidationHelper.IsSafeRedirectUrl(returnUrl.ToString())) { context.Response.Redirect(returnUrl.ToString()); } 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; + } + } +} 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()