From 5ff6ff57a82015b56d06fa6de9077fd48f71bb38 Mon Sep 17 00:00:00 2001
From: rmartinc <rmartinc@redhat.com>
Date: Fri, 6 Aug 2021 14:37:38 +0200
Subject: [PATCH] [KEYCLOAK-18535] KeycloakSanitizerMethod causes
 java.lang.IndexOutOfBoundsException when there is more then one href in a
 sanitized message

---
 .../theme/KeycloakSanitizerMethod.java        | 25 +++++++++++-------
 .../keycloak/theme/KeycloakSanitizerTest.java | 26 ++++++++++++++++---
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/services/src/main/java/org/keycloak/theme/KeycloakSanitizerMethod.java b/services/src/main/java/org/keycloak/theme/KeycloakSanitizerMethod.java
index b05ef66e06..bf2d6068d6 100644
--- a/services/src/main/java/org/keycloak/theme/KeycloakSanitizerMethod.java
+++ b/services/src/main/java/org/keycloak/theme/KeycloakSanitizerMethod.java
@@ -34,6 +34,8 @@ import org.owasp.html.PolicyFactory;
 public class KeycloakSanitizerMethod implements TemplateMethodModelEx {
     
     private static final PolicyFactory KEYCLOAK_POLICY = KeycloakSanitizerPolicy.POLICY_DEFINITION;
+
+    private static final Pattern HREF_PATTERN = Pattern.compile("\\s+href=\"([^\"]*)\"");
     
     @Override
     public Object exec(List list) throws TemplateModelException {
@@ -48,16 +50,19 @@ public class KeycloakSanitizerMethod implements TemplateMethodModelEx {
     }
 
     private String fixURLs(String msg) {
-        Pattern hrefs = Pattern.compile("href=\"([^\"]*)\"");
-        Matcher matcher = hrefs.matcher(msg);
-        int count = 0;
-        while(matcher.find()) {
-            count++;
-            String original = matcher.group(count);
-            String href = original.replaceAll("&#61;", "=")
-                    .replaceAll("\\.\\.", ".")
-                    .replaceAll("&amp;", "&");
-            msg = msg.replace(original, href);
+        Matcher matcher = HREF_PATTERN.matcher(msg);
+        if (matcher.find()) {
+            int last = 0;
+            StringBuilder result = new StringBuilder(msg.length());
+            do {
+                String href = matcher.group(1).replaceAll("&#61;", "=")
+                        .replaceAll("\\.\\.", ".")
+                        .replaceAll("&amp;", "&");
+                result.append(msg.substring(last, matcher.start(1))).append(href);
+                last = matcher.end(1);
+            } while (matcher.find());
+            result.append(msg.substring(last));
+            return result.toString();
         }
         return msg;
     }
diff --git a/services/src/test/java/org/keycloak/theme/KeycloakSanitizerTest.java b/services/src/test/java/org/keycloak/theme/KeycloakSanitizerTest.java
index 9e503f5b4a..878995610c 100644
--- a/services/src/test/java/org/keycloak/theme/KeycloakSanitizerTest.java
+++ b/services/src/test/java/org/keycloak/theme/KeycloakSanitizerTest.java
@@ -30,11 +30,11 @@ import static org.junit.Assert.fail;
  * @author Stan Silvert
  */
 public class KeycloakSanitizerTest {
-    private KeycloakSanitizerMethod kcEscape = new KeycloakSanitizerMethod();
+    private final KeycloakSanitizerMethod kcEscape = new KeycloakSanitizerMethod();
     
     @Test
     public void testEscapes() throws Exception {
-        List<String> html = new ArrayList();
+        List<String> html = new ArrayList<>();
 
         html.add("<div class=\"kc-logo-text\"><script>alert('foo');</script><span>Keycloak</span></div>");
         String expectedResult = "<div class=\"kc-logo-text\"><span>Keycloak</span></div>";
@@ -59,7 +59,27 @@ public class KeycloakSanitizerTest {
         expectedResult = "";
         assertResult(expectedResult, html);
     }
-    
+
+    @Test
+    public void testUrls() throws Exception {
+        List<String> html = new ArrayList<>();
+
+        html.add("<p><a href='https://localhost'>link</a></p>");
+        assertResult("<p><a href=\"https://localhost\" rel=\"nofollow\">link</a></p>", html);
+
+        html.set(0, "<p><a href=\"\">link</a></p>");
+        assertResult("<p>link</p>", html);
+
+        html.set(0, "<p><a href=\"javascript:alert('hello!');\">link</a></p>");
+        assertResult("<p>link</p>", html);
+
+        html.set(0, "<p><a href=\"https://localhost?key=123&msg=abc\">link</a></p>");
+        assertResult("<p><a href=\"https://localhost?key=123&msg=abc\" rel=\"nofollow\">link</a></p>", html);
+
+        html.set(0, "<p><a href='https://localhost?key=123&msg=abc'>link1</a><a href=\"https://localhost?key=abc&msg=123\">link2</a></p>");
+        assertResult("<p><a href=\"https://localhost?key=123&msg=abc\" rel=\"nofollow\">link1</a><a href=\"https://localhost?key=abc&msg=123\" rel=\"nofollow\">link2</a></p>", html);
+    }
+
     private void assertResult(String expectedResult, List<String> html) throws Exception {
         String result = kcEscape.exec(html).toString();
         assertEquals(expectedResult, result);