From 082b0ed3084963cc3096e2db3cc2fc67b6c0b950 Mon Sep 17 00:00:00 2001
From: rmartinc <rmartinc@redhat.com>
Date: Tue, 19 Sep 2023 12:09:50 +0200
Subject: [PATCH] verifyRedirectUri should return null when the passed
 redirectUri is invalid Closes
 https://github.com/keycloak/keycloak/issues/22778

---
 .../protocol/oidc/utils/RedirectUtils.java        |  9 +++++++--
 .../protocol/oidc/utils/RedirectUtilsTest.java    | 15 +++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java
index 7c9c4755e7..b1bc4e3aa8 100644
--- a/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java
+++ b/services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java
@@ -107,6 +107,12 @@ public class RedirectUtils {
             logger.debug("No Redirect URIs supplied");
             redirectUri = null;
         } else {
+            URI originalRedirect = toUri(redirectUri);
+            if (originalRedirect == null) {
+                // invalid URI passed as redirectUri
+                return null;
+            }
+
             // Make the validations against fully decoded and normalized redirect-url. This also allows wildcards (case when client configured "Valid redirect-urls" contain wildcards)
             String decodedRedirectUri = decodeRedirectUri(redirectUri);
             URI decodedRedirect = toUri(decodedRedirectUri);
@@ -135,8 +141,7 @@ public class RedirectUtils {
             }
 
             // Return the original redirectUri, which can be partially encoded - for example http://localhost:8280/foo/bar%20bar%2092%2F72/3 . Just make sure it is normalized
-            URI redirect = toUri(redirectUri);
-            redirectUri = getNormalizedRedirectUri(redirect);
+            redirectUri = getNormalizedRedirectUri(originalRedirect);
 
             // We try to check validity also for original (encoded) redirectUrl, but just in case it exactly matches some "Valid Redirect URL" specified for client (not wildcards allowed)
             if (valid == null) {
diff --git a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java
index 1d953cd897..4a5cd883c6 100644
--- a/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java
+++ b/services/src/test/java/org/keycloak/protocol/oidc/utils/RedirectUtilsTest.java
@@ -127,4 +127,19 @@ public class RedirectUtilsTest {
 
         Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/%7Babc%7D", set, false));
     }
+
+    @Test
+    public void testverifyInvalidRedirectUri() {
+        Set<String> set = Stream.of(
+                "https://keycloak.org/*"
+        ).collect(Collectors.toSet());
+
+        Assert.assertEquals("https://keycloak.org/path%20space/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path%20space/", set, false));
+        Assert.assertEquals("https://keycloak.org/path%3Cless/", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path%3Cless/", set, false));
+        Assert.assertEquals("https://keycloak.org/path/index.jsp?param=v1+v2", RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path/index.jsp?param=v1+v2", set, false));
+
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path space/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path<less/", set, false));
+        Assert.assertNull(RedirectUtils.verifyRedirectUri(session, null, "https://keycloak.org/path/index.jsp?param=v1 v2", set, false));
+    }
 }