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
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public IActionResult SignIn(
{
scheme ??= OpenIdConnectDefaults.AuthenticationScheme;
string redirect;
if (!string.IsNullOrEmpty(redirectUri) && Url.IsLocalUrl(redirectUri))
if (!string.IsNullOrEmpty(redirectUri) && Url.IsLocalUrl(redirectUri) && !IsPercentEncodedSlashBypass(redirectUri))
{
redirect = redirectUri;
}
Expand Down Expand Up @@ -254,6 +254,12 @@ private static bool IsSameOrigin(Uri absolute, HttpRequest request)
/// closes. Comparison is case-insensitive because the RFC 3986 encoding is
/// hex-case-insensitive.
/// </summary>
/// <remarks>
/// Canonical implementation: <c>RedirectUriHelper.HasPercentEncodedSlashPrefix</c> in
/// <c>Microsoft.Identity.Web</c>. This private copy exists because <c>RedirectUriHelper</c>
/// is internal and <c>Microsoft.Identity.Web.UI</c> is a separate assembly. Keep both
/// in sync.
/// </remarks>
private static bool IsPercentEncodedSlashBypass(string path) =>
path.StartsWith("/%2f", StringComparison.OrdinalIgnoreCase)
|| path.StartsWith("/%5c", StringComparison.OrdinalIgnoreCase);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,5 +439,51 @@ public void Challenge_WithSameOriginAbsoluteButSlashBackslashPathAndQuery_UsesDe
var challengeResult = Assert.IsType<ChallengeResult>(result);
Assert.Equal("/", challengeResult.Properties!.RedirectUri);
}

// -----------------------------------------------------------------------------
// SignIn action — percent-encoded slash bypass.
//
// A redirectUri of "/%2fevil.example" (the value the controller sees after ASP.NET
// Core has query-decoded the request once — the raw query string would be "%252f")
// passes Url.IsLocalUrl because the framework checks for literal "//" and "/\" only.
// A normalising reverse proxy (NGINX, IIS ARR, F5) that decodes percent-encoded
// path segments on the way out turns that into "//evil.example", which browsers
// treat as a protocol-relative URL. The IsPercentEncodedSlashBypass guard must
// block this on the SignIn path, matching the guard already on the Challenge action.
// -----------------------------------------------------------------------------

[Fact]
public void SignIn_WithPercentEncodedSlashRedirectUri_UsesDefaultRedirectUri()
{
// Arrange: use a realistic IsLocalUrl mock so that removing either the
// IsLocalUrl check or the IsPercentEncodedSlashBypass check would cause this
// test to fail.
UseRealisticIsLocalUrl();
string scheme = OpenIdConnectDefaults.AuthenticationScheme;

// "/%2fevil.example" — passes IsLocalUrl (starts with "/" and second char is "%"),
// but must be rejected by IsPercentEncodedSlashBypass.
// Act
var result = _accountController.SignIn(scheme, "/%2fevil.example");

// Assert: redirect must fall back to default, not to the attacker-controlled URI.
var challengeResult = Assert.IsType<ChallengeResult>(result);
Assert.Equal("/", challengeResult.Properties!.RedirectUri);
}

[Fact]
public void SignIn_WithPercentEncodedBackslashRedirectUri_UsesDefaultRedirectUri()
{
// Arrange: "/%5cevil.example" — backslash variant of the same bypass.
UseRealisticIsLocalUrl();
string scheme = OpenIdConnectDefaults.AuthenticationScheme;

// Act
var result = _accountController.SignIn(scheme, "/%5cevil.example");

// Assert
var challengeResult = Assert.IsType<ChallengeResult>(result);
Assert.Equal("/", challengeResult.Properties!.RedirectUri);
}
}
}
Loading