diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/audit/AuditEvent.java b/zookeeper-server/src/main/java/org/apache/zookeeper/audit/AuditEvent.java index e499552a948..8138d0ebc5d 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/audit/AuditEvent.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/audit/AuditEvent.java @@ -20,6 +20,7 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import org.apache.zookeeper.common.StringUtils; public final class AuditEvent { private static final char PAIR_SEPARATOR = '\t'; @@ -75,7 +76,7 @@ public String toString() { buffer.append(PAIR_SEPARATOR); } buffer.append(key).append(KEY_VAL_SEPARATOR) - .append(value); + .append(StringUtils.sanitizeForLog(value)); } } //add result field 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..867c706adff 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,19 @@ public static boolean isEmpty(String str) { return str == null || str.length() == 0; } + /** + * Sanitizes a string for safe inclusion in log messages by removing + * any control characters between [\x00-\x1F]. 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.replaceAll("[\\x00-\\x1F]", ""); + } } 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 331db2c5b31..93d9b7d6d13 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(); - String logEnsembleName = receivedEnsembleName.replaceAll("[\\x00-\\x1F]", ""); + String logEnsembleName = StringUtils.sanitizeForLog(receivedEnsembleName); LOG.warn("Unexpected ensemble name: ensemble name: {} client ip: {}", logEnsembleName, id); lastFailureLogged = currentTime; } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/audit/AuditEventTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/audit/AuditEventTest.java index faa872ae6f0..d76720ba0fd 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/audit/AuditEventTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/audit/AuditEventTest.java @@ -42,4 +42,14 @@ public void testFormatShouldIgnoreKeyIfValueIsNull() { String expected = "operation=Value2\tresult=success"; assertEquals(expected, actual); } + + @Test + public void testSanitizeInvalidOutput() { + AuditEvent auditEvent = new AuditEvent(Result.SUCCESS); + auditEvent.addEntry(AuditEvent.FieldName.USER, "Value1"); + auditEvent.addEntry(AuditEvent.FieldName.OPERATION, "\nfake\toperation=delete\rznode=/forged:pw"); + String actual = auditEvent.toString(); + String expected = "user=Value1\toperation=fakeoperation=deleteznode=/forged:pw\tresult=success"; + assertEquals(expected, actual); + } }