From 1daf1db7c32501fc3547131d42f7659de678d708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mehmet=20Can=20Karag=C3=B6z?= Date: Tue, 10 Nov 2026 14:05:39 +0300 Subject: [PATCH] Fix open redirect validation in authentication flows --- .../Redirect/AuthRedirectResolver.cs | 8 ++- .../Redirect/ReturnUrlParser.cs | 17 ++++-- .../Server/RedirectTests.cs | 53 +++++++++++++++++++ .../Server/ReturnUrlParserTests.cs | 20 +++++-- 4 files changed, 85 insertions(+), 13 deletions(-) diff --git a/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/AuthRedirectResolver.cs b/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/AuthRedirectResolver.cs index afddbead..efbaa8f7 100644 --- a/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/AuthRedirectResolver.cs +++ b/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/AuthRedirectResolver.cs @@ -97,14 +97,12 @@ private RedirectDecision Resolve(AuthFlowContext flow, HttpContext ctx, string? private static void ValidateAllowed(string baseAddress, UAuthServerOptions options) { - if (options.Hub.AllowedClientOrigins.Count == 0) - return; - var normalized = OriginHelper.Normalize(baseAddress); - if (!options.Hub.AllowedClientOrigins.Any(o => OriginHelper.Normalize(o) == normalized)) + if (!options.Hub.AllowedClientOrigins.Any(origin => OriginHelper.Normalize(origin) == normalized)) { - throw new InvalidOperationException($"Redirect to '{baseAddress}' is not allowed."); + throw new InvalidOperationException($"Redirect to '{baseAddress}' is not allowed. " + + "The origin is not present in AllowedClientOrigins."); } } } diff --git a/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/ReturnUrlParser.cs b/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/ReturnUrlParser.cs index 2126605d..b480e53f 100644 --- a/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/ReturnUrlParser.cs +++ b/src/CodeBeam.UltimateAuth.Server/Infrastructure/Redirect/ReturnUrlParser.cs @@ -11,6 +11,14 @@ public static ReturnUrlInfo Parse(string? returnUrl) returnUrl = returnUrl.Trim(); + // Reject ambiguous paths, backslashes and control characters. + if (returnUrl.StartsWith("//", StringComparison.Ordinal) || + returnUrl.Contains('\\') || + returnUrl.Any(char.IsControl)) + { + throw new UAuthValidationException("return_url_invalid"); + } + if (returnUrl.StartsWith("/", StringComparison.Ordinal) || returnUrl.StartsWith("./", StringComparison.Ordinal) || returnUrl.StartsWith("../", StringComparison.Ordinal)) @@ -18,14 +26,13 @@ public static ReturnUrlInfo Parse(string? returnUrl) return ReturnUrlInfo.Relative(returnUrl); } - if (Uri.TryCreate(returnUrl, UriKind.Absolute, out var abs) && (abs.Scheme == Uri.UriSchemeHttp || abs.Scheme == Uri.UriSchemeHttps)) + if (Uri.TryCreate(returnUrl, UriKind.Absolute, out var absolute) && + (absolute.Scheme == Uri.UriSchemeHttp || + absolute.Scheme == Uri.UriSchemeHttps)) { - return ReturnUrlInfo.Absolute(abs); + return ReturnUrlInfo.Absolute(absolute); } - if (returnUrl.StartsWith("//", StringComparison.Ordinal)) - throw new UAuthValidationException("return_url_invalid"); - throw new UAuthValidationException("return_url_invalid"); } } diff --git a/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/RedirectTests.cs b/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/RedirectTests.cs index 4e8a1896..261db9ca 100644 --- a/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/RedirectTests.cs +++ b/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/RedirectTests.cs @@ -86,6 +86,7 @@ public void Absolute_ReturnUrl_Is_Used_When_Override_Allowed() var resolver = TestRedirectResolver.Create(); var ctx = TestHttpContext.Create(); + flow.Options.Hub.AllowedClientOrigins.Add("https://app.example.com"); var decision = resolver.ResolveSuccess(flow, ctx); decision.Enabled.Should().BeTrue(); decision.TargetUrl.Should().Be("https://app.example.com/dashboard"); @@ -226,4 +227,56 @@ public void Failure_Redirect_Contains_Payload_With_Mapped_Error_Code() payload.Status.Should().Be("failed"); payload.Reason.Should().Be(AuthFailureReason.InvalidCredentials); } + + [Fact] + public void Absolute_ReturnUrl_Is_Rejected_When_AllowedOrigins_Is_Empty() + { + var flow = AuthFlowTestFactory.LoginSuccess( + returnUrlInfo: ReturnUrlParser.Parse( + "https://evil.example/phishing"), + redirect: new EffectiveRedirectResponse( + enabled: true, + successPath: "/welcome", + failurePath: null, + failureCodes: null, + allowReturnUrlOverride: true, + includeLockoutTiming: true, + includeRemainingAttempts: false)); + + flow.Options.Hub.AllowedClientOrigins.Clear(); + + var resolver = TestRedirectResolver.Create(); + var ctx = TestHttpContext.Create(); + + Action act = () => resolver.ResolveSuccess(flow, ctx); + + act.Should() + .Throw() + .WithMessage("*not allowed*"); + } + + [Fact] + public void Relative_ReturnUrl_Works_Without_AllowedOrigins() + { + var flow = AuthFlowTestFactory.LoginSuccess( + returnUrlInfo: ReturnUrlParser.Parse("/dashboard"), + redirect: new EffectiveRedirectResponse( + enabled: true, + successPath: "/welcome", + failurePath: null, + failureCodes: null, + allowReturnUrlOverride: true, + includeLockoutTiming: true, + includeRemainingAttempts: false)); + + flow.Options.Hub.AllowedClientOrigins.Clear(); + + var resolver = TestRedirectResolver.Create(); + var ctx = TestHttpContext.Create(); + + var decision = resolver.ResolveSuccess(flow, ctx); + + decision.Enabled.Should().BeTrue(); + decision.TargetUrl.Should().Be("https://app.example.com/dashboard"); + } } diff --git a/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/ReturnUrlParserTests.cs b/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/ReturnUrlParserTests.cs index 25197a38..7ae9e255 100644 --- a/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/ReturnUrlParserTests.cs +++ b/tests/CodeBeam.UltimateAuth.Tests.Unit/Server/ReturnUrlParserTests.cs @@ -1,8 +1,5 @@ using CodeBeam.UltimateAuth.Core.Errors; -using CodeBeam.UltimateAuth.Server.Auth; using CodeBeam.UltimateAuth.Server.Infrastructure; -using CodeBeam.UltimateAuth.Server.Options; -using CodeBeam.UltimateAuth.Tests.Unit.Helpers; using AwesomeAssertions; namespace CodeBeam.UltimateAuth.Tests.Unit; @@ -43,4 +40,21 @@ public void Parser_Rejects_Unsafe_Schemes(string returnUrl) Action act = () => ReturnUrlParser.Parse(returnUrl); act.Should().Throw().Which.Code.Should().Be("return_url_invalid"); } + + + [Theory] + [InlineData("//evil.example")] + [InlineData("///evil.example")] + [InlineData("/\\evil.example")] + [InlineData("\\\\evil.example")] + [InlineData("/dashboard\\evil")] + [InlineData("/dashboard\r\nLocation: https://evil.example")] + public void Parser_Rejects_Ambiguous_Or_Unsafe_Paths(string returnUrl) + { + Action act = () => ReturnUrlParser.Parse(returnUrl); + + act.Should() + .Throw() + .Which.Code.Should().Be("return_url_invalid"); + } }