diff --git a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml index c2b376b95c6d..7c67ccd9a67e 100755 --- a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml +++ b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml @@ -2433,4 +2433,23 @@ + + + + + + + + + + + + + + + + + + + diff --git a/sdk/core/azure-core/pom.xml b/sdk/core/azure-core/pom.xml index 923521b8d089..b17fa816a83c 100644 --- a/sdk/core/azure-core/pom.xml +++ b/sdk/core/azure-core/pom.xml @@ -183,6 +183,23 @@ + + + + com.github.spotbugs + spotbugs-maven-plugin + 3.1.12.2 + + + + com.h3xstream.findsecbugs + findsecbugs-plugin + 1.9.0 + + + + + diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/ClientLogger.java b/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/ClientLogger.java index 090072a409e1..eb390c706729 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/ClientLogger.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/ClientLogger.java @@ -12,6 +12,7 @@ import java.util.Arrays; import java.util.Objects; +import java.util.regex.Pattern; /** * This is a fluent logger helper class that wraps a pluggable {@link Logger}. @@ -35,6 +36,7 @@ * @see Configuration */ public class ClientLogger { + private static final Pattern CRLF_PATTERN = Pattern.compile("[\r\n]"); private final Logger logger; /** @@ -70,7 +72,7 @@ public ClientLogger(String className) { */ public void verbose(String message) { if (logger.isDebugEnabled()) { - logger.debug(message); + logger.debug(sanitizeLogMessageInput(message)); } } @@ -106,7 +108,7 @@ public void verbose(String format, Object... args) { */ public void info(String message) { if (logger.isInfoEnabled()) { - logger.info(message); + logger.info(sanitizeLogMessageInput(message)); } } @@ -142,7 +144,7 @@ public void info(String format, Object... args) { */ public void warning(String message) { if (logger.isWarnEnabled()) { - logger.warn(message); + logger.warn(sanitizeLogMessageInput(message)); } } @@ -178,7 +180,7 @@ public void warning(String format, Object... args) { */ public void error(String message) { if (logger.isErrorEnabled()) { - logger.error(message); + logger.error(sanitizeLogMessageInput(message)); } } @@ -327,6 +329,7 @@ private void performLogging(LogLevel logLevel, boolean isExceptionLogging, Strin } } + sanitizeLogMessageInput(format); switch (logLevel) { case VERBOSE: logger.debug(format, args); @@ -401,4 +404,14 @@ private boolean doesArgsHaveThrowable(Object... args) { private Object[] removeThrowable(Object... args) { return Arrays.copyOf(args, args.length - 1); } + + /** + * Removes CRLF pattern in the {@code logMessage}. + * + * @param logMessage The log message to sanitize. + * @return The updated logMessage. + */ + private static String sanitizeLogMessageInput(String logMessage) { + return CRLF_PATTERN.matcher(logMessage).replaceAll(""); + } } diff --git a/sdk/core/azure-core/src/test/java/com/azure/core/util/logging/ClientLoggerTests.java b/sdk/core/azure-core/src/test/java/com/azure/core/util/logging/ClientLoggerTests.java index 91b4c78d6845..11d6a8a804da 100644 --- a/sdk/core/azure-core/src/test/java/com/azure/core/util/logging/ClientLoggerTests.java +++ b/sdk/core/azure-core/src/test/java/com/azure/core/util/logging/ClientLoggerTests.java @@ -80,6 +80,30 @@ public void logSimpleMessage(LogLevel logLevelToConfigure, LogLevel logLevelToUs assertEquals(logContainsMessage, logValues.contains(logMessage)); } + /** + * Test whether a log will be captured when the ClientLogger and message are configured to the passed log levels. + */ + @ParameterizedTest + @MethodSource("logMaliciousErrorSupplier") + @ResourceLock("SYSTEM_OUT") + public void logMaliciousMessage(LogLevel logLevelToConfigure, LogLevel logLevelToUse) + throws UnsupportedEncodingException { + String logMessage = "You have successfully authenticated, \r\n[INFO] User dummy was not" + + " successfully authenticated."; + + String expectedMessage = "You have successfully authenticated, [INFO] User dummy was not" + + " successfully authenticated."; + + String originalLogLevel = setupLogLevel(logLevelToConfigure.getLogLevel()); + logMessage(new ClientLogger(ClientLoggerTests.class), logLevelToUse, logMessage); + + setPropertyToOriginalOrClear(originalLogLevel); + + String logValues = logCaptureStream.toString("UTF-8"); + System.out.println(logValues); + assertEquals(true, logValues.contains(expectedMessage)); + } + @ParameterizedTest @MethodSource("singleLevelCheckSupplier") @ResourceLock("SYSTEM_OUT") @@ -461,6 +485,41 @@ private static Stream multiLevelCheckSupplier() { ); } + private static Stream logMaliciousErrorSupplier() { + return Stream.of( + // Supported logging level set to VERBOSE. + // Checking VERBOSE. + Arguments.of(LogLevel.VERBOSE, LogLevel.VERBOSE, true), + + // Checking INFORMATIONAL. + Arguments.of(LogLevel.VERBOSE, LogLevel.INFORMATIONAL, true), + + // Checking WARNING. + Arguments.of(LogLevel.VERBOSE, LogLevel.WARNING, true), + + // Checking ERROR. + Arguments.of(LogLevel.VERBOSE, LogLevel.ERROR, true), + + // Checking INFORMATIONAL. + Arguments.of(LogLevel.INFORMATIONAL, LogLevel.INFORMATIONAL, true), + + // Checking WARNING. + Arguments.of(LogLevel.INFORMATIONAL, LogLevel.WARNING, true), + + // Checking ERROR. + Arguments.of(LogLevel.INFORMATIONAL, LogLevel.ERROR, true), + + // Checking WARNING. + Arguments.of(LogLevel.WARNING, LogLevel.WARNING, true), + + // Checking ERROR. + Arguments.of(LogLevel.WARNING, LogLevel.ERROR, true), + + // Checking ERROR. + Arguments.of(LogLevel.ERROR, LogLevel.ERROR, true) + ); + } + private static Stream logExceptionAsWarningSupplier() { return Stream.of( Arguments.of(LogLevel.VERBOSE, true, true),