diff --git a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java
index e11e8c47673..d05ac6a97b2 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/xml/XmlConfiguration.java
@@ -27,6 +27,7 @@
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.FactoryConfigurationError;
+import javax.xml.parsers.ParserConfigurationException;
import org.apache.log4j.Appender;
import org.apache.log4j.Layout;
import org.apache.log4j.Level;
@@ -162,7 +163,7 @@ private void doConfigure(final ParseAction action) throws FactoryConfigurationEr
}
try {
- dbf.setValidating(true);
+ disableDtdProcessing(dbf);
final DocumentBuilder docBuilder = dbf.newDocumentBuilder();
@@ -180,6 +181,27 @@ private void doConfigure(final ParseAction action) throws FactoryConfigurationEr
}
}
+ private static void disableDtdProcessing(final DocumentBuilderFactory factory) {
+ factory.setValidating(false);
+ factory.setExpandEntityReferences(false);
+ setFeature(factory, "http://xml.org/sax/features/external-general-entities", false);
+ setFeature(factory, "http://xml.org/sax/features/external-parameter-entities", false);
+ setFeature(factory, "http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+ }
+
+ private static void setFeature(
+ final DocumentBuilderFactory factory, final String featureName, final boolean value) {
+ try {
+ factory.setFeature(featureName, value);
+ } catch (final ParserConfigurationException e) {
+ LOGGER.warn(
+ "The DocumentBuilderFactory [{}] does not support the feature [{}]: {}", factory, featureName, e);
+ } catch (final AbstractMethodError err) {
+ LOGGER.warn(
+ "The DocumentBuilderFactory [{}] is out of date and does not support setFeature: {}", factory, err);
+ }
+ }
+
@Override
public Configuration reconfigure() {
try {
diff --git a/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationXxeTest.java b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationXxeTest.java
new file mode 100644
index 00000000000..f02761d91d1
--- /dev/null
+++ b/log4j-1.2-api/src/test/java/org/apache/log4j/config/XmlConfigurationXxeTest.java
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.log4j.config;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.log4j.xml.XmlConfigurationFactory;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Checks that the Log4j 1.x XML configuration reader does not resolve external entities.
+ */
+class XmlConfigurationXxeTest {
+
+ @Test
+ void doesNotResolveExternalEntities(@TempDir final Path tempDir) throws Exception {
+ // If the external entity is resolved, its replacement text injects this logger into the configuration.
+ final Path injected = tempDir.resolve("injected.xml");
+ Files.write(injected, "".getBytes(StandardCharsets.UTF_8));
+
+ final Path configFile = tempDir.resolve("log4j1-xxe.xml");
+ final String xml = "\n"
+ + "\n"
+ + "]>\n"
+ + "\n"
+ + " \n"
+ + " \n"
+ + " \n"
+ + " &xxe;\n"
+ + " \n"
+ + " \n"
+ + " \n"
+ + " \n"
+ + "\n";
+ Files.write(configFile, xml.getBytes(StandardCharsets.UTF_8));
+
+ final ConfigurationSource source = new ConfigurationSource(Files.newInputStream(configFile), configFile);
+ final LoggerContext context = LoggerContext.getContext(false);
+ final Configuration configuration = new XmlConfigurationFactory().getConfiguration(context, source);
+ assertNotNull(configuration, "No configuration created");
+ configuration.initialize();
+
+ assertFalse(
+ configuration.getLoggers().containsKey("xxe-injected"),
+ "External entity was resolved; the parser must not process external entities");
+ }
+}
diff --git a/src/changelog/.2.x.x/fix_log4j1_xml_configuration_xxe.xml b/src/changelog/.2.x.x/fix_log4j1_xml_configuration_xxe.xml
new file mode 100644
index 00000000000..14757162250
--- /dev/null
+++ b/src/changelog/.2.x.x/fix_log4j1_xml_configuration_xxe.xml
@@ -0,0 +1,12 @@
+
+
+
+
+ Disable external entity resolution in the Log4j 1.x `XmlConfiguration` XML reader
+
+