mirror of
https://github.com/keycloak/keycloak.git
synced 2026-02-03 20:39:33 -05:00
Add a global filter which throws bad request if a query parameter value has a control character
Closes #41117 Signed-off-by: Johannes Knutsen <johannes@kodet.no>
This commit is contained in:
parent
f9cb8dfe3d
commit
973e9ad176
3 changed files with 65 additions and 0 deletions
|
|
@ -0,0 +1,52 @@
|
|||
package org.keycloak.services.filters;
|
||||
|
||||
import jakarta.annotation.Priority;
|
||||
import jakarta.ws.rs.BadRequestException;
|
||||
import jakarta.ws.rs.container.ContainerRequestContext;
|
||||
import jakarta.ws.rs.container.ContainerRequestFilter;
|
||||
import jakarta.ws.rs.container.PreMatching;
|
||||
import jakarta.ws.rs.ext.Provider;
|
||||
import org.jboss.logging.Logger;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
@Provider
|
||||
@PreMatching
|
||||
@Priority(10)
|
||||
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<String, List<String>> queryParams = requestContext.getUriInfo().getQueryParameters();
|
||||
|
||||
for (final List<String> queryParamValues : queryParams.values()) {
|
||||
for (final String queryParamValue : queryParamValues) {
|
||||
if (containsAnyNULCharacter(queryParamValue)) {
|
||||
LOGGER.debugf("Request with invalid query parameter value is blocked");
|
||||
throw new BadRequestException("Blocking invalid query parameter value");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Unsafe character values we can safely assume is a bad request:
|
||||
* NUL U+0000 Breaks encoding (esp. UTF-8)
|
||||
*
|
||||
* @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) {
|
||||
if (input == null) {
|
||||
return false;
|
||||
}
|
||||
return input.contains(NUL_CHARACTER);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -24,6 +24,7 @@ import org.keycloak.models.KeycloakSessionFactory;
|
|||
import org.keycloak.services.error.KcUnrecognizedPropertyExceptionHandler;
|
||||
import org.keycloak.services.error.KeycloakErrorHandler;
|
||||
import org.keycloak.services.error.KeycloakMismatchedInputExceptionHandler;
|
||||
import org.keycloak.services.filters.InvalidQueryParameterFilter;
|
||||
import org.keycloak.services.filters.KeycloakSecurityHeadersFilter;
|
||||
import org.keycloak.services.resources.KeycloakApplication;
|
||||
import org.keycloak.services.resources.LoadBalancerResource;
|
||||
|
|
@ -48,6 +49,7 @@ public class ResteasyKeycloakApplication extends KeycloakApplication {
|
|||
classes.add(AdminRoot.class);
|
||||
}
|
||||
classes.add(ThemeResource.class);
|
||||
classes.add(InvalidQueryParameterFilter.class);
|
||||
classes.add(KeycloakSecurityHeadersFilter.class);
|
||||
classes.add(KeycloakErrorHandler.class);
|
||||
classes.add(KcUnrecognizedPropertyExceptionHandler.class);
|
||||
|
|
|
|||
|
|
@ -161,6 +161,17 @@ public class AuthorizationCodeTest extends AbstractKeycloakTest {
|
|||
assertEquals("Invalid parameter: redirect_uri", errorPage.getError());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInvalidNULCharacterClientId() {
|
||||
ClientManager.realm(adminClient.realm("test")).clientId("test-app").addRedirectUris(oauth.getRedirectUri());
|
||||
|
||||
oauth.client("%00");
|
||||
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");
|
||||
|
|
|
|||
Loading…
Reference in a new issue