Skip to content

Commit a182eb9

Browse files
authored
Fix open redirect vulnerability (#15952)
1 parent 07181cf commit a182eb9

4 files changed

Lines changed: 114 additions & 2 deletions

File tree

src/Aspire.Dashboard/Components/Pages/Login.razor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ private async Task SubmitAsync()
108108

109109
private string GetRedirectUrl()
110110
{
111-
return ReturnUrl ?? DashboardUrls.ResourcesUrl();
111+
return UrlValidationHelper.IsSafeRedirectUrl(ReturnUrl) ? ReturnUrl : DashboardUrls.ResourcesUrl();
112112
}
113113

114114
public async ValueTask DisposeAsync()

src/Aspire.Dashboard/Model/ValidateTokenMiddleware.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ public async Task InvokeAsync(HttpContext context)
6868

6969
private static void RedirectAfterValidation(HttpContext context)
7070
{
71-
if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl))
71+
// An optional returnurl value must be a valid URL and it must be local.
72+
// We don't want an open redirect (the potential to create a URL that redirects to a third-party site).
73+
if (context.Request.Query.TryGetValue("returnUrl", out var returnUrl)
74+
&& UrlValidationHelper.IsSafeRedirectUrl(returnUrl.ToString()))
7275
{
7376
context.Response.Redirect(returnUrl.ToString());
7477
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Diagnostics.CodeAnalysis;
5+
6+
namespace Aspire.Dashboard.Utils;
7+
8+
/// <summary>
9+
/// Helpers for validating redirect URLs.
10+
/// </summary>
11+
internal static class UrlValidationHelper
12+
{
13+
/// <summary>
14+
/// Checks whether a URL is safe to use as a redirect target.
15+
/// The URL must be a valid URI and a local path (not absolute or protocol-relative).
16+
/// </summary>
17+
internal static bool IsSafeRedirectUrl([NotNullWhen(true)] string? url)
18+
{
19+
return Uri.TryCreate(url, UriKind.Relative, out _) && IsLocalUrl(url);
20+
}
21+
22+
// Copied from ASP.NET Core's IsLocalUrl implementation:
23+
// https://github.com/dotnet/aspnetcore/blob/7cbda0e023075490b4365a0754ca410ce6eff59a/src/Shared/ResultsHelpers/SharedUrlHelper.cs#L33
24+
internal static bool IsLocalUrl([NotNullWhen(true)] string? url)
25+
{
26+
if (string.IsNullOrEmpty(url))
27+
{
28+
return false;
29+
}
30+
31+
// Allows "/" or "/foo" but not "//" or "/\".
32+
if (url[0] == '/')
33+
{
34+
// url is exactly "/"
35+
if (url.Length == 1)
36+
{
37+
return true;
38+
}
39+
40+
// url doesn't start with "//" or "/\"
41+
if (url[1] != '/' && url[1] != '\\')
42+
{
43+
return !HasControlCharacter(url.AsSpan(1));
44+
}
45+
46+
return false;
47+
}
48+
49+
// Allows "~/" or "~/foo" but not "~//" or "~/\".
50+
if (url[0] == '~' && url.Length > 1 && url[1] == '/')
51+
{
52+
// url is exactly "~/"
53+
if (url.Length == 2)
54+
{
55+
return true;
56+
}
57+
58+
// url doesn't start with "~//" or "~/\"
59+
if (url[2] != '/' && url[2] != '\\')
60+
{
61+
return !HasControlCharacter(url.AsSpan(2));
62+
}
63+
64+
return false;
65+
}
66+
67+
return false;
68+
69+
static bool HasControlCharacter(ReadOnlySpan<char> readOnlySpan)
70+
{
71+
// URLs may not contain ASCII control characters.
72+
for (var i = 0; i < readOnlySpan.Length; i++)
73+
{
74+
if (char.IsControl(readOnlySpan[i]))
75+
{
76+
return true;
77+
}
78+
}
79+
80+
return false;
81+
}
82+
}
83+
}

tests/Aspire.Dashboard.Tests/Middleware/ValidateTokenMiddlewareTests.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,32 @@ public async Task ValidateToken_BrowserTokenAuth_RightToken_RedirectsToReturnUrl
6363
Assert.Equal("/test", response.Headers.Location?.OriginalString);
6464
}
6565

66+
[Theory]
67+
[InlineData("https://evil.com")]
68+
[InlineData("http://evil.com")]
69+
[InlineData("//evil.com")]
70+
[InlineData("//evil.com/path")]
71+
[InlineData("/\\evil.com")]
72+
public async Task ValidateToken_NotBrowserTokenAuth_AbsoluteReturnUrl_RedirectsToHomepage(string returnUrl)
73+
{
74+
using var host = await SetUpHostAsync(FrontendAuthMode.Unsecured, string.Empty).DefaultTimeout();
75+
var response = await host.GetTestClient().GetAsync($"/login?t=test&returnUrl={Uri.EscapeDataString(returnUrl)}").DefaultTimeout();
76+
Assert.Equal("/", response.Headers.Location?.OriginalString);
77+
}
78+
79+
[Theory]
80+
[InlineData("https://evil.com")]
81+
[InlineData("http://evil.com")]
82+
[InlineData("//evil.com")]
83+
[InlineData("//evil.com/path")]
84+
[InlineData("/\\evil.com")]
85+
public async Task ValidateToken_BrowserTokenAuth_RightToken_AbsoluteReturnUrl_RedirectsToHomepage(string returnUrl)
86+
{
87+
using var host = await SetUpHostAsync(FrontendAuthMode.BrowserToken, "token").DefaultTimeout();
88+
var response = await host.GetTestClient().GetAsync($"/login?t=token&returnUrl={Uri.EscapeDataString(returnUrl)}").DefaultTimeout();
89+
Assert.Equal("/", response.Headers.Location?.OriginalString);
90+
}
91+
6692
private static async Task<IHost> SetUpHostAsync(FrontendAuthMode authMode, string expectedToken)
6793
{
6894
return await new HostBuilder()

0 commit comments

Comments
 (0)