From dc32a6937bbb18b986f5487b45201d0a30a383ab Mon Sep 17 00:00:00 2001 From: Carsten Ziegeler Date: Thu, 16 Jul 2026 12:38:47 +0200 Subject: [PATCH] fix(auth): prevent open redirect and log injection in feedback handlers Co-authored-by: Maia --- .../DefaultAuthenticationFeedbackHandler.java | 18 ++++++++++++++---- ...ltJakartaAuthenticationFeedbackHandler.java | 18 ++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java b/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java index ca4c365..afbc5a5 100644 --- a/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java +++ b/src/main/java/org/apache/sling/auth/core/spi/DefaultAuthenticationFeedbackHandler.java @@ -81,13 +81,15 @@ public static boolean handleRedirect(final HttpServletRequest request, final Htt if (redirect != null) { // and redirect ensuring the response is sent to the client try { - response.sendRedirect(redirect); + final String redirectTarget = (redirect.startsWith("/") && !redirect.contains("://")) ? redirect : "/"; + response.sendRedirect(redirectTarget); } catch (Exception e) { // expected: IOException and IllegalStateException + final String sanitizedRedirect = sanitizeForLog(redirect); LoggerFactory.getLogger(DefaultAuthenticationFeedbackHandler.class) .error( - "handleRedirect: Failed to send redirect to " + redirect - + ", aborting request without redirect", + "handleRedirect: Failed to send redirect to {}, aborting request without redirect", + sanitizedRedirect, e); } @@ -130,14 +132,22 @@ private static String getValidatedRedirectTarget(final HttpServletRequest reques // absolute target (in the servlet context) if (!AuthUtil.isRedirectValid(request, redirect)) { + final String sanitizedRedirect = sanitizeForLog(redirect); LoggerFactory.getLogger(DefaultAuthenticationFeedbackHandler.class) - .error("handleRedirect: Redirect target '{}' is invalid, redirecting to '/'", redirect); + .error("handleRedirect: Redirect target '{}' is invalid, redirecting to '/'", sanitizedRedirect); redirect = "/"; } return redirect; } + private static String sanitizeForLog(final String value) { + if (value == null) { + return null; + } + return value.replace("\r", "\\r").replace("\n", "\\n"); + } + /** * This default implementation does nothing. *

diff --git a/src/main/java/org/apache/sling/auth/core/spi/DefaultJakartaAuthenticationFeedbackHandler.java b/src/main/java/org/apache/sling/auth/core/spi/DefaultJakartaAuthenticationFeedbackHandler.java index f6f2f95..252201c 100644 --- a/src/main/java/org/apache/sling/auth/core/spi/DefaultJakartaAuthenticationFeedbackHandler.java +++ b/src/main/java/org/apache/sling/auth/core/spi/DefaultJakartaAuthenticationFeedbackHandler.java @@ -81,13 +81,15 @@ public static boolean handleRedirect(final HttpServletRequest request, final Htt if (redirect != null) { // and redirect ensuring the response is sent to the client try { - response.sendRedirect(redirect); + final String redirectTarget = (redirect.startsWith("/") && !redirect.contains("://")) ? redirect : "/"; + response.sendRedirect(redirectTarget); } catch (Exception e) { // expected: IOException and IllegalStateException + final String sanitizedRedirect = sanitizeForLog(redirect); LoggerFactory.getLogger(DefaultJakartaAuthenticationFeedbackHandler.class) .error( - "handleRedirect: Failed to send redirect to " + redirect - + ", aborting request without redirect", + "handleRedirect: Failed to send redirect to {}, aborting request without redirect", + sanitizedRedirect, e); } @@ -130,14 +132,22 @@ private static String getValidatedRedirectTarget(final HttpServletRequest reques // absolute target (in the servlet context) if (!AuthUtil.isRedirectValid(request, redirect)) { + final String sanitizedRedirect = sanitizeForLog(redirect); LoggerFactory.getLogger(DefaultJakartaAuthenticationFeedbackHandler.class) - .error("handleRedirect: Redirect target '{}' is invalid, redirecting to '/'", redirect); + .error("handleRedirect: Redirect target '{}' is invalid, redirecting to '/'", sanitizedRedirect); redirect = "/"; } return redirect; } + private static String sanitizeForLog(final String value) { + if (value == null) { + return null; + } + return value.replace("\r", "\\r").replace("\n", "\\n"); + } + /** * This default implementation does nothing. *