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 + +