Better handling of incorrect roleName in KeycloakModelUtils.getRoleFromString

closes #38579

Signed-off-by: mposolda <mposolda@gmail.com>

server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java

@@ -111,6 +111,8 @@ public final class KeycloakModelUtils {
     public static final String GROUP_PATH_ESCAPE = "~";
     private static final char CLIENT_ROLE_SEPARATOR = '.';
 
+    public static final int MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE = 25;
+
     private KeycloakModelUtils() {
     }
 
@@ -937,8 +939,10 @@ public final class KeycloakModelUtils {
         }
 
         // Check client roles for all possible splits by dot
+        int counter = 0;
         int scopeIndex = roleName.lastIndexOf(CLIENT_ROLE_SEPARATOR);
-        while (scopeIndex >= 0) {
+        while (scopeIndex >= 0 && counter < MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE) {
+            counter++;
             String appName = roleName.substring(0, scopeIndex);
             ClientModel client = realm.getClientByClientId(appName);
             if (client != null) {
@@ -948,6 +952,10 @@ public final class KeycloakModelUtils {
 
             scopeIndex = roleName.lastIndexOf(CLIENT_ROLE_SEPARATOR, scopeIndex - 1);
         }
+        if (counter >= MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE) {
+            logger.warnf("Not able to retrieve role model from the role name '%s'. Please use shorter role names with the limited amount of dots, roleName", roleName.length() > 100 ? roleName.substring(0, 100) + "..." : roleName);
+            return null;
+        }
 
         // determine if roleName is a realm role
         return realm.getRole(roleName);

server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java

@@ -25,9 +25,13 @@ import static org.junit.Assert.assertNull;
 
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Stream;
+
+import org.junit.Assert;
 import org.junit.Test;
 import org.keycloak.models.utils.KeycloakModelUtils;
+import org.keycloak.models.utils.RealmModelDelegate;
 
 /**
  * @author <a href="mailto:daniel.fesenmeyer@bosch.io">Daniel Fesenmeyer</a>
@@ -67,6 +71,30 @@ public class KeycloakModelUtilsTest {
         assertParsedRoleQualifier(clientIdAndRoleName, "my.client.id", "role-name");
     }
 
+    // Tests that count of client lookups during KeycloakModelUtils.getRoleFromString is limited (to prevent issues like DoS or OOM in case that incorrect configuration of the role mapper was provided)
+    @Test
+    public void testLimitCountOfClientLookupsDuringGetRoleFromString() {
+        AtomicInteger counter = new AtomicInteger(0);
+
+        RealmModel realm = new RealmModelDelegate(null) {
+
+            @Override
+            public ClientModel getClientByClientId(String clientId) {
+                counter.incrementAndGet();
+                return null;
+            }
+        };
+
+        String badRoleName = ".";
+        for (int i = 0 ; i < 16 ; i++) {
+            badRoleName = badRoleName + badRoleName;
+        }
+        Assert.assertEquals(65536, badRoleName.length());
+
+        Assert.assertNull(KeycloakModelUtils.getRoleFromString(realm, badRoleName));
+        Assert.assertEquals(KeycloakModelUtils.MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE, counter.get());
+    }
+
     @Test
     public void testSplitEscapedPath() {
         assertArrayEquals(new String[]{"parent", "child"}, KeycloakModelUtils.splitPath("/parent/child", true));