From ccd7f6f39792938c1e8753977b7e0fd34ea83e6a Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Mon, 31 Aug 2026 15:32:40 +0200 Subject: [PATCH] Harden XML parsing via commons-secure-xml Create SAX parser and document builder factories through org.apache.commons:commons-secure-xml. The secure factories enable FEATURE_SECURE_PROCESSING and install a non-removable entity-resolver floor on every parser they produce: external DTD, entity, schema and XInclude lookups that a caller-set resolver does not resolve are resolved to empty content instead of being fetched, and internal entity expansion is bounded, regardless of the JAXP implementation on the classpath. Changes: - Add the commons-secure-xml dependency (1.0.0-SNAPSHOT until its first release) to commons-digester3-core. - Route factory creation through SecureSAXParserFactory in Digester.getFactory() and DigesterLoader, and through SecureDocumentBuilderFactory in NodeCreateRule. The caller-facing configuration (namespace and XInclude awareness, validation, schema, features and properties) keeps working, and getFactory() still returns a plain SAXParserFactory. - Digester itself acts as the entity resolver of the readers it creates, and resolvers installed by the caller keep precedence over the floor, so registered entities, relative DTDs and DTD validation resolve exactly as before; the floor only takes effect for parsers whose resolver chain leaves a lookup unresolved. - Callers supplying their own SAXParser or XMLReader (constructors and DigesterLoader.newDigester overloads) keep full control of their parser configuration. - Run the CI and CodeQL builds with -Puse-apache-snapshots (inherited from the org.apache:apache parent POM) so the commons-secure-xml SNAPSHOT resolves; CodeQL's autobuild receives the profile through MAVEN_ARGS. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MHgnMnGWHQoH2zD2jFdoMT --- .github/workflows/codeql-analysis.yml | 2 ++ .github/workflows/maven.yml | 2 +- commons-digester3-core/pom.xml | 5 +++++ .../src/main/java/org/apache/commons/digester3/Digester.java | 3 ++- .../java/org/apache/commons/digester3/NodeCreateRule.java | 3 ++- .../org/apache/commons/digester3/binder/DigesterLoader.java | 3 ++- src/changes/changes.xml | 3 +++ 7 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index c3d9b7d29..dc0f79327 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -68,6 +68,8 @@ jobs: # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild uses: github/codeql-action/autobuild@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + env: + MAVEN_ARGS: -Puse-apache-snapshots # ℹī¸ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index c42810878..826dc7c9d 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -54,4 +54,4 @@ jobs: distribution: 'temurin' java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress -Ddoclint=none + run: mvn --errors --show-version --batch-mode --no-transfer-progress -Ddoclint=none -Puse-apache-snapshots diff --git a/commons-digester3-core/pom.xml b/commons-digester3-core/pom.xml index 763cd337c..9947a09be 100644 --- a/commons-digester3-core/pom.xml +++ b/commons-digester3-core/pom.xml @@ -34,6 +34,11 @@ Apache Commons Digester :: Core + + org.apache.commons + commons-secure-xml + 1.0.0-SNAPSHOT + cglib cglib diff --git a/commons-digester3-core/src/main/java/org/apache/commons/digester3/Digester.java b/commons-digester3-core/src/main/java/org/apache/commons/digester3/Digester.java index 5aa17f36f..dc8a7781a 100644 --- a/commons-digester3-core/src/main/java/org/apache/commons/digester3/Digester.java +++ b/commons-digester3-core/src/main/java/org/apache/commons/digester3/Digester.java @@ -48,6 +48,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.xml.sax.Attributes; import org.xml.sax.ContentHandler; import org.xml.sax.EntityResolver; @@ -1589,7 +1590,7 @@ public SAXParserFactory getFactory() { if ( factory == null ) { - factory = SAXParserFactory.newInstance(); + factory = SecureSAXParserFactory.newInstance(); factory.setNamespaceAware( namespaceAware ); factory.setXIncludeAware( xincludeAware ); factory.setValidating( validating ); diff --git a/commons-digester3-core/src/main/java/org/apache/commons/digester3/NodeCreateRule.java b/commons-digester3-core/src/main/java/org/apache/commons/digester3/NodeCreateRule.java index 87bd7ef21..7882fab47 100644 --- a/commons-digester3-core/src/main/java/org/apache/commons/digester3/NodeCreateRule.java +++ b/commons-digester3-core/src/main/java/org/apache/commons/digester3/NodeCreateRule.java @@ -23,6 +23,7 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.commons.xml.secure.SecureDocumentBuilderFactory; import org.w3c.dom.Attr; import org.w3c.dom.DOMException; import org.w3c.dom.Document; @@ -335,7 +336,7 @@ public NodeCreateRule( final DocumentBuilder documentBuilder ) public NodeCreateRule( final int nodeType ) throws ParserConfigurationException { - this( nodeType, DocumentBuilderFactory.newInstance().newDocumentBuilder() ); + this( nodeType, SecureDocumentBuilderFactory.newInstance().newDocumentBuilder() ); } /** diff --git a/commons-digester3-core/src/main/java/org/apache/commons/digester3/binder/DigesterLoader.java b/commons-digester3-core/src/main/java/org/apache/commons/digester3/binder/DigesterLoader.java index 0fa0c2069..8cf5cd20e 100644 --- a/commons-digester3-core/src/main/java/org/apache/commons/digester3/binder/DigesterLoader.java +++ b/commons-digester3-core/src/main/java/org/apache/commons/digester3/binder/DigesterLoader.java @@ -43,6 +43,7 @@ import org.apache.commons.digester3.RulesBase; import org.apache.commons.digester3.StackAction; import org.apache.commons.digester3.Substitutor; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.xml.sax.EntityResolver; import org.xml.sax.ErrorHandler; import org.xml.sax.Locator; @@ -107,7 +108,7 @@ public static DigesterLoader newLoader( final RulesModule... rulesModules ) /** * The SAXParserFactory to create new default {@link Digester} instances. */ - private final SAXParserFactory factory = SAXParserFactory.newInstance(); + private final SAXParserFactory factory = SecureSAXParserFactory.newInstance(); private final Iterable rulesModules; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 241d9c42e..303c6de7c 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -27,6 +27,9 @@ Add DefaultThrowingErrorHandler to Digester API. Add Maven property project.build.outputTimestamp for build reproducibility. + + Create SAX parser and document builder factories through org.apache.commons:commons-secure-xml, which bounds entity expansion and enables XML secure processing. + Regression: DigesterTestCase#testPopNamedStackNotPushed expects EmptyStackException.