From c045765b45ee3054d2b9ef49731b4d884078d9f4 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 2 Mar 2026 18:29:25 +0100 Subject: [PATCH] Prevent ASCII control characters in query parameters Closes #46740 Signed-off-by: Alexander Schwartz --- .../filters/InvalidQueryParameterFilter.java | 32 +++++++++++++++---- .../oauth/AuthorizationCodeTest.java | 11 +++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/filters/InvalidQueryParameterFilter.java b/services/src/main/java/org/keycloak/services/filters/InvalidQueryParameterFilter.java index 8e142f86e41..42e73d3ffef 100644 --- a/services/src/main/java/org/keycloak/services/filters/InvalidQueryParameterFilter.java +++ b/services/src/main/java/org/keycloak/services/filters/InvalidQueryParameterFilter.java @@ -1,6 +1,8 @@ package org.keycloak.services.filters; import java.io.IOException; +import java.text.CharacterIterator; +import java.text.StringCharacterIterator; import java.util.List; import java.util.Map; @@ -20,15 +22,13 @@ public class InvalidQueryParameterFilter implements ContainerRequestFilter { private static final Logger LOGGER = Logger.getLogger(InvalidQueryParameterFilter.class); - private static final String NUL_CHARACTER = "\u0000"; - @Override public void filter(ContainerRequestContext requestContext) throws IOException { final Map> queryParams = requestContext.getUriInfo().getQueryParameters(); for (final List queryParamValues : queryParams.values()) { for (final String queryParamValue : queryParamValues) { - if (containsAnyNULCharacter(queryParamValue)) { + if (containsAnyASCIIControlCharacter(queryParamValue)) { LOGGER.debugf("Request with invalid query parameter value is blocked"); throw new BadRequestException("Blocking invalid query parameter value"); } @@ -39,15 +39,35 @@ public class InvalidQueryParameterFilter implements ContainerRequestFilter { /** * Unsafe character values we can safely assume is a bad request: * NUL U+0000 Breaks encoding (esp. UTF-8) + * ESC U+001B Can lead to strange behavior in ANSI terminals + * ... or any other character below 0x20 excluding TAB (09) or LF/CR (0A/0D). * * @param input the value to check if contains unsafe characters * @return true if the input contains at least one of the unsafe characters */ - private boolean containsAnyNULCharacter(String input) { + private boolean containsAnyASCIIControlCharacter(String input) { if (input == null) { return false; } - return input.contains(NUL_CHARACTER); + CharacterIterator it = new StringCharacterIterator(input); + while (true) { + char c = it.current(); + if (c == CharacterIterator.DONE) { + break; + } else { + if (c < 32) { + switch (c) { + case 0x09: // TAB + case 0x0A: // LF + case 0x0D: // CR + break; + default: + return true; + } + } + it.next(); + } + } + return false; } - } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java index 801a286191b..f75e73d4bb6 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java @@ -174,6 +174,17 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest { assertEquals("An internal server error has occurred", errorPage.getError()); } + @Test + public void testInvalidESCCharacterClientId() { + ClientManager.realm(adminClient.realm("test")).clientId("test-app").addRedirectUris(oauth.getRedirectUri()); + + oauth.client("%1B"); + oauth.openLoginForm(); + + assertTrue(errorPage.isCurrent()); + assertEquals("An internal server error has occurred", errorPage.getError()); + } + @Test public void authorizationRequestNoState() throws IOException { AuthorizationEndpointResponse response = oauth.doLogin("test-user@localhost", "password");