ZOOKEEPER-5082: Remove special characters from audit logs

Reviewers: tisonkun, phunt, PDavid
Author: anmolnar
Closes #2448 from anmolnar/ZOOKEEPER-5082

(cherry picked from commit e4f0cd4)
Signed-off-by: Andor Molnar <andor@cloudera.com>

Author: anmolnar

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

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]", "");
+    }
 }

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;
         }

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);
+    }
 }