From 0761d76c3c9e6f97a37c6a60f7c4a187b7428bcd Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Mon, 31 Aug 2026 10:53:06 -0500 Subject: [PATCH 1/3] ZOOKEEPER-5082. Sanitize value of audit events before logging --- .../java/org/apache/zookeeper/audit/AuditEvent.java | 6 +++++- .../org/apache/zookeeper/audit/AuditEventTest.java | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) 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..2f337ee5c3b 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 @@ -75,7 +75,7 @@ public String toString() { buffer.append(PAIR_SEPARATOR); } buffer.append(key).append(KEY_VAL_SEPARATOR) - .append(value); + .append(sanitize(value)); } } //add result field @@ -94,5 +94,9 @@ public enum FieldName { public enum Result { SUCCESS, FAILURE, INVOKED } + + private String sanitize(String input) { + return input.replaceAll("[\\t\\n\\r]", ""); + } } 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); + } } From 7c5fde3101d31ca4e5b0dc6ca75126c6a2857b37 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Mon, 31 Aug 2026 11:34:21 -0500 Subject: [PATCH 2/3] ZOOKEEPER-5082. Consolidate with ensemble auth provider --- .../org/apache/zookeeper/audit/AuditEvent.java | 8 +++----- .../org/apache/zookeeper/common/StringUtils.java | 15 +++++++++++++++ .../auth/EnsembleAuthenticationProvider.java | 3 ++- 3 files changed, 20 insertions(+), 6 deletions(-) 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 2f337ee5c3b..e7f900b8d53 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 @@ -17,6 +17,8 @@ */ package org.apache.zookeeper.audit; +import org.apache.zookeeper.common.StringUtils; + import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -75,7 +77,7 @@ public String toString() { buffer.append(PAIR_SEPARATOR); } buffer.append(key).append(KEY_VAL_SEPARATOR) - .append(sanitize(value)); + .append(StringUtils.sanitizeForLog(value)); } } //add result field @@ -94,9 +96,5 @@ public enum FieldName { public enum Result { SUCCESS, FAILURE, INVOKED } - - private String sanitize(String input) { - return input.replaceAll("[\\t\\n\\r]", ""); - } } 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; } From adc44278f6e0feaab18b51187fcd347884803262 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Mon, 31 Aug 2026 14:05:21 -0500 Subject: [PATCH 3/3] ZOOKEEPER-5082. Checkstyle --- .../src/main/java/org/apache/zookeeper/audit/AuditEvent.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 e7f900b8d53..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 @@ -17,11 +17,10 @@ */ package org.apache.zookeeper.audit; -import org.apache.zookeeper.common.StringUtils; - 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';