diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/StringUtils.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/StringUtils.java index 481a3aa2b42..82871391aab 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/StringUtils.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/StringUtils.java @@ -92,4 +92,21 @@ public static boolean isEmpty(String str) { return str == null || str.length() == 0; } + /** + * Sanitizes a string for safe inclusion in log messages by replacing + * newline ({@code \n}), carriage return ({@code \r}), and tab ({@code \t}) + * characters with their printable escape sequences. This prevents + * log-injection attacks (CWE-117) where attacker-controlled input + * containing newlines could forge fake log entries. + * + * @param str the string to sanitize, may be null + * @return the sanitized string, or {@code null} if input is {@code null} + */ + public static String sanitizeForLog(String str) { + if (str == null) { + return null; + } + return str.replace("\n", "\\n").replace("\r", "\\r").replace("\t", "\\t"); + } + } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java index 1e24c928353..ae8b5157f3d 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.Set; import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.common.StringUtils; import org.apache.zookeeper.server.ServerCnxn; import org.apache.zookeeper.server.ServerMetrics; import org.slf4j.Logger; @@ -92,7 +93,7 @@ public KeeperException.Code handleAuthentication(ServerCnxn cnxn, byte[] authDat long currentTime = System.currentTimeMillis(); if (lastFailureLogged + MIN_LOGGING_INTERVAL_MS < currentTime) { String id = cnxn.getRemoteSocketAddress().getAddress().getHostAddress(); - LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", receivedEnsembleName, id); + LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", StringUtils.sanitizeForLog(receivedEnsembleName), id); lastFailureLogged = currentTime; } /* diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/StringUtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/StringUtilTest.java index 35c70e095ea..3026557c2a4 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/StringUtilTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/StringUtilTest.java @@ -69,4 +69,19 @@ public void testStringJoin() { StringUtils.joinStrings(Arrays.asList("a", "B", null, "d"), ",")); } + @Test + public void testSanitizeForLog() { + assertNull(StringUtils.sanitizeForLog(null)); + assertEquals("normal-string", StringUtils.sanitizeForLog("normal-string")); + assertEquals("line1\\nline2", StringUtils.sanitizeForLog("line1\nline2")); + assertEquals("line1\\rline2", StringUtils.sanitizeForLog("line1\rline2")); + assertEquals("col1\\tcol2", StringUtils.sanitizeForLog("col1\tcol2")); + assertEquals("a\\r\\nb", StringUtils.sanitizeForLog("a\r\nb")); + // Verify a typical log-injection payload is neutralized + assertEquals( + "fake-cluster\\n2024-01-01 00:00:00 ERROR forged log line", + StringUtils.sanitizeForLog("fake-cluster\n2024-01-01 00:00:00 ERROR forged log line") + ); + } + }