Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]", "");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Loading