From cb9672c2d2e336e5c51fa91d50ddcda7088583fb Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Mon, 31 Aug 2026 15:30:31 +0200 Subject: [PATCH 1/3] Harden XML parsing via commons-secure-xml Create XML stream readers, parsers, schema factories and transformers 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). - Route factory creation through SecureXMLInputFactory, SecureSchemaFactory and SecureDocumentBuilderFactory in SCXMLReader, and through SecureTransformerFactory in SCXMLWriter and ContentParser. A Configuration-supplied XMLResolver still takes precedence: the floor only handles lookups the resolver leaves unresolved, and the factoryId/factoryClassLoader override still selects the underlying implementation. - ContentParser.parseXml now wraps its argument in an InputSource: DocumentBuilder.parse(String) interprets its argument as a URI, so the method never actually parsed the XML content it was documented to parse. - 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 +- pom.xml | 5 +++++ src/changes/changes.xml | 2 ++ .../org/apache/commons/scxml2/io/ContentParser.java | 11 +++++++---- .../org/apache/commons/scxml2/io/SCXMLReader.java | 12 +++++++----- .../org/apache/commons/scxml2/io/SCXMLWriter.java | 5 +++-- 7 files changed, 27 insertions(+), 12 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 6a0376283..a758ed6bb 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -64,6 +64,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 529756551..ea82e4eac 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -56,4 +56,4 @@ jobs: distribution: 'temurin' java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress + run: mvn --errors --show-version --batch-mode --no-transfer-progress -Puse-apache-snapshots diff --git a/pom.xml b/pom.xml index dc552c616..d4a8b8505 100644 --- a/pom.xml +++ b/pom.xml @@ -139,6 +139,11 @@ + + org.apache.commons + commons-secure-xml + 1.0.0-SNAPSHOT + commons-logging commons-logging diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 07d9edd3a..5503526e5 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -30,6 +30,8 @@ [18-10-2018] Before executing invoke handlers after a macrostep all internal events must have been processed Fix Apache RAT plugin console warnings. + Create XML parsers, stream readers and transformers through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default. + ContentParser.parseXml now parses its argument as XML content instead of interpreting it as a URI. [10-10-2018] Clear up exception handling in tests diff --git a/src/main/java/org/apache/commons/scxml2/io/ContentParser.java b/src/main/java/org/apache/commons/scxml2/io/ContentParser.java index 875364bab..eb6880f0a 100644 --- a/src/main/java/org/apache/commons/scxml2/io/ContentParser.java +++ b/src/main/java/org/apache/commons/scxml2/io/ContentParser.java @@ -18,17 +18,16 @@ import java.io.IOException; import java.io.InputStream; +import java.io.StringReader; import java.io.StringWriter; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Properties; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; -import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; @@ -37,8 +36,11 @@ import org.apache.commons.scxml2.model.NodeValue; import org.apache.commons.scxml2.model.ParsedValue; import org.apache.commons.scxml2.model.TextValue; +import org.apache.commons.xml.secure.SecureDocumentBuilderFactory; +import org.apache.commons.xml.secure.SecureTransformerFactory; import org.w3c.dom.Document; import org.w3c.dom.Node; +import org.xml.sax.InputSource; import org.xml.sax.SAXException; import com.fasterxml.jackson.core.JsonParser; @@ -224,7 +226,8 @@ public ParsedValue parseResource(final String resourceURL) throws IOException { public Node parseXml(final String xmlString) throws IOException { Document doc; try { - doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(xmlString); + // Wrap in an InputSource: DocumentBuilder.parse(String) would interpret the content as a URI. + doc = SecureDocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new InputSource(new StringReader(xmlString))); } catch (SAXException | ParserConfigurationException e) { throw new IOException(e); } @@ -252,7 +255,7 @@ public String toJson(final Object jsonObject) throws IOException { public String toXml(final Node node) throws IOException { try { final StringWriter writer = new StringWriter(); - final Transformer transformer = TransformerFactory.newInstance().newTransformer(); + final Transformer transformer = SecureTransformerFactory.newInstance().newTransformer(); final Properties outputProps = new Properties(); outputProps.put(OutputKeys.OMIT_XML_DECLARATION, "no"); outputProps.put(OutputKeys.STANDALONE, "no"); diff --git a/src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java b/src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java index 52c0cde5d..265497816 100644 --- a/src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java +++ b/src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java @@ -31,7 +31,6 @@ import java.util.List; import java.util.Map; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.stream.Location; import javax.xml.stream.XMLInputFactory; @@ -98,6 +97,9 @@ import org.apache.commons.scxml2.model.TransitionType; import org.apache.commons.scxml2.model.TransitionalState; import org.apache.commons.scxml2.model.Var; +import org.apache.commons.xml.secure.SecureDocumentBuilderFactory; +import org.apache.commons.xml.secure.SecureSchemaFactory; +import org.apache.commons.xml.secure.SecureXMLInputFactory; import org.w3c.dom.Attr; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -589,9 +591,9 @@ private static XMLStreamReader getReader(final Configuration configuration, fina throws IOException, XMLStreamException { // Instantiate the XMLInputFactory - XMLInputFactory factory = XMLInputFactory.newInstance(); + XMLInputFactory factory = SecureXMLInputFactory.newInstance(); if (configuration.factoryId != null && configuration.factoryClassLoader != null) { - factory = XMLInputFactory.newFactory(configuration.factoryId, configuration.factoryClassLoader); + factory = SecureXMLInputFactory.newFactory(configuration.factoryId, configuration.factoryClassLoader); } factory.setEventAllocator(configuration.allocator); if (factory.isPropertySupported(XMLInputFactory_JDK_PROP_REPORT_CDATA)) { @@ -623,7 +625,7 @@ private static XMLStreamReader getReader(final Configuration configuration, fina // Validation requires us to use a Source final URL scxmlSchema = new URL("TODO"); // TODO, point to appropriate location - final SchemaFactory schemaFactory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema"); + final SchemaFactory schemaFactory = SecureSchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema"); Schema schema; try { schema = schemaFactory.newSchema(scxmlSchema); @@ -1341,7 +1343,7 @@ private static Element readElement(final XMLStreamReader reader) // Create a document in which to build the DOM node Document document; try { - document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument(); + document = SecureDocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument(); } catch (final ParserConfigurationException pce) { throw new XMLStreamException(ERR_PARSER_CFG); } diff --git a/src/main/java/org/apache/commons/scxml2/io/SCXMLWriter.java b/src/main/java/org/apache/commons/scxml2/io/SCXMLWriter.java index 4cf3e4c6d..b42a9c0d8 100644 --- a/src/main/java/org/apache/commons/scxml2/io/SCXMLWriter.java +++ b/src/main/java/org/apache/commons/scxml2/io/SCXMLWriter.java @@ -81,6 +81,7 @@ import org.apache.commons.scxml2.model.Transition; import org.apache.commons.scxml2.model.TransitionTarget; import org.apache.commons.scxml2.model.Var; +import org.apache.commons.xml.secure.SecureTransformerFactory; import org.w3c.dom.Node; /** @@ -339,7 +340,7 @@ private static Transformer getTransformer() { outputProps.put(OutputKeys.STANDALONE, "no"); outputProps.put(OutputKeys.INDENT, "yes"); try { - final TransformerFactory tfFactory = TransformerFactory.newInstance(); + final TransformerFactory tfFactory = SecureTransformerFactory.newInstance(); transformer = tfFactory.newTransformer(); transformer.setOutputProperties(outputProps); } catch (TransformerFactoryConfigurationError | TransformerConfigurationException t) { @@ -1130,7 +1131,7 @@ private static void writePretty(final Configuration configuration, final OutputS prettyPrintResult = scxmlResult; } - final TransformerFactory factory = TransformerFactory.newInstance(); + final TransformerFactory factory = SecureTransformerFactory.newInstance(); try { final Transformer transformer = factory.newTransformer(); if (configuration.encoding != null) { From d48f14424e864a2d1535ccec16350ce8aa683ee4 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Thu, 3 Sep 2026 07:04:09 +0200 Subject: [PATCH 2/3] Use the Commons Secure XML 1.0.0 release candidate Bump org.apache.commons:commons-secure-xml from 1.0.0-SNAPSHOT to 1.0.0 and add the temporary staging repository https://repository.apache.org/content/repositories/orgapachecommons-1962/ after Central, so the vote gets downstream CI results. Drop the -Puse-apache-snapshots profile from the CI workflows, which the release version no longer needs. Remove the staging repository once 1.0.0 is released. Assisted-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0167e29ScPEdfzJnEFm95imK --- .github/workflows/codeql-analysis.yml | 2 -- .github/workflows/maven.yml | 2 +- pom.xml | 23 ++++++++++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index a758ed6bb..6a0376283 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -64,8 +64,6 @@ 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 ea82e4eac..529756551 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -56,4 +56,4 @@ jobs: distribution: 'temurin' java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress -Puse-apache-snapshots + run: mvn --errors --show-version --batch-mode --no-transfer-progress diff --git a/pom.xml b/pom.xml index d4a8b8505..84f6f86dc 100644 --- a/pom.xml +++ b/pom.xml @@ -138,11 +138,32 @@ + + + + central + Central Repository + https://repo.maven.apache.org/maven2 + + false + + + + + apache.commons.staging + Apache Commons Secure XML 1.0.0 release candidate + https://repository.apache.org/content/repositories/orgapachecommons-1962/ + + false + + + + org.apache.commons commons-secure-xml - 1.0.0-SNAPSHOT + 1.0.0 commons-logging From 27e4b6971b000b8b88a8049bd21019f7fcb58571 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sun, 6 Sep 2026 08:39:37 -0400 Subject: [PATCH 3/3] Bump Apache Commons Secure XML from 1.0.0-SNAPSHOT to 1.0.0 --- pom.xml | 21 --------------------- src/changes/changes.xml | 2 +- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/pom.xml b/pom.xml index 84f6f86dc..ff3c2d526 100644 --- a/pom.xml +++ b/pom.xml @@ -138,27 +138,6 @@ - - - - central - Central Repository - https://repo.maven.apache.org/maven2 - - false - - - - - apache.commons.staging - Apache Commons Secure XML 1.0.0 release candidate - https://repository.apache.org/content/repositories/orgapachecommons-1962/ - - false - - - - org.apache.commons diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 5503526e5..3b5d27dd6 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -30,7 +30,7 @@ [18-10-2018] Before executing invoke handlers after a macrostep all internal events must have been processed Fix Apache RAT plugin console warnings. - Create XML parsers, stream readers and transformers through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default. + Create XML parsers, stream readers and transformers through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default. ContentParser.parseXml now parses its argument as XML content instead of interpreting it as a URI.