This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch feat/use-commons-xml in repository https://gitbox.apache.org/repos/asf/commons-scxml.git
commit bb35dc42208e19c37aa83c7e9826594e9b45342a Author: Piotr P. Karwasz <[email protected]> AuthorDate: Mon Aug 31 15:30:31 2026 +0200 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 <[email protected]> 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 ++ .../java/org/apache/commons/scxml2/io/ContentParser.java | 11 +++++++---- src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java | 12 +++++++----- src/main/java/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 55a85d0e..78edf361 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@5595ccaf912efad79be6eef63a5619ff05969be3 # 4.37.6 + 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 4d8aa80b..44e58528 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 b9a248a7..6646daf0 100644 --- a/pom.xml +++ b/pom.xml @@ -139,6 +139,11 @@ </contributors> <dependencies> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-secure-xml</artifactId> + <version>1.0.0-SNAPSHOT</version> + </dependency> <dependency> <groupId>commons-logging</groupId> <artifactId>commons-logging</artifactId> diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 9f77c372..19c23c7a 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 </action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix Apache RAT plugin console warnings.</action> + <action type="fix" dev="pkarwasz">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.</action> + <action type="fix" dev="pkarwasz">ContentParser.parseXml now parses its argument as XML content instead of interpreting it as a URI.</action> <!-- UPDATE --> <action dev="woonsan" type="update" issue="SCXML-284" due-to="Allon Mureinik"> [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 875364ba..eb6880f0 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 @@ package org.apache.commons.scxml2.io; 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.JsonValue; 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 class ContentParser { 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 class ContentParser { 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 52c0cde5..26549781 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.LinkedHashMap; 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.Transition; 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 @@ public final class SCXMLReader { 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 @@ public final class SCXMLReader { // 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 @@ public final class SCXMLReader { // 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 4cf3e4c6..b42a9c0d 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.TextValue; 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 @@ public class SCXMLWriter { 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 @@ public class SCXMLWriter { prettyPrintResult = scxmlResult; } - final TransformerFactory factory = TransformerFactory.newInstance(); + final TransformerFactory factory = SecureTransformerFactory.newInstance(); try { final Transformer transformer = factory.newTransformer(); if (configuration.encoding != null) {
