From f9ce7b0f31cc1428c54886370fbd6517dcb22d0e Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Mon, 1 Jun 2026 19:47:56 +0200 Subject: [PATCH] Collapse scheme-relative leading slashes in Rewrite middleware redirect/rewrite targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AddRedirect`, IIS URL Rewrite, and Apache mod_rewrite rules let an attacker-controlled or misconfigured back-reference produce a target starting with `//`, `///`, or `/\` (and longer runs). When `PathBase` is empty, the resulting `Location` header is scheme-relative and resolves off-origin — an open redirect. This change adds a small internal helper `UrlNormalizer.CollapseLeadingSlashes` that mirrors the rejection predicate in `SharedUrlHelper.IsLocalUrl` but coerces instead of returning a boolean: any leading run of `/` and `\` is collapsed to a single `/`. The helper is applied at the three sinks that emit `Location` or mutate `Request.Path`: * `RedirectRule.ApplyRule` (`AddRedirect` surface) * `UrlActions.RedirectAction.ApplyAction` (IIS / Apache redirect import) * `UrlActions.RewriteAction.ApplyAction` (defense in depth for `Request.Path`) Adds regression theories on all three surfaces covering `//`, `///`, `//////`, `/\` shapes, both as literal replacements and as back-reference-synthesized captures. No public API surface change; `PublicAPI.Shipped.txt` / `PublicAPI.Unshipped.txt` unchanged. --- src/Middleware/Rewrite/src/RedirectRule.cs | 1 + .../Rewrite/src/UrlActions/RedirectAction.cs | 3 +- .../Rewrite/src/UrlActions/RewriteAction.cs | 6 ++ src/Middleware/Rewrite/src/UrlNormalizer.cs | 25 +++++++++ .../ModRewriteMiddlewareTest.cs | 56 +++++++++++++++++++ .../test/IISUrlRewrite/MiddleWareTests.cs | 35 ++++++++++++ .../Rewrite/test/MiddlewareTests.cs | 31 ++++++++++ 7 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 src/Middleware/Rewrite/src/UrlNormalizer.cs diff --git a/src/Middleware/Rewrite/src/RedirectRule.cs b/src/Middleware/Rewrite/src/RedirectRule.cs index d934aa8dd66a..ef668234a12e 100644 --- a/src/Middleware/Rewrite/src/RedirectRule.cs +++ b/src/Middleware/Rewrite/src/RedirectRule.cs @@ -43,6 +43,7 @@ public void ApplyRule(RewriteContext context) if (initMatchResults.Success) { var newPath = initMatchResults.Result(Replacement); + newPath = UrlNormalizer.CollapseLeadingSlashes(newPath); var response = context.HttpContext.Response; response.StatusCode = StatusCode; diff --git a/src/Middleware/Rewrite/src/UrlActions/RedirectAction.cs b/src/Middleware/Rewrite/src/UrlActions/RedirectAction.cs index f2e34378551b..e258c255547e 100644 --- a/src/Middleware/Rewrite/src/UrlActions/RedirectAction.cs +++ b/src/Middleware/Rewrite/src/UrlActions/RedirectAction.cs @@ -43,6 +43,8 @@ public override void ApplyAction(RewriteContext context, BackReferenceCollection return; } + pattern = UrlNormalizer.CollapseLeadingSlashes(pattern); + if (!pattern.Contains(Uri.SchemeDelimiter, StringComparison.Ordinal) && pattern[0] != '/') { pattern = '/' + pattern; @@ -51,7 +53,6 @@ public override void ApplyAction(RewriteContext context, BackReferenceCollection // url can either contain the full url or the path and query // always add to location header. - // TODO check for false positives var split = pattern.IndexOf('?'); if (split >= 0 && QueryStringAppend) diff --git a/src/Middleware/Rewrite/src/UrlActions/RewriteAction.cs b/src/Middleware/Rewrite/src/UrlActions/RewriteAction.cs index 8fa0f5bbca86..f27fd9054129 100644 --- a/src/Middleware/Rewrite/src/UrlActions/RewriteAction.cs +++ b/src/Middleware/Rewrite/src/UrlActions/RewriteAction.cs @@ -58,6 +58,12 @@ public override void ApplyAction(RewriteContext context, BackReferenceCollection pattern = Uri.EscapeDataString(pattern); } + // Defense in depth: this rule does not emit a Location header itself, but the rewritten value is + // assigned to Request.Path below and a downstream component (or a later rule) may issue a redirect + // from it. PathString does not reject a leading '//' or '/\', so neutralize the scheme-relative + // shape here rather than auditing every downstream consumer. + pattern = UrlNormalizer.CollapseLeadingSlashes(pattern); + // TODO PERF, substrings, object creation, etc. if (pattern.Contains(Uri.SchemeDelimiter, StringComparison.Ordinal)) { diff --git a/src/Middleware/Rewrite/src/UrlNormalizer.cs b/src/Middleware/Rewrite/src/UrlNormalizer.cs new file mode 100644 index 000000000000..c04eb27692c5 --- /dev/null +++ b/src/Middleware/Rewrite/src/UrlNormalizer.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Rewrite; + +internal static class UrlNormalizer +{ + // Collapses a leading run of '/' and '\' to a single '/' so a redirect/rewrite target cannot resolve as a + // scheme-relative authority. Mirrors the rejection predicate in SharedUrlHelper.IsLocalUrl. + public static string CollapseLeadingSlashes(string url) + { + if (url.Length < 2 || url[0] != '/' || (url[1] != '/' && url[1] != '\\')) + { + return url; + } + + var i = 1; + while (i < url.Length && (url[i] == '/' || url[i] == '\\')) + { + i++; + } + + return i == url.Length ? "/" : string.Concat("/", url.AsSpan(i)); + } +} diff --git a/src/Middleware/Rewrite/test/ApacheModRewrite/ModRewriteMiddlewareTest.cs b/src/Middleware/Rewrite/test/ApacheModRewrite/ModRewriteMiddlewareTest.cs index 0b800f85819b..353cde3f3a8e 100644 --- a/src/Middleware/Rewrite/test/ApacheModRewrite/ModRewriteMiddlewareTest.cs +++ b/src/Middleware/Rewrite/test/ApacheModRewrite/ModRewriteMiddlewareTest.cs @@ -37,6 +37,62 @@ public async Task Invoke_RewritePathWhenMatching() Assert.Equal("/hello", response); } + [Theory] + [InlineData("/legacy//example.com")] + [InlineData("/legacy///example.com")] + [InlineData(@"/legacy/\example.com")] + public async Task Invoke_RedirectCollapsesSchemeRelativeBackReference(string requestPath) + { + var options = new RewriteOptions().AddApacheModRewrite(new StringReader("RewriteRule /legacy/(.*) /$1 [R=302,L]")); + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseRewriter(options); + app.Run(context => context.Response.WriteAsync(context.Response.Headers.Location)); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetAsync(requestPath); + + Assert.Equal("/example.com", response.Headers.Location.OriginalString); + } + + [Theory] + [InlineData("/legacy//example.com")] + [InlineData("/legacy///example.com")] + [InlineData(@"/legacy/\example.com")] + public async Task Invoke_RewriteCollapsesSchemeRelativeBackReference(string requestPath) + { + var options = new RewriteOptions().AddApacheModRewrite(new StringReader("RewriteRule /legacy/(.*) /$1")); + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseRewriter(options); + app.Run(context => context.Response.WriteAsync(context.Request.Path)); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetStringAsync(requestPath); + + Assert.Equal("/example.com", response); + } + [Fact] public async Task Invoke_RewritePathTerminatesOnFirstSuccessOfRule() { diff --git a/src/Middleware/Rewrite/test/IISUrlRewrite/MiddleWareTests.cs b/src/Middleware/Rewrite/test/IISUrlRewrite/MiddleWareTests.cs index 9de45043ff7e..6c83b25b860c 100644 --- a/src/Middleware/Rewrite/test/IISUrlRewrite/MiddleWareTests.cs +++ b/src/Middleware/Rewrite/test/IISUrlRewrite/MiddleWareTests.cs @@ -47,6 +47,41 @@ public async Task Invoke_RedirectPathToPathAndQuery() Assert.Equal("/article.aspx?id=10&title=hey", response.Headers.Location.OriginalString); } + [Theory] + [InlineData("legacy//example.com")] + [InlineData("legacy///example.com")] + [InlineData(@"legacy/\example.com")] + public async Task Invoke_RedirectCollapsesSchemeRelativeBackReference(string requestPath) + { + var options = new RewriteOptions().AddIISUrlRewrite(new StringReader(@" + + + + + + + ")); + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseRewriter(options); + app.Run(context => context.Response.WriteAsync(context.Response.Headers.Location)); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetAsync(requestPath); + + Assert.Equal("/example.com", response.Headers.Location.OriginalString); + } + [Fact] public async Task Invoke_RewritePathToPathAndQuery() { diff --git a/src/Middleware/Rewrite/test/MiddlewareTests.cs b/src/Middleware/Rewrite/test/MiddlewareTests.cs index b6078a1c5aef..ab38d85aaac6 100644 --- a/src/Middleware/Rewrite/test/MiddlewareTests.cs +++ b/src/Middleware/Rewrite/test/MiddlewareTests.cs @@ -174,6 +174,37 @@ public async Task CheckRedirectPath(string pattern, string replacement, string b Assert.Equal(expectedUrl, response.Headers.Location.OriginalString); } + [Theory] + [InlineData("(.*)", "//example.com", "anything")] + [InlineData("(.*)", "///example.com", "anything")] + [InlineData("(.*)", @"/\example.com", "anything")] + [InlineData("(.*)", "//////example.com", "anything")] + [InlineData("legacy/(.*)", "/$1", "legacy//example.com")] + [InlineData("legacy/(.*)", "/$1", "legacy///example.com")] + [InlineData("legacy/(.*)", "/$1", @"legacy/\example.com")] + public async Task CheckRedirect_CollapsesSchemeRelativeTarget(string pattern, string replacement, string requestUrl) + { + var options = new RewriteOptions().AddRedirect(pattern, replacement, statusCode: StatusCodes.Status302Found); + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseTestServer() + .Configure(app => + { + app.UseRewriter(options); + }); + }).Build(); + + await host.StartAsync(); + + var server = host.GetTestServer(); + + var response = await server.CreateClient().GetAsync(requestUrl); + + Assert.Equal("/example.com", response.Headers.Location.OriginalString); + } + [Fact] public async Task RewriteRulesCanComeFromConfigureOptions() {