This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch iris-galliform in repository https://gitbox.apache.org/repos/asf/maven.git
commit 5bf8a9d21525daa2c61321d6c88e9390116e5c56 Author: Guillaume Nodet <[email protected]> AuthorDate: Fri Jun 12 23:09:03 2026 +0200 Centralize XXE hardening for StAX XML parsers Add XmlService.newXMLInputFactory() as a single shared method that creates a hardened XMLInputFactory with SUPPORT_DTD and IS_SUPPORTING_EXTERNAL_ENTITIES disabled. Replace all direct XMLInputFactory.newFactory() calls across the codebase with this centralized method. Also harden the Velocity templates (reader-stax.vm and reader.vm) that generate StAX readers for settings, toolchains, metadata, and plugin descriptors. Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../java/org/apache/maven/api/xml/XmlService.java | 15 ++++++++++++++ .../maven/model/root/DefaultRootLocator.java | 5 +++-- .../plugin/descriptor/PluginDescriptorBuilder.java | 21 +++++++++---------- .../maven/project/ExtensionDescriptorBuilder.java | 3 +-- .../project/ExtensionDescriptorBuilderTest.java | 24 ++++++++++++++++++++++ .../apache/maven/impl/DefaultModelXmlFactory.java | 3 ++- .../impl/model/rootlocator/PomXmlRootDetector.java | 4 ++-- .../maven/internal/xml/DefaultXmlService.java | 5 ++--- .../maven/internal/xml/XmlNodeStaxBuilder.java | 5 ++--- src/mdo/reader-stax.vm | 2 ++ src/mdo/reader.vm | 6 ++++++ 11 files changed, 69 insertions(+), 24 deletions(-) diff --git a/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlService.java b/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlService.java index e6735e255f..7c2bc6392c 100644 --- a/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlService.java +++ b/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlService.java @@ -18,6 +18,7 @@ */ package org.apache.maven.api.xml; +import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -102,6 +103,20 @@ public abstract class XmlService { */ public static final String KEYS_COMBINATION_MODE_ATTRIBUTE = "combine.keys"; + /** + * Creates a new {@link XMLInputFactory} hardened against XXE attacks. + * The returned factory has DTD support and external entity resolution disabled. + * + * @return a hardened XMLInputFactory + * @since 4.1.0 + */ + public static XMLInputFactory newXMLInputFactory() { + XMLInputFactory factory = XMLInputFactory.newFactory(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + return factory; + } + /** * Convenience method to merge two XML nodes using default settings. */ diff --git a/compat/maven-model-builder/src/main/java/org/apache/maven/model/root/DefaultRootLocator.java b/compat/maven-model-builder/src/main/java/org/apache/maven/model/root/DefaultRootLocator.java index d248ec94f8..9620ad7ef2 100644 --- a/compat/maven-model-builder/src/main/java/org/apache/maven/model/root/DefaultRootLocator.java +++ b/compat/maven-model-builder/src/main/java/org/apache/maven/model/root/DefaultRootLocator.java @@ -19,7 +19,6 @@ package org.apache.maven.model.root; import javax.inject.Named; -import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -28,6 +27,8 @@ import java.nio.file.Files; import java.nio.file.Path; +import org.apache.maven.api.xml.XmlService; + /** * @deprecated use {@code org.apache.maven.api.services.model.RootLocator} instead */ @@ -43,7 +44,7 @@ public boolean isRootDirectory(Path dir) { // we're too early to use the modelProcessor ... Path pom = dir.resolve("pom.xml"); try (InputStream is = Files.newInputStream(pom)) { - XMLStreamReader parser = XMLInputFactory.newFactory().createXMLStreamReader(is); + XMLStreamReader parser = XmlService.newXMLInputFactory().createXMLStreamReader(is); if (parser.nextTag() == XMLStreamReader.START_ELEMENT && parser.getLocalName().equals("project")) { for (int i = 0; i < parser.getAttributeCount(); i++) { diff --git a/compat/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/PluginDescriptorBuilder.java b/compat/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/PluginDescriptorBuilder.java index 086a49daaf..4db13772d6 100644 --- a/compat/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/PluginDescriptorBuilder.java +++ b/compat/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/PluginDescriptorBuilder.java @@ -18,7 +18,6 @@ */ package org.apache.maven.plugin.descriptor; -import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -74,12 +73,12 @@ public PluginDescriptor build(Reader reader, String source) throws PlexusConfigu try { BufferedReader br = new BufferedReader(reader, BUFFER_SIZE); br.mark(BUFFER_SIZE); - XMLStreamReader xsr = XMLInputFactory.newFactory().createXMLStreamReader(br); + XMLStreamReader xsr = XmlService.newXMLInputFactory().createXMLStreamReader(br); xsr.nextTag(); String nsUri = xsr.getNamespaceURI(); br.reset(); if (PLUGIN_2_0_0.equals(nsUri)) { - xsr = XMLInputFactory.newFactory().createXMLStreamReader(br); + xsr = XmlService.newXMLInputFactory().createXMLStreamReader(br); return new PluginDescriptor(new PluginDescriptorStaxReader().read(xsr, true)); } else { // Call buildConfiguration() for backward compatibility with subclasses that override it @@ -98,11 +97,11 @@ public PluginDescriptor build(ReaderSupplier readerSupplier) throws PlexusConfig public PluginDescriptor build(ReaderSupplier readerSupplier, String source) throws PlexusConfigurationException { try (BufferedReader br = new BufferedReader(readerSupplier.open(), BUFFER_SIZE)) { br.mark(BUFFER_SIZE); - XMLStreamReader xsr = XMLInputFactory.newFactory().createXMLStreamReader(br); + XMLStreamReader xsr = XmlService.newXMLInputFactory().createXMLStreamReader(br); xsr.nextTag(); String nsUri = xsr.getNamespaceURI(); try (BufferedReader br2 = reset(readerSupplier, br)) { - xsr = XMLInputFactory.newFactory().createXMLStreamReader(br2); + xsr = XmlService.newXMLInputFactory().createXMLStreamReader(br2); return build(source, nsUri, xsr); } } catch (XMLStreamException | IOException e) { @@ -118,12 +117,12 @@ public PluginDescriptor build(InputStream input, String source) throws PlexusCon try { BufferedInputStream bis = new BufferedInputStream(input, BUFFER_SIZE); bis.mark(BUFFER_SIZE); - XMLStreamReader xsr = XMLInputFactory.newFactory().createXMLStreamReader(bis); + XMLStreamReader xsr = XmlService.newXMLInputFactory().createXMLStreamReader(bis); xsr.nextTag(); String nsUri = xsr.getNamespaceURI(); bis.reset(); if (PLUGIN_2_0_0.equals(nsUri)) { - xsr = XMLInputFactory.newFactory().createXMLStreamReader(bis); + xsr = XmlService.newXMLInputFactory().createXMLStreamReader(bis); return new PluginDescriptor(new PluginDescriptorStaxReader().read(xsr, true)); } else { // Call buildConfiguration() for backward compatibility with subclasses that override it @@ -142,11 +141,11 @@ public PluginDescriptor build(StreamSupplier inputSupplier) throws PlexusConfigu public PluginDescriptor build(StreamSupplier inputSupplier, String source) throws PlexusConfigurationException { try (BufferedInputStream bis = new BufferedInputStream(inputSupplier.open(), BUFFER_SIZE)) { bis.mark(BUFFER_SIZE); - XMLStreamReader xsr = XMLInputFactory.newFactory().createXMLStreamReader(bis); + XMLStreamReader xsr = XmlService.newXMLInputFactory().createXMLStreamReader(bis); xsr.nextTag(); String nsUri = xsr.getNamespaceURI(); try (BufferedInputStream bis2 = reset(inputSupplier, bis)) { - xsr = XMLInputFactory.newFactory().createXMLStreamReader(bis2); + xsr = XmlService.newXMLInputFactory().createXMLStreamReader(bis2); return build(source, nsUri, xsr); } } catch (XMLStreamException | IOException e) { @@ -504,7 +503,7 @@ public MojoDescriptor buildComponentDescriptor(PlexusConfiguration c, PluginDesc public PlexusConfiguration buildConfiguration(Reader configuration) throws PlexusConfigurationException { try { - XMLStreamReader reader = XMLInputFactory.newFactory().createXMLStreamReader(configuration); + XMLStreamReader reader = XmlService.newXMLInputFactory().createXMLStreamReader(configuration); return XmlPlexusConfiguration.toPlexusConfiguration(XmlService.read(reader)); } catch (XMLStreamException e) { throw new PlexusConfigurationException(e.getMessage(), e); @@ -513,7 +512,7 @@ public PlexusConfiguration buildConfiguration(Reader configuration) throws Plexu public PlexusConfiguration buildConfiguration(InputStream configuration) throws PlexusConfigurationException { try { - XMLStreamReader reader = XMLInputFactory.newFactory().createXMLStreamReader(configuration); + XMLStreamReader reader = XmlService.newXMLInputFactory().createXMLStreamReader(configuration); return XmlPlexusConfiguration.toPlexusConfiguration(XmlService.read(reader)); } catch (XMLStreamException e) { throw new PlexusConfigurationException(e.getMessage(), e); diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/ExtensionDescriptorBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/project/ExtensionDescriptorBuilder.java index a29cc4bd80..e2bbbc36ca 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/ExtensionDescriptorBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/ExtensionDescriptorBuilder.java @@ -18,7 +18,6 @@ */ package org.apache.maven.project; -import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -88,7 +87,7 @@ public ExtensionDescriptor build(InputStream is) throws IOException { XmlNode dom; try { - XMLStreamReader reader = XMLInputFactory.newFactory().createXMLStreamReader(is); + XMLStreamReader reader = XmlService.newXMLInputFactory().createXMLStreamReader(is); dom = XmlService.read(reader); } catch (XMLStreamException e) { throw new IOException(e.getMessage(), e); diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/ExtensionDescriptorBuilderTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/ExtensionDescriptorBuilderTest.java index 4232e08c23..fb563d9ae7 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/ExtensionDescriptorBuilderTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/ExtensionDescriptorBuilderTest.java @@ -19,8 +19,11 @@ package org.apache.maven.project; import java.io.ByteArrayInputStream; +import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import org.junit.jupiter.api.AfterEach; @@ -28,6 +31,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -80,4 +84,24 @@ void testCompleteDescriptor() throws Exception { assertEquals(Arrays.asList("a", "b", "c"), ed.getExportedPackages()); assertEquals(Arrays.asList("x", "y", "z"), ed.getExportedArtifacts()); } + + @Test + void testExternalEntityIsNotResolved() throws Exception { + Path secret = Files.createTempFile("extension-xxe", ".txt"); + Files.writeString(secret, "TOPSECRET"); + try { + String xml = "<?xml version='1.0'?>\n" + "<!DOCTYPE extension [ <!ENTITY xxe SYSTEM \"" + + secret.toUri() + "\"> ]>\n" + + "<extension><exportedPackages><exportedPackage>&xxe;</exportedPackage></exportedPackages></extension>"; + + try { + ExtensionDescriptor ed = builder.build(toStream(xml)); + assertFalse(ed.getExportedPackages().contains("TOPSECRET")); + } catch (IOException expected) { + // doctype / external entities rejected at parse time + } + } finally { + Files.deleteIfExists(secret); + } + } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java index e9ac14d848..49324026ce 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java @@ -47,6 +47,7 @@ import org.apache.maven.api.services.xml.XmlReaderRequest; import org.apache.maven.api.services.xml.XmlWriterException; import org.apache.maven.api.services.xml.XmlWriterRequest; +import org.apache.maven.api.xml.XmlService; import org.apache.maven.model.v4.MavenStaxReader; import org.apache.maven.model.v4.MavenStaxWriter; @@ -191,7 +192,7 @@ static class InputFactoryHolder { static final XMLInputFactory XML_INPUT_FACTORY; static { - XMLInputFactory factory = XMLInputFactory.newFactory(); + XMLInputFactory factory = XmlService.newXMLInputFactory(); factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, true); factory.setProperty(XMLInputFactory.IS_COALESCING, true); XML_INPUT_FACTORY = factory; diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/rootlocator/PomXmlRootDetector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/rootlocator/PomXmlRootDetector.java index 506b634253..56151a593f 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/rootlocator/PomXmlRootDetector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/rootlocator/PomXmlRootDetector.java @@ -18,7 +18,6 @@ */ package org.apache.maven.impl.model.rootlocator; -import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -29,6 +28,7 @@ import org.apache.maven.api.di.Named; import org.apache.maven.api.services.model.RootDetector; +import org.apache.maven.api.xml.XmlService; @Named public class PomXmlRootDetector implements RootDetector { @@ -37,7 +37,7 @@ public boolean isRootDirectory(Path dir) { // we're too early to use the modelProcessor ... Path pom = dir.resolve("pom.xml"); try (InputStream is = Files.newInputStream(pom)) { - XMLStreamReader parser = XMLInputFactory.newFactory().createXMLStreamReader(is); + XMLStreamReader parser = XmlService.newXMLInputFactory().createXMLStreamReader(is); if (parser.nextTag() == XMLStreamReader.START_ELEMENT && parser.getLocalName().equals("project")) { for (int i = 0; i < parser.getAttributeCount(); i++) { diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java index 085bf46806..90c31ed7bb 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java @@ -18,7 +18,6 @@ */ package org.apache.maven.internal.xml; -import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -53,7 +52,7 @@ public class DefaultXmlService extends XmlService { @Override public XmlNode doRead(InputStream input, @Nullable XmlService.InputLocationBuilder locationBuilder) throws XMLStreamException { - XMLStreamReader parser = XMLInputFactory.newFactory().createXMLStreamReader(input); + XMLStreamReader parser = XmlService.newXMLInputFactory().createXMLStreamReader(input); return doRead(parser, locationBuilder); } @@ -61,7 +60,7 @@ public XmlNode doRead(InputStream input, @Nullable XmlService.InputLocationBuild @Override public XmlNode doRead(Reader reader, @Nullable XmlService.InputLocationBuilder locationBuilder) throws XMLStreamException { - XMLStreamReader parser = XMLInputFactory.newFactory().createXMLStreamReader(reader); + XMLStreamReader parser = XmlService.newXMLInputFactory().createXMLStreamReader(reader); return doRead(parser, locationBuilder); } diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/XmlNodeStaxBuilder.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/XmlNodeStaxBuilder.java index c346275630..c666afa8fd 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/XmlNodeStaxBuilder.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/XmlNodeStaxBuilder.java @@ -18,7 +18,6 @@ */ package org.apache.maven.internal.xml; -import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; @@ -40,12 +39,12 @@ public class XmlNodeStaxBuilder { public static XmlNode build(InputStream stream, InputLocationBuilderStax locationBuilder) throws XMLStreamException { - XMLStreamReader parser = XMLInputFactory.newFactory().createXMLStreamReader(stream); + XMLStreamReader parser = XmlService.newXMLInputFactory().createXMLStreamReader(stream); return build(parser, DEFAULT_TRIM, locationBuilder); } public static XmlNode build(Reader reader, InputLocationBuilderStax locationBuilder) throws XMLStreamException { - XMLStreamReader parser = XMLInputFactory.newFactory().createXMLStreamReader(reader); + XMLStreamReader parser = XmlService.newXMLInputFactory().createXMLStreamReader(reader); return build(parser, DEFAULT_TRIM, locationBuilder); } diff --git a/src/mdo/reader-stax.vm b/src/mdo/reader-stax.vm index 730cbf89b0..7b60ba43c4 100644 --- a/src/mdo/reader-stax.vm +++ b/src/mdo/reader-stax.vm @@ -110,6 +110,8 @@ public class ${className} { static { XMLInputFactory factory = XMLInputFactory.newFactory(); factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); XML_INPUT_FACTORY = factory; } } diff --git a/src/mdo/reader.vm b/src/mdo/reader.vm index dd2150eb66..f9674a677b 100644 --- a/src/mdo/reader.vm +++ b/src/mdo/reader.vm @@ -360,6 +360,8 @@ public class ${className} { public ${root.name} read(Reader reader, boolean strict) throws IOException, XMLStreamException { XMLInputFactory factory = XMLInputFactory.newFactory(); factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); XMLStreamReader parser = null; try { parser = factory.createXMLStreamReader(reader); @@ -394,6 +396,8 @@ public class ${className} { public ${root.name} read(InputStream in, boolean strict) throws IOException, XMLStreamException { XMLInputFactory factory = XMLInputFactory.newFactory(); factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); StreamSource streamSource = new StreamSource(in, null); XMLStreamReader parser = factory.createXMLStreamReader(streamSource); return read(parser, strict); @@ -411,6 +415,8 @@ public class ${className} { public ${root.name} read(InputStream in) throws IOException, XMLStreamException { XMLInputFactory factory = XMLInputFactory.newFactory(); factory.setProperty(XMLInputFactory.IS_REPLACING_ENTITY_REFERENCES, false); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); StreamSource streamSource = new StreamSource(in, null); XMLStreamReader parser = factory.createXMLStreamReader(streamSource); return read(parser,true);
