Remove most alerts

ppkarwasz · ppkarwasz · commit c0fcaaa0c688 · 2023-11-06T16:21:58.000+01:00
Removes all the security alerts, except PATH_TRAVERSAL_IN/OUT and URLCONNECTION_SSRF_FD.
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RolloverFilePatternTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RolloverFilePatternTest.java
@@ -30,30 +30,30 @@ public class RolloverFilePatternTest {
     @Test
     public void testFilePatternWithoutPadding() throws Exception {
       final Matcher matcher = AbstractRolloverStrategy.PATTERN_COUNTER.matcher("target/logs/test-%i.log.gz");
-      assertTrue(matcher.matches());
+      assertTrue(matcher.find());
       assertNull(matcher.group("ZEROPAD"));
       assertNull(matcher.group("PADDING"));
     }
 
     @Test
     public void testFilePatternWithSpacePadding() throws Exception {
       final Matcher matcher = AbstractRolloverStrategy.PATTERN_COUNTER.matcher("target/logs/test-%3i.log.gz");
-      assertTrue(matcher.matches());
+      assertTrue(matcher.find());
       assertNull(matcher.group("ZEROPAD"));
       assertEquals("3", matcher.group("PADDING"));
     }
 
     @Test
     public void testFilePatternWithZeroPadding() throws Exception {
       final Matcher matcher = AbstractRolloverStrategy.PATTERN_COUNTER.matcher("target/logs/test-%03i.log.gz");
-      assertTrue(matcher.matches());
+      assertTrue(matcher.find());
       assertEquals("0", matcher.group("ZEROPAD"));
       assertEquals("3", matcher.group("PADDING"));
     }
 
     @Test
     public void testFilePatternUnmatched() throws Exception {
       final Matcher matcher = AbstractRolloverStrategy.PATTERN_COUNTER.matcher("target/logs/test-%n.log.gz");
-      assertFalse(matcher.matches());
+      assertFalse(matcher.find());
     }
 }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
@@ -38,6 +38,7 @@
 import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.StringLayout;
@@ -589,6 +590,10 @@ private boolean commitAndCloseAll() {
         return true;
     }
 
+    @SuppressFBWarnings(
+            value = "SQL_INJECTION_JDBC",
+            justification = "The SQL statement is generated based on the configuration file."
+    )
     private void connectAndPrepare() throws SQLException {
         logger().debug("Acquiring JDBC connection from {}", this.getConnectionSource());
         this.connection = getConnectionSource().getConnection();
@@ -654,6 +659,10 @@ public String getTableName() {
         return factoryData.tableName;
     }
 
+    @SuppressFBWarnings(
+            value = "SQL_INJECTION_JDBC",
+            justification = "The SQL statement is generated based on the configuration file."
+    )
     private void initColumnMetaData() throws SQLException {
         // Could use:
         // this.connection.getMetaData().getColumns(catalog, schemaPattern, tableNamePattern, columnNamePattern);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/AbstractRolloverStrategy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/AbstractRolloverStrategy.java
@@ -48,7 +48,7 @@ public abstract class AbstractRolloverStrategy implements RolloverStrategy {
      */
     protected static final Logger LOGGER = StatusLogger.getLogger();
 
-    public static final Pattern PATTERN_COUNTER= Pattern.compile(".*%((?<ZEROPAD>0)?(?<PADDING>\\d+))?i.*");
+    public static final Pattern PATTERN_COUNTER = Pattern.compile(".*%(?<ZEROPAD>0)?(?<PADDING>\\d+)?i.*");
 
     protected final StrSubstitutor strSubstitutor;
 
@@ -121,7 +121,7 @@ protected SortedMap<Integer, Path> getEligibleFiles(final String currentFile, fi
         } else {
             parent.mkdirs();
         }
-        if (!PATTERN_COUNTER.matcher(logfilePattern).matches()) {
+        if (!PATTERN_COUNTER.matcher(logfilePattern).find()) {
             return eligibleFiles;
         }
         final Path dir = parent.toPath();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/TimeBasedTriggeringPolicy.java
@@ -19,6 +19,7 @@
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.Core;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
@@ -106,6 +107,7 @@ public long getNextRolloverMillis() {
      * @param aManager The RollingFileManager.
      */
     @Override
+    @SuppressFBWarnings("PREDICTABLE_RANDOM")
     public void initialize(final RollingFileManager aManager) {
         this.manager = aManager;
         long current = aManager.getFileTime();
@@ -127,6 +129,7 @@ public void initialize(final RollingFileManager aManager) {
      * @return true if a rollover should occur.
      */
     @Override
+    @SuppressFBWarnings("PREDICTABLE_RANDOM")
     public boolean isTriggeringEvent(final LogEvent event) {
         final long nowMillis = event.getTimeMillis();
         if (nowMillis >= nextRolloverMillis) {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java
@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.Set;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.Core;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
@@ -114,6 +115,10 @@ public static class Builder implements org.apache.logging.log4j.core.util.Builde
         private String fileGroup;
 
         @Override
+        @SuppressFBWarnings(
+                value = "OVERLY_PERMISSIVE_FILE_PERMISSION",
+                justification = "File permissions are specified in a configuration file."
+        )
         public PosixViewAttributeAction build() {
             if (Strings.isEmpty(basePath)) {
                 LOGGER.error("Posix file attribute view action not valid because base path is empty.");
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/builder/impl/DefaultConfigurationBuilder.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/builder/impl/DefaultConfigurationBuilder.java
@@ -39,6 +39,7 @@
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.Filter;
 import org.apache.logging.log4j.core.LoggerContext;
@@ -91,6 +92,10 @@ public class DefaultConfigurationBuilder<T extends BuiltConfiguration> implement
     private LoggerContext loggerContext;
     private String name;
 
+    @SuppressFBWarnings(
+            value = {"XXE_DTD_TRANSFORM_FACTORY", "XXE_XSLT_TRANSFORM_FACTORY"},
+            justification = "This method only uses internally generated data."
+    )
     public static void formatXml(final Source source, final Result result)
         throws TransformerConfigurationException, TransformerFactoryConfigurationError, TransformerException {
         final Transformer transformer = TransformerFactory.newInstance().newTransformer();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java
@@ -34,6 +34,7 @@
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.config.AbstractConfiguration;
 import org.apache.logging.log4j.core.config.Configuration;
@@ -74,6 +75,10 @@ public class XmlConfiguration extends AbstractConfiguration implements Reconfigu
     private boolean strict;
     private String schemaResource;
 
+    @SuppressFBWarnings(
+            value = "XXE_DOCUMENT",
+            justification = "The `newDocumentBuilder` method disables DTD processing."
+    )
     public XmlConfiguration(final LoggerContext loggerContext, final ConfigurationSource configSource) {
         super(loggerContext, configSource);
         final File configFile = configSource.getFile();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
@@ -41,6 +41,7 @@
 import javax.management.NotificationBroadcasterSupport;
 import javax.management.ObjectName;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.LoggerContext;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.ConfigurationFactory;
@@ -152,6 +153,10 @@ public String getConfigText() throws IOException {
     }
 
     @Override
+    @SuppressFBWarnings(
+            value = "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE",
+            justification = "JMX should be considered a trusted channel."
+    )
     public String getConfigText(final String charsetName) throws IOException {
         try {
             final ConfigurationSource source = loggerContext.getConfiguration().getConfigurationSource();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/GelfLayout.java
@@ -30,6 +30,7 @@
 import java.util.zip.DeflaterOutputStream;
 import java.util.zip.GZIPOutputStream;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.LogEvent;
@@ -747,6 +748,10 @@ private int formatLevel(final Level level) {
     /**
      * Non-private to make it accessible from unit test.
      */
+    @SuppressFBWarnings(
+            value = "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE",
+            justification = "Log4j prints stacktraces only to logs, which should be private."
+    )
     static CharSequence formatThrowable(final Throwable throwable) {
         // stack traces are big enough to provide a reasonably large initial capacity here
         final StringWriter sw = new StringWriter(2048);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/HtmlLayout.java
@@ -27,6 +27,7 @@
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.LogEvent;
@@ -238,6 +239,10 @@ public String getContentType() {
         return contentType;
     }
 
+    @SuppressFBWarnings(
+            value = "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE",
+            justification = "Log4j prints stacktraces only to logs, which should be private."
+    )
     private void appendThrowableAsHtml(final Throwable throwable, final StringBuilder sbuf) {
         final StringWriter sw = new StringWriter();
         final PrintWriter pw = new PrintWriter(sw);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java
@@ -25,6 +25,7 @@
 import javax.naming.InitialContext;
 import javax.naming.NamingException;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.appender.AbstractManager;
 import org.apache.logging.log4j.core.appender.ManagerFactory;
 import org.apache.logging.log4j.core.util.JndiCloser;
@@ -226,6 +227,10 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) {
      * @throws  NamingException if a naming exception is encountered
      */
     @SuppressWarnings({"unchecked", "BanJNDI"})
+    @SuppressFBWarnings(
+            value = "LDAP_INJECTION",
+            justification = "This method only accepts an empty or 'java:' URI scheme."
+    )
     public <T> T lookup(final String name) throws NamingException {
         if (context == null) {
             return null;
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SmtpManager.java
@@ -37,6 +37,7 @@
 import javax.mail.util.ByteArrayDataSource;
 import javax.net.ssl.SSLSocketFactory;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.LoggingException;
 import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.LogEvent;
@@ -224,8 +225,11 @@ protected void sendMultipartMessage(final MimeMessage msg, final MimeMultipart m
         }
     }
 
-    protected void sendMultipartMessage(final MimeMessage msg, final MimeMultipart mp, final String subject) throws MessagingException {
-        synchronized (msg) {
+    @SuppressFBWarnings(
+            value = "SMTP_HEADER_INJECTION",
+            justification = "False positive, since MimeMessage#setSubject does actually escape new lines."
+    )
+    protected void sendMultipartMessage(final MimeMessage msg, final MimeMultipart mp, final String subject) throws MessagingException {synchronized (msg) {
             msg.setContent(mp);
             msg.setSentDate(new Date());
             msg.setSubject(subject);
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/TcpSocketManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/TcpSocketManager.java
@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.appender.AppenderLoggingException;
 import org.apache.logging.log4j.core.appender.ManagerFactory;
@@ -398,6 +399,9 @@ protected Socket createSocket(final InetSocketAddress socketAddress) throws IOEx
         return createSocket(socketAddress, socketOptions, connectTimeoutMillis);
     }
 
+    @SuppressFBWarnings(
+            value = "UNENCRYPTED_SOCKET"
+    )
     protected static Socket createSocket(final InetSocketAddress socketAddress, final SocketOptions socketOptions,
             final int connectTimeoutMillis) throws IOException {
         LOGGER.debug("Creating socket {}", socketAddress.toString());
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/LaxHostnameVerifier.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/ssl/LaxHostnameVerifier.java
@@ -19,6 +19,8 @@
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 /**
  * An HostnameVerifier which accepts everything.
  */
@@ -32,6 +34,7 @@ private LaxHostnameVerifier() {
     }
 
     @Override
+    @SuppressFBWarnings("WEAK_HOSTNAME_VERIFIER")
     public boolean verify(final String s, final SSLSession sslSession) {
         return true;
     }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowablePatternConverter.java
@@ -22,6 +22,7 @@
 import java.util.Collections;
 import java.util.List;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
@@ -185,6 +186,10 @@ else if (ThrowableFormatOptions.FILE_NAME.equalsIgnoreCase(rawOption)) {
         }
     }
 
+    @SuppressFBWarnings(
+            value = "INFORMATION_EXPOSURE_THROUGH_AN_ERROR_MESSAGE",
+            justification = "Formatting a throwable is the main purpose of this class."
+    )
     private void formatOption(final Throwable throwable, final String suffix, final StringBuilder buffer) {
         final int len = buffer.length();
         if (len > 0 && !Character.isWhitespace(buffer.charAt(len - 1))) {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/tools/Generate.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/tools/Generate.java
@@ -21,6 +21,7 @@
 import java.util.Arrays;
 import java.util.List;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.core.util.Integers;
 
 /**
@@ -1111,6 +1112,10 @@ private static void usage(final PrintStream out, final Class<?> generator) {
         out.println("       For each custom log level, specify NAME=intLevel (without spaces).");
     }
 
+    @SuppressFBWarnings(
+            value = "FORMAT_STRING_MANIPULATION",
+            justification = "The format strings come from constants. The replacement is done for readability."
+    )
     static String generateSource(final String classNameFQN, final List<LevelInfo> levels, final Type type) {
         final StringBuilder sb = new StringBuilder(10000 * levels.size());
         final int lastDot = classNameFQN.lastIndexOf('.');
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NameUtil.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NameUtil.java
@@ -21,6 +21,7 @@
 import java.security.NoSuchAlgorithmException;
 import java.util.Objects;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.logging.log4j.util.Strings;
 
 /**
@@ -50,6 +51,11 @@ public static String getSubName(final String name) {
      * @param input string to be hashed
      * @return string composed of 32 hexadecimal digits of the calculated hash
      */
+    @SuppressFBWarnings(
+            value = "WEAK_MESSAGE_DIGEST_MD5",
+            justification = "Used to create unique identifiers."
+    )
+    @Deprecated
     public static String md5(final String input) {
         Objects.requireNonNull(input, "input");
         try {
@@ -58,11 +64,8 @@ public static String md5(final String input) {
             final byte[] bytes = digest.digest(inputBytes);
             final StringBuilder md5 = new StringBuilder(bytes.length * 2);
             for (final byte b : bytes) {
-                final String hex = Integer.toHexString(0xFF & b);
-                if (hex.length() == 1) {
-                    md5.append('0');
-                }
-                md5.append(hex);
+                md5.append(Character.forDigit((0xFF & b) >> 4, 16));
+                md5.append(Character.forDigit(0x0F & b, 16));
             }
             return md5.toString();
         }
@@ -74,5 +77,4 @@ public static String md5(final String input) {
             throw new RuntimeException(error);
         }
     }
-
 }

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ public abstract class AbstractRolloverStrategy implements RolloverStrategy {`
`48`	`48`	`*/`
`49`	`49`	`protected static final Logger LOGGER = StatusLogger.getLogger();`
`50`	`50`
`51`		`- public static final Pattern PATTERN_COUNTER= Pattern.compile(".%((?<ZEROPAD>0)?(?<PADDING>\\d+))?i.");`
	`51`	`+ public static final Pattern PATTERN_COUNTER = Pattern.compile(".%(?<ZEROPAD>0)?(?<PADDING>\\d+)?i.");`
`52`	`52`
`53`	`53`	`protected final StrSubstitutor strSubstitutor;`
`54`	`54`
`@@ -121,7 +121,7 @@ protected SortedMap<Integer, Path> getEligibleFiles(final String currentFile, fi`
`121`	`121`	`} else {`
`122`	`122`	`parent.mkdirs();`
`123`	`123`	`}`
`124`		`- if (!PATTERN_COUNTER.matcher(logfilePattern).matches()) {`
	`124`	`+ if (!PATTERN_COUNTER.matcher(logfilePattern).find()) {`
`125`	`125`	`return eligibleFiles;`
`126`	`126`	`}`
`127`	`127`	`final Path dir = parent.toPath();`