Joe/7.0.6 to main

src/EntityFramework.Storage/Extensions/StringsExtensions.cs

@@ -136,6 +136,8 @@ public static string CleanUrlPath(this string url)
     [DebuggerStepThrough]
     public static bool IsLocalUrl(this string url)
     {
+        // This implementation is a copy of a https://github.com/dotnet/aspnetcore/blob/3f1acb59718cadf111a0a796681e3d3509bb3381/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs#L315
+        // We originally copied that code to avoid a dependency, but we could potentially remove this entirely by switching to the Microsoft.NET.Sdk.Web sdk.
         if (string.IsNullOrEmpty(url))
         {
             return false;
@@ -153,7 +155,7 @@ public static bool IsLocalUrl(this string url)
             // url doesn't start with "//" or "/\"
             if (url[1] != '/' && url[1] != '\\')
             {
-                return true;
+                return !HasControlCharacter(url.AsSpan(1));
             }
 
             return false;
@@ -171,13 +173,27 @@ public static bool IsLocalUrl(this string url)
             // url doesn't start with "~//" or "~/\"
             if (url[2] != '/' && url[2] != '\\')
             {
-                return true;
+                return !HasControlCharacter(url.AsSpan(2));
             }
 
             return false;
         }
 
         return false;
+
+        static bool HasControlCharacter(ReadOnlySpan<char> 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;
+        }
     }
 
     [DebuggerStepThrough]

src/IdentityServer/Extensions/StringsExtensions.cs

@@ -139,6 +139,8 @@ public static string CleanUrlPath(this string url)
     [DebuggerStepThrough]
     public static bool IsLocalUrl(this string url)
     {
+        // This implementation is a copy of a https://github.com/dotnet/aspnetcore/blob/3f1acb59718cadf111a0a796681e3d3509bb3381/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs#L315
+        // We originally copied that code to avoid a dependency, but we could potentially remove this entirely by switching to the Microsoft.NET.Sdk.Web sdk.
         if (string.IsNullOrEmpty(url))
         {
             return false;
@@ -156,7 +158,7 @@ public static bool IsLocalUrl(this string url)
             // url doesn't start with "//" or "/\"
             if (url[1] != '/' && url[1] != '\\')
             {
-                return true;
+                return !HasControlCharacter(url.AsSpan(1));
             }
 
             return false;
@@ -174,13 +176,27 @@ public static bool IsLocalUrl(this string url)
             // url doesn't start with "~//" or "~/\"
             if (url[2] != '/' && url[2] != '\\')
             {
-                return true;
+                return !HasControlCharacter(url.AsSpan(2));
             }
 
             return false;
         }
 
         return false;
+
+        static bool HasControlCharacter(ReadOnlySpan<char> 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;
+        }
     }
 
     [DebuggerStepThrough]

test/EntityFramework.Storage.UnitTests/IsLocalUrlTests.cs

@@ -0,0 +1,62 @@
+using System.Collections.Generic;
+using FluentAssertions;
+using Xunit;
+using Duende.IdentityServer.EntityFramework.Extensions;
+
+namespace UnitTests.Validation;
+
+public class IsLocalUrlTests
+{
+    private const string queryParameters = "?client_id=mvc.code" +
+        "&redirect_uri=https%3A%2F%2Flocalhost%3A44302%2Fsignin-oidc" +
+        "&response_type=code" +
+        "&scope=openid%20profile%20email%20custom.profile%20resource1.scope1%20resource2.scope1%20offline_access" +
+        "&code_challenge=LcJN1shWmezC0J5EU7QOi7N_amBuvMDb6PcTY0sB2YY" +
+        "&code_challenge_method=S256" +
+        "&response_mode=form_post" +
+        "&nonce=nonce" +
+        "&state=state";
+
+    public static IEnumerable<object[]> TestCases =>
+        new List<object[]>
+        {
+            new object[] { "/connect/authorize/callback" + queryParameters, true },
+            new object[] { "//evil.com/" + queryParameters, false },
+            // Tab character
+            new object[] { "/\t/evil.com/connect/authorize/callback" + queryParameters, false },
+            // Tabs and Spaces
+            new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/  \t/evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/   \t/evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/\t  /evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/\t   /evil.com/connect/authorize/callback" + queryParameters, false },
+            // Various new line related things
+            new object[] { "/\n/evil.com/" + queryParameters, false },
+            new object[] { "/\n\n/evil.com/" + queryParameters, false },
+            new object[] { "/\r/evil.com/" + queryParameters, false },
+            new object[] { "/\r\r/evil.com/" + queryParameters, false },
+            new object[] { "/\r\n/evil.com/" + queryParameters, false },
+            new object[] { "/\r\n\r\n/evil.com/" + queryParameters, false },
+            // Tabs and Newlines
+            new object[] { "/\t\n/evil.com/" + queryParameters, false },
+            new object[] { "/\t\n\n/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r\r/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r\n/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r\n\r\n/evil.com/" + queryParameters, false },
+            new object[] { "/\n/evil.com\t/" + queryParameters, false },
+            new object[] { "/\n\n/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r\r/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r\n/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r\n\r\n/evil.com\t/" + queryParameters, false },
+        };
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void IsLocalUrl(string returnUrl, bool expected)
+    {
+        returnUrl.IsLocalUrl().Should().Be(expected);
+    }
+}

test/IdentityServer.UnitTests/Validation/IsLocalUrlTests.cs

@@ -0,0 +1,179 @@
+using System.Collections.Generic;
+using Duende.IdentityServer.Configuration;
+using Duende.IdentityServer.Extensions;
+using Duende.IdentityServer.Services;
+using Duende.IdentityServer.Validation;
+using FluentAssertions;
+using Microsoft.Extensions.Logging;
+using UnitTests.Common;
+using UnitTests.Endpoints.Authorize;
+using Xunit;
+
+namespace UnitTests.Validation;
+
+public class IsLocalUrlTests
+{
+    private const string queryParameters = "?client_id=mvc.code" +
+        "&redirect_uri=https%3A%2F%2Flocalhost%3A44302%2Fsignin-oidc" +
+        "&response_type=code" +
+        "&scope=openid%20profile%20email%20custom.profile%20resource1.scope1%20resource2.scope1%20offline_access" +
+        "&code_challenge=LcJN1shWmezC0J5EU7QOi7N_amBuvMDb6PcTY0sB2YY" +
+        "&code_challenge_method=S256" +
+        "&response_mode=form_post" +
+        "&nonce=nonce" +
+        "&state=state";
+
+    public static IEnumerable<object[]> TestCases =>
+        new List<object[]>
+        {
+            new object[] { "/connect/authorize/callback" + queryParameters, true },
+            new object[] { "//evil.com/" + queryParameters, false },
+            // Tab character
+            new object[] { "/\t/evil.com/connect/authorize/callback" + queryParameters, false },
+            // Tabs and Spaces
+            new object[] { "/ \t/evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/  \t/evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/   \t/evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/\t /evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/\t  /evil.com/connect/authorize/callback" + queryParameters, false },
+            new object[] { "/\t   /evil.com/connect/authorize/callback" + queryParameters, false },
+            // Various new line related things
+            new object[] { "/\n/evil.com/" + queryParameters, false },
+            new object[] { "/\n\n/evil.com/" + queryParameters, false },
+            new object[] { "/\r/evil.com/" + queryParameters, false },
+            new object[] { "/\r\r/evil.com/" + queryParameters, false },
+            new object[] { "/\r\n/evil.com/" + queryParameters, false },
+            new object[] { "/\r\n\r\n/evil.com/" + queryParameters, false },
+            // Tabs and Newlines
+            new object[] { "/\t\n/evil.com/" + queryParameters, false },
+            new object[] { "/\t\n\n/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r\r/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r\n/evil.com/" + queryParameters, false },
+            new object[] { "/\t\r\n\r\n/evil.com/" + queryParameters, false },
+            new object[] { "/\n/evil.com\t/" + queryParameters, false },
+            new object[] { "/\n\n/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r\r/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r\n/evil.com\t/" + queryParameters, false },
+            new object[] { "/\r\n\r\n/evil.com\t/" + queryParameters, false },
+        };
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public async void GetAuthorizationContextAsync(string returnUrl, bool expected)
+    {
+        var interactionService = new DefaultIdentityServerInteractionService(null, null, null, null, null, null, null, 
+            GetReturnUrlParser(), new LoggerFactory().CreateLogger<DefaultIdentityServerInteractionService>());
+        var actual = await interactionService.GetAuthorizationContextAsync(returnUrl);
+        if (expected)
+        {
+            actual.Should().NotBeNull();
+        }
+        else
+        {
+            actual.Should().BeNull();
+        }
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void IsLocalUrl(string returnUrl, bool expected)
+    {
+        returnUrl.IsLocalUrl().Should().Be(expected);
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void GetIdentityServerRelativeUrl(string returnUrl, bool expected)
+    {
+        var serverUrls = new MockServerUrls
+        {
+            Origin = "https://localhost:5001",
+            BasePath = "/"
+        };
+        var actual = serverUrls.GetIdentityServerRelativeUrl(returnUrl);
+        if (expected)
+        {
+            actual.Should().NotBeNull();
+        }
+        else
+        {
+            actual.Should().BeNull();
+        }
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public async void OidcReturnUrlParser_ParseAsync(string returnUrl, bool expected)
+    {
+        var oidcParser = GetOidcReturnUrlParser();
+        var actual = await oidcParser.ParseAsync(returnUrl);
+        if (expected)
+        {
+            actual.Should().NotBeNull();
+        }
+        else
+        {
+            actual.Should().BeNull();
+        }
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void OidcReturnUrlParser_IsValidReturnUrl(string returnUrl, bool expected)
+    {
+        var oidcParser = GetOidcReturnUrlParser();
+        oidcParser.IsValidReturnUrl(returnUrl).Should().Be(expected);
+    }
+
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public void ReturnUrlParser_IsValidReturnUrl(string returnUrl, bool expected)
+    {
+        var parser = GetReturnUrlParser();
+        parser.IsValidReturnUrl(returnUrl).Should().Be(expected);
+    }
+
+    [Theory]
+    [MemberData(nameof(TestCases))]
+    public async void ReturnUrlParser_ParseAsync(string returnUrl, bool expected)
+    {
+        var parser = GetReturnUrlParser();
+        var actual = await parser.ParseAsync(returnUrl);
+        if (expected)
+        {
+            actual.Should().NotBeNull();
+        }
+        else
+        {
+            actual.Should().BeNull();
+        }
+    }
+
+    private static ReturnUrlParser GetReturnUrlParser()
+    {
+        var oidcParser = GetOidcReturnUrlParser();
+        var parser = new ReturnUrlParser(new IReturnUrlParser[] { oidcParser });
+        return parser;
+    }
+
+    private static OidcReturnUrlParser GetOidcReturnUrlParser()
+    {
+        return new OidcReturnUrlParser(
+            new IdentityServerOptions(),
+            new StubAuthorizeRequestValidator
+            {
+                Result = new AuthorizeRequestValidationResult
+                (
+                    new ValidatedAuthorizeRequest()
+                )
+            },
+            new MockUserSession(),
+            new MockServerUrls(),
+            new LoggerFactory().CreateLogger<OidcReturnUrlParser>());
+    }
+
+
+}