[KEYCLOAK-18535] KeycloakSanitizerMethod causes java.lang.IndexOutOfBoundsException when there is more then one href in a sanitized message

This commit is contained in:
rmartinc 2021-08-06 14:37:38 +02:00 committed by Marek Posolda
parent 9e9e716369
commit 5ff6ff57a8
2 changed files with 38 additions and 13 deletions

View file

@ -34,6 +34,8 @@ import org.owasp.html.PolicyFactory;
public class KeycloakSanitizerMethod implements TemplateMethodModelEx { public class KeycloakSanitizerMethod implements TemplateMethodModelEx {
private static final PolicyFactory KEYCLOAK_POLICY = KeycloakSanitizerPolicy.POLICY_DEFINITION; private static final PolicyFactory KEYCLOAK_POLICY = KeycloakSanitizerPolicy.POLICY_DEFINITION;
private static final Pattern HREF_PATTERN = Pattern.compile("\\s+href=\"([^\"]*)\"");
@Override @Override
public Object exec(List list) throws TemplateModelException { public Object exec(List list) throws TemplateModelException {
@ -48,16 +50,19 @@ public class KeycloakSanitizerMethod implements TemplateMethodModelEx {
} }
private String fixURLs(String msg) { private String fixURLs(String msg) {
Pattern hrefs = Pattern.compile("href=\"([^\"]*)\""); Matcher matcher = HREF_PATTERN.matcher(msg);
Matcher matcher = hrefs.matcher(msg); if (matcher.find()) {
int count = 0; int last = 0;
while(matcher.find()) { StringBuilder result = new StringBuilder(msg.length());
count++; do {
String original = matcher.group(count); String href = matcher.group(1).replaceAll("=", "=")
String href = original.replaceAll("=", "=") .replaceAll("\\.\\.", ".")
.replaceAll("\\.\\.", ".") .replaceAll("&", "&");
.replaceAll("&", "&"); result.append(msg.substring(last, matcher.start(1))).append(href);
msg = msg.replace(original, href); last = matcher.end(1);
} while (matcher.find());
result.append(msg.substring(last));
return result.toString();
} }
return msg; return msg;
} }

View file

@ -30,11 +30,11 @@ import static org.junit.Assert.fail;
* @author Stan Silvert * @author Stan Silvert
*/ */
public class KeycloakSanitizerTest { public class KeycloakSanitizerTest {
private KeycloakSanitizerMethod kcEscape = new KeycloakSanitizerMethod(); private final KeycloakSanitizerMethod kcEscape = new KeycloakSanitizerMethod();
@Test @Test
public void testEscapes() throws Exception { 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>"); 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>"; String expectedResult = "<div class=\"kc-logo-text\"><span>Keycloak</span></div>";
@ -59,7 +59,27 @@ public class KeycloakSanitizerTest {
expectedResult = ""; expectedResult = "";
assertResult(expectedResult, html); 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 { private void assertResult(String expectedResult, List<String> html) throws Exception {
String result = kcEscape.exec(html).toString(); String result = kcEscape.exec(html).toString();
assertEquals(expectedResult, result); assertEquals(expectedResult, result);