You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opennlp.apache.org by ma...@apache.org on 2026/09/17 14:09:36 UTC
(opennlp) branch main updated: OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation (#1306)
This is an automated email from the ASF dual-hosted git repository.
mawiesne pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opennlp.git
The following commit(s) were added to refs/heads/main by this push:
new a5689758e OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation (#1306)
a5689758e is described below
commit a5689758e0617c2b17f5c12c8ea739b7450d2e2f
Author: Piotr P. Karwasz <pi...@github.copernik.eu>
AuthorDate: Thu Sep 17 16:09:31 2026 +0200
OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation (#1306)
* OPENNLP-1957: Use Apache Commons Secure XML for JAXP factory creation
Replace the hand-rolled XXE hardening in `XmlUtil` with the secure-by-default
factories of Apache Commons Secure XML, which enables secure processing and
blocks external DTD, entity, schema and XInclude fetches on every supported
JAXP implementation, including Android.
The public API of `XmlUtil` is unchanged; callers in `opennlp-runtime` and
`opennlp-formats` need no changes. New tests check that external entities
are not resolved through either the DOM or the SAX path, that entity
expansion is bounded, that the SAX parser is namespace-aware and that neither
parser is XInclude-aware.
---
SECURITY.md | 8 +-
.../formats/nkjp/NKJPSegmentationDocument.java | 4 +-
.../tools/formats/nkjp/NKJPTextDocument.java | 4 +-
opennlp-core/opennlp-runtime/pom.xml | 5 +
.../serializer/DictionaryEntryPersistor.java | 5 +-
.../src/main/java/opennlp/tools/util/XmlUtil.java | 177 +++++++++------------
.../tools/util/featuregen/GeneratorFactory.java | 3 +-
.../test/java/opennlp/tools/util/XmlUtilTest.java | 172 +++++++++++++++++++-
.../src/test/java/opennlp/uima/util/CasUtil.java | 18 +--
pom.xml | 29 +++-
10 files changed, 290 insertions(+), 135 deletions(-)
diff --git a/SECURITY.md b/SECURITY.md
index f39f6a6d2..f1cebdcd7 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -129,9 +129,11 @@ Existing hardening in this area includes:
top-level object, so foreign payloads are rejected before they are
materialized. `SvmDoccatModel` installs an allowlist as well, and additionally
bounds graph depth, reference count, and array length.
-- **Hardened XML parsing.** `XmlUtil` enables secure processing and disables
- DOCTYPE declarations and external DTD and schema access, so dictionary and
- descriptor XML cannot pull in external entities.
+- **Hardened XML parsing.**
+ Every XML parser comes from [Apache Commons Secure XML](https://commons.apache.org/proper/commons-secure-xml/threat_model.html),
+ which fetches no external resource
+ and bounds entity expansion,
+ so dictionary and descriptor XML cannot reach the file system or the network.
- **Fail-fast format checks.** Binary formats validate magic numbers and version
fields before consuming the body.
diff --git a/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPSegmentationDocument.java b/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPSegmentationDocument.java
index 2954f6643..a9b35a0c3 100644
--- a/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPSegmentationDocument.java
+++ b/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPSegmentationDocument.java
@@ -29,7 +29,6 @@ import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathExpressionException;
-import javax.xml.xpath.XPathFactory;
import org.w3c.dom.Document;
import org.w3c.dom.NamedNodeMap;
@@ -98,8 +97,7 @@ public class NKJPSegmentationDocument {
DocumentBuilder docBuilder = XmlUtil.createDocumentBuilder();
Document doc = docBuilder.parse(is);
- XPathFactory xPathfactory = XPathFactory.newInstance();
- XPath xpath = xPathfactory.newXPath();
+ XPath xpath = XmlUtil.createXPath();
final XPathExpression SENT_NODES = xpath.compile("/teiCorpus/TEI/text/body/p/s");
final XPathExpression SEG_NODES = xpath.compile("./seg|./choice");
diff --git a/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPTextDocument.java b/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPTextDocument.java
index 205914ebc..ffd17746d 100644
--- a/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPTextDocument.java
+++ b/opennlp-core/opennlp-formats/src/main/java/opennlp/tools/formats/nkjp/NKJPTextDocument.java
@@ -30,7 +30,6 @@ import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathExpressionException;
-import javax.xml.xpath.XPathFactory;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
@@ -77,8 +76,7 @@ public class NKJPTextDocument {
DocumentBuilder docBuilder = XmlUtil.createDocumentBuilder();
Document doc = docBuilder.parse(is);
- XPathFactory xPathfactory = XPathFactory.newInstance();
- XPath xpath = xPathfactory.newXPath();
+ XPath xpath = XmlUtil.createXPath();
final XPathExpression TEXT_NODES_EXAMPLE = xpath.compile("/teiCorpus/TEI/text/group/text");
final XPathExpression TEXT_NODES_SAMPLE = xpath.compile("/teiCorpus/TEI/text");
diff --git a/opennlp-core/opennlp-runtime/pom.xml b/opennlp-core/opennlp-runtime/pom.xml
index 0dec48db8..32e47e0de 100644
--- a/opennlp-core/opennlp-runtime/pom.xml
+++ b/opennlp-core/opennlp-runtime/pom.xml
@@ -47,6 +47,11 @@
<artifactId>slf4j-api</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-secure-xml</artifactId>
+ </dependency>
+
<!-- TEST scoped -->
<dependency>
<groupId>org.junit.jupiter</groupId>
diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/dictionary/serializer/DictionaryEntryPersistor.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/dictionary/serializer/DictionaryEntryPersistor.java
index 199e95ad6..595109dc3 100644
--- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/dictionary/serializer/DictionaryEntryPersistor.java
+++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/dictionary/serializer/DictionaryEntryPersistor.java
@@ -31,6 +31,7 @@ import javax.xml.transform.sax.SAXTransformerFactory;
import javax.xml.transform.sax.TransformerHandler;
import javax.xml.transform.stream.StreamResult;
+import org.apache.commons.xml.secure.SecureTransformerFactory;
import org.xml.sax.ContentHandler;
import org.xml.sax.InputSource;
import org.xml.sax.Locator;
@@ -255,8 +256,8 @@ public class DictionaryEntryPersistor {
public static void serialize(OutputStream out, Iterator<Entry> entries,
boolean casesensitive) throws IOException {
StreamResult streamResult = new StreamResult(out);
- SAXTransformerFactory tf = (SAXTransformerFactory)
- SAXTransformerFactory.newInstance();
+ // Commons Secure XML always returns a SAXTransformerFactory subclass.
+ SAXTransformerFactory tf = (SAXTransformerFactory) SecureTransformerFactory.newInstance();
TransformerHandler hd;
try {
diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/XmlUtil.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/XmlUtil.java
index b7930bb82..d9b26734b 100644
--- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/XmlUtil.java
+++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/XmlUtil.java
@@ -17,22 +17,55 @@
package opennlp.tools.util;
-import javax.xml.XMLConstants;
+import java.io.InputStream;
import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
-import javax.xml.parsers.SAXParserFactory;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.xpath.XPath;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.commons.xml.secure.SecureDocumentBuilderFactory;
+import org.apache.commons.xml.secure.SecureSAXParserFactory;
+import org.apache.commons.xml.secure.SecureXMLInputFactory;
+import org.apache.commons.xml.secure.SecureXPathFactory;
+import org.xml.sax.EntityResolver;
import org.xml.sax.SAXException;
-import org.xml.sax.SAXNotRecognizedException;
-import org.xml.sax.SAXNotSupportedException;
+import opennlp.tools.util.model.UncloseableInputStream;
+
+/**
+ * Creates XML parsers that process untrusted input securely.
+ * <p>
+ * Every factory comes from
+ * <a href="https://commons.apache.org/proper/commons-secure-xml/">Apache Commons Secure XML</a>.
+ * The parsers it creates fetch no external resource unless an {@link EntityResolver}
+ * explicitly allows it, and they bound entity expansion. Its
+ * <a href="https://commons.apache.org/proper/commons-secure-xml/threat_model.html">threat model</a>
+ * lists the settings a caller may still change without weakening these guarantees.
+ *
+ * @since 1.8.2
+ */
public class XmlUtil {
- private static final Logger logger = LoggerFactory.getLogger(XmlUtil.class);
+ /**
+ * Holds the shared StAX factory.
+ * <p>
+ * {@link XMLInputFactory} is thread-safe once configured, and the lazy holder keeps StAX out
+ * of the static initialization of {@link XmlUtil}, so the DOM and SAX helpers keep working on
+ * a platform like Android without a StAX implementation.
+ */
+ private static final class StaxHolder {
+
+ private static final XMLInputFactory FACTORY = createFactory();
+
+ private static XMLInputFactory createFactory() {
+ final XMLInputFactory factory = SecureXMLInputFactory.newInstance();
+ factory.setProperty(XMLInputFactory.IS_COALESCING, Boolean.TRUE);
+ return factory;
+ }
+ }
/**
* Create a new {@link DocumentBuilder} which processes XML securely.
@@ -41,120 +74,54 @@ public class XmlUtil {
* @throws IllegalStateException Thrown if errors occurred creating the builder.
*/
public static DocumentBuilder createDocumentBuilder() {
- final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
- try {
- documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
- } catch (ParserConfigurationException e) {
- /// {@link XMLConstants.FEATURE_SECURE_PROCESSING} is not supported on Android.
- /// See {@link DocumentBuilderFactory#setFeature}
- logger.warn("Failed to enable XMLConstants.FEATURE_SECURE_PROCESSING, it's unsupported on" +
- " this platform.", e);
- }
- setAttributeIfSupported(documentBuilderFactory, XMLConstants.ACCESS_EXTERNAL_DTD, "");
- setAttributeIfSupported(documentBuilderFactory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
- setFeatureIfSupported(documentBuilderFactory,
- "http://apache.org/xml/features/disallow-doctype-decl", true);
- setFeatureIfSupported(documentBuilderFactory,
- "http://xml.org/sax/features/external-general-entities", false);
- setFeatureIfSupported(documentBuilderFactory,
- "http://xml.org/sax/features/external-parameter-entities", false);
- setFeatureIfSupported(documentBuilderFactory,
- "http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
- setXIncludeAwareIfSupported(documentBuilderFactory, false);
- documentBuilderFactory.setExpandEntityReferences(false);
try {
- return documentBuilderFactory.newDocumentBuilder();
+ return SecureDocumentBuilderFactory.newInstance().newDocumentBuilder();
} catch (ParserConfigurationException e) {
- throw new IllegalStateException(e);
+ // Not expected from any known JAXP implementation: the factory is fully configured.
+ throw new IllegalStateException("Failed to create a secure DocumentBuilder.", e);
}
}
/**
- * Create a new {@link SAXParser} which processes XML securely.
+ * Create a new namespace-aware {@link SAXParser} which processes XML securely.
*
* @return A valid {@link SAXParser} instance.
* @throws IllegalStateException Thrown if errors occurred creating the parser.
*/
public static SAXParser createSaxParser() {
- final SAXParserFactory spf = SAXParserFactory.newInstance();
- spf.setNamespaceAware(true);
- setXIncludeAwareIfSupported(spf, false);
- try {
- spf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
- } catch (ParserConfigurationException | SAXException e) {
- /// {@link XMLConstants.FEATURE_SECURE_PROCESSING} is not supported on Android.
- /// See {@link SAXParserFactory#setFeature}
- logger.warn("Failed to enable XMLConstants.FEATURE_SECURE_PROCESSING, it's unsupported on" +
- " this platform.", e);
- }
- setFeatureIfSupported(spf, "http://apache.org/xml/features/disallow-doctype-decl", true);
- setFeatureIfSupported(spf, "http://xml.org/sax/features/external-general-entities", false);
- setFeatureIfSupported(spf, "http://xml.org/sax/features/external-parameter-entities", false);
- setFeatureIfSupported(spf, "http://apache.org/xml/features/nonvalidating/load-external-dtd",
- false);
try {
- final SAXParser parser = spf.newSAXParser();
- setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_DTD, "");
- setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
- return parser;
+ return SecureSAXParserFactory.newNSInstance().newSAXParser();
} catch (ParserConfigurationException | SAXException e) {
- throw new IllegalStateException(e);
+ // Not expected from any known JAXP implementation: the factory is fully configured.
+ throw new IllegalStateException("Failed to create a secure SAXParser.", e);
}
}
- private static void setFeatureIfSupported(DocumentBuilderFactory factory, String name,
- boolean value) {
- try {
- factory.setFeature(name, value);
- } catch (ParserConfigurationException e) {
- logger.warn("Failed to set XML parser feature {}, it's unsupported on this platform.",
- name, e);
- }
- }
-
- private static void setAttributeIfSupported(DocumentBuilderFactory factory, String name,
- Object value) {
- try {
- factory.setAttribute(name, value);
- } catch (IllegalArgumentException e) {
- logger.warn("Failed to set XML parser attribute {}, it's unsupported on this platform.",
- name, e);
- }
- }
-
- private static void setXIncludeAwareIfSupported(DocumentBuilderFactory factory, boolean state) {
- try {
- factory.setXIncludeAware(state);
- } catch (UnsupportedOperationException e) {
- logger.warn("Failed to set XML parser XInclude awareness, it's unsupported on " +
- "this platform.", e);
- }
- }
-
- private static void setPropertyIfSupported(SAXParser parser, String name, Object value) {
- try {
- parser.setProperty(name, value);
- } catch (SAXNotRecognizedException | SAXNotSupportedException e) {
- logger.warn("Failed to set XML parser property {}, it's unsupported on this platform.",
- name, e);
- }
- }
-
- private static void setFeatureIfSupported(SAXParserFactory factory, String name, boolean value) {
- try {
- factory.setFeature(name, value);
- } catch (ParserConfigurationException | SAXException e) {
- logger.warn("Failed to set XML parser feature {}, it's unsupported on this platform.",
- name, e);
- }
+ /**
+ * Create a new {@link XMLStreamReader} which processes XML securely.
+ * <p>
+ * The reader coalesces adjacent text sections.
+ * <p>
+ * The returned reader respects the StAX contract and does <strong>not</strong> close the
+ * underlying input stream, regardless of the implementation on the classpath.
+ *
+ * @param in A valid, open {@link InputStream} of XML.
+ * @return A valid {@link XMLStreamReader} instance positioned before the first event.
+ * @throws XMLStreamException Thrown if the stream does not start a well-formed document.
+ * @since 3.0.0
+ */
+ public static XMLStreamReader createXmlStreamReader(InputStream in) throws XMLStreamException {
+ return StaxHolder.FACTORY.createXMLStreamReader(new UncloseableInputStream(in));
}
- private static void setXIncludeAwareIfSupported(SAXParserFactory factory, boolean state) {
- try {
- factory.setXIncludeAware(state);
- } catch (UnsupportedOperationException e) {
- logger.warn("Failed to set XML parser XInclude awareness, it's unsupported on " +
- "this platform.", e);
- }
+ /**
+ * Create a new {@link XPath} which evaluates expressions securely,
+ * including those given an {@link org.xml.sax.InputSource} to parse.
+ *
+ * @return A valid {@link XPath} instance.
+ * @since 3.0.0
+ */
+ public static XPath createXPath() {
+ return SecureXPathFactory.newInstance().newXPath();
}
}
diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
index 59c81ba75..2bd7b1255 100644
--- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
+++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
@@ -29,7 +29,6 @@ import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathExpressionException;
-import javax.xml.xpath.XPathFactory;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
@@ -292,7 +291,7 @@ public class GeneratorFactory {
final List<Element> elements = new ArrayList<>();
final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
- final XPath xPath = XPathFactory.newInstance().newXPath();
+ final XPath xPath = XmlUtil.createXPath();
NodeList allElements;
try {
final XPathExpression exp = xPath.compile("//*");
diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/XmlUtilTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/XmlUtilTest.java
index e5cd82ae8..ac6e26890 100644
--- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/XmlUtilTest.java
+++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/util/XmlUtilTest.java
@@ -17,18 +17,187 @@
package opennlp.tools.util;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.io.StringReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.stream.XMLStreamConstants;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import org.apache.commons.xml.secure.SecureDocumentBuilderFactory;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.w3c.dom.Document;
import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.DefaultHandler;
public class XmlUtilTest {
+ private static final String SECRET = "s3cr3t-content";
+
+ @TempDir
+ Path tempDir;
+
+ private static InputStream utf8(String xml) {
+ return new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8));
+ }
+
+ private static String readCharacters(XMLStreamReader reader) throws XMLStreamException {
+ StringBuilder text = new StringBuilder();
+ try {
+ while (reader.hasNext()) {
+ if (reader.next() == XMLStreamConstants.CHARACTERS) {
+ text.append(reader.getText());
+ }
+ }
+ } finally {
+ reader.close();
+ }
+ return text.toString();
+ }
+
+ private String externalEntityPayload() throws IOException {
+ Path secret = tempDir.resolve("secret.txt");
+ Files.writeString(secret, SECRET, StandardCharsets.UTF_8);
+ return "<!DOCTYPE root [<!ENTITY xxe SYSTEM \"" + secret.toUri() + "\">]>"
+ + "<root>&xxe;</root>";
+ }
+
+ @Test
+ void testDocumentBuilderDoesNotResolveExternalEntities() throws Exception {
+ DocumentBuilder documentBuilder = XmlUtil.createDocumentBuilder();
+ try {
+ Document document = documentBuilder.parse(
+ new InputSource(new StringReader(externalEntityPayload())));
+ Assertions.assertFalse(document.getDocumentElement().getTextContent().contains(SECRET),
+ "external entity must not be resolved");
+ } catch (SAXParseException e) {
+ // Rejecting the document outright is an acceptable outcome as well.
+ }
+ }
+
+ @Test
+ void testSaxParserDoesNotResolveExternalEntities() throws Exception {
+ XMLReader reader = XmlUtil.createSaxParser().getXMLReader();
+ StringBuilder text = new StringBuilder();
+ reader.setContentHandler(new DefaultHandler() {
+ @Override
+ public void characters(char[] ch, int start, int length) {
+ text.append(ch, start, length);
+ }
+ });
+ try {
+ reader.parse(new InputSource(new StringReader(externalEntityPayload())));
+ Assertions.assertFalse(text.toString().contains(SECRET),
+ "external entity must not be resolved");
+ } catch (SAXParseException e) {
+ // Rejecting the document outright is an acceptable outcome as well.
+ }
+ }
+
+ @Test
+ void testXmlStreamReaderDoesNotResolveExternalEntities() throws Exception {
+ try {
+ String text = readCharacters(XmlUtil.createXmlStreamReader(utf8(externalEntityPayload())));
+ Assertions.assertFalse(text.contains(SECRET), "external entity must not be resolved");
+ } catch (XMLStreamException e) {
+ // Rejecting the document outright is an acceptable outcome as well.
+ }
+ }
+
+ @Test
+ void testXmlStreamReaderExpandsInternalEntities() throws Exception {
+ String payload = "<!DOCTYPE root [<!ENTITY greeting \"hello\">]><root>&greeting;</root>";
+ Assertions.assertEquals("hello",
+ readCharacters(XmlUtil.createXmlStreamReader(utf8(payload))));
+ }
+
+ @Test
+ void testXmlStreamReaderIsCoalescing() throws Exception {
+ XMLStreamReader reader = XmlUtil.createXmlStreamReader(utf8("<root>a<![CDATA[b]]>c</root>"));
+ int characterEvents = 0;
+ String text = null;
+ try {
+ while (reader.hasNext()) {
+ if (reader.next() == XMLStreamConstants.CHARACTERS) {
+ characterEvents++;
+ text = reader.getText();
+ }
+ }
+ } finally {
+ reader.close();
+ }
+ Assertions.assertEquals(1, characterEvents);
+ Assertions.assertEquals("abc", text);
+ }
+
+ @Test
+ void testXmlStreamReaderDoesNotCloseStream() throws Exception {
+ boolean[] closed = new boolean[1];
+ InputStream in = new ByteArrayInputStream("<root/>".getBytes(StandardCharsets.UTF_8)) {
+ @Override
+ public void close() {
+ closed[0] = true;
+ }
+ };
+ XMLStreamReader reader = XmlUtil.createXmlStreamReader(in);
+ while (reader.hasNext()) {
+ reader.next();
+ }
+ reader.close();
+ Assertions.assertFalse(closed[0], "closing the reader must not close the caller's stream");
+ }
+
+ @Test
+ void testSaxParserIsNamespaceAware() throws Exception {
+ XMLReader reader = XmlUtil.createSaxParser().getXMLReader();
+ String[] namespace = new String[1];
+ reader.setContentHandler(new DefaultHandler() {
+ @Override
+ public void startElement(String uri, String localName, String qName,
+ org.xml.sax.Attributes attributes) {
+ namespace[0] = uri;
+ }
+ });
+ reader.parse(new InputSource(new StringReader("<root xmlns=\"urn:test\"/>")));
+ Assertions.assertEquals("urn:test", namespace[0]);
+ }
+
+ @Test
+ void testParsersAreNotXIncludeAware() {
+ Assertions.assertFalse(XmlUtil.createDocumentBuilder().isXIncludeAware());
+ Assertions.assertFalse(XmlUtil.createSaxParser().isXIncludeAware());
+ }
+
+ @Test
+ void testDocumentBuilderRejectsEntityExpansionBomb() {
+ StringBuilder dtd = new StringBuilder("<!DOCTYPE root [<!ENTITY e0 \"lol\">");
+ for (int i = 1; i < 10; i++) {
+ dtd.append("<!ENTITY e").append(i).append(" \"");
+ for (int j = 0; j < 10; j++) {
+ dtd.append("&e").append(i - 1).append(';');
+ }
+ dtd.append("\">");
+ }
+ dtd.append("]>");
+ String payload = dtd + "<root>&e9;</root>";
+ DocumentBuilder documentBuilder = XmlUtil.createDocumentBuilder();
+ Assertions.assertThrows(SAXException.class,
+ () -> documentBuilder.parse(new InputSource(new StringReader(payload))));
+ }
+
@Test
void testCreateDocumentBuilderWithUnsupportedSecurityOptions() throws Exception {
String property = DocumentBuilderFactory.class.getName();
@@ -51,7 +220,8 @@ public class XmlUtilTest {
public static class ThrowingSecurityOptionsDocumentBuilderFactory
extends DocumentBuilderFactory {
- private final DocumentBuilderFactory delegate = DocumentBuilderFactory.newDefaultInstance();
+ private final DocumentBuilderFactory delegate =
+ SecureDocumentBuilderFactory.newDefaultInstance();
@Override
public DocumentBuilder newDocumentBuilder() throws ParserConfigurationException {
diff --git a/opennlp-extensions/opennlp-uima/src/test/java/opennlp/uima/util/CasUtil.java b/opennlp-extensions/opennlp-uima/src/test/java/opennlp/uima/util/CasUtil.java
index 5e2d029bc..dbbda62e5 100644
--- a/opennlp-extensions/opennlp-uima/src/test/java/opennlp/uima/util/CasUtil.java
+++ b/opennlp-extensions/opennlp-uima/src/test/java/opennlp/uima/util/CasUtil.java
@@ -20,9 +20,7 @@ package opennlp.uima.util;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
-import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
-import javax.xml.parsers.SAXParserFactory;
import org.apache.uima.ResourceSpecifierFactory;
import org.apache.uima.UIMAFramework;
@@ -41,6 +39,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xml.sax.SAXException;
+import opennlp.tools.util.XmlUtil;
+
public final class CasUtil {
private static final Logger logger = LoggerFactory.getLogger(CasUtil.class);
@@ -95,19 +95,7 @@ public final class CasUtil {
public static void deserializeXmiCAS(CAS cas, InputStream xmiIn) throws IOException {
- SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
- saxParserFactory.setValidating(false);
-
- SAXParser saxParser;
-
- try {
- saxParser = saxParserFactory.newSAXParser();
- } catch (ParserConfigurationException e) {
- throw new IllegalStateException(
- "SAXParser should be configured correctly!", e);
- } catch (SAXException e) {
- throw new IllegalStateException("SAX error while creating parser!", e);
- }
+ SAXParser saxParser = XmlUtil.createSaxParser();
XmiCasDeserializer dezerializer = new XmiCasDeserializer(
cas.getTypeSystem());
diff --git a/pom.xml b/pom.xml
index 31305ba44..14f4eb48d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -110,6 +110,12 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-secure-xml</artifactId>
+ <version>${commons-secure-xml.version}</version>
+ </dependency>
+
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
@@ -247,6 +253,7 @@
<opennlp.model.v15.base.url>https://opennlp.sourceforge.net/models-1.5/</opennlp.model.v15.base.url>
<!-- Dependency versions -->
+ <commons-secure-xml.version>1.0.0</commons-secure-xml.version>
<junit.version>6.1.3</junit.version>
<junit5-system-exit.version>2.0.3</junit5-system-exit.version>
<onnxruntime.version>1.29.0</onnxruntime.version>
@@ -404,10 +411,30 @@
<bundledSignature>jdk-deprecated-${java.version}</bundledSignature>
<bundledSignature>jdk-non-portable</bundledSignature>
</bundledSignatures>
+ <signatures><![CDATA[
+@defaultMessage Create JAXP factories through opennlp.tools.util.XmlUtil or Apache Commons Secure XML, which secure them against XXE (OPENNLP-1957).
+javax.xml.parsers.DocumentBuilderFactory#newDefaultInstance()
+javax.xml.parsers.DocumentBuilderFactory#newDefaultNSInstance()
+javax.xml.parsers.DocumentBuilderFactory#newInstance(**)
+javax.xml.parsers.DocumentBuilderFactory#newNSInstance(**)
+javax.xml.parsers.SAXParserFactory#newDefaultInstance()
+javax.xml.parsers.SAXParserFactory#newDefaultNSInstance()
+javax.xml.parsers.SAXParserFactory#newInstance(**)
+javax.xml.parsers.SAXParserFactory#newNSInstance(**)
+javax.xml.stream.XMLInputFactory#newDefaultFactory()
+javax.xml.stream.XMLInputFactory#newFactory(**)
+javax.xml.stream.XMLInputFactory#newInstance(**)
+javax.xml.transform.TransformerFactory#newDefaultInstance()
+javax.xml.transform.TransformerFactory#newInstance(**)
+javax.xml.validation.SchemaFactory#newDefaultInstance()
+javax.xml.validation.SchemaFactory#newInstance(**)
+javax.xml.xpath.XPathFactory#newDefaultInstance()
+javax.xml.xpath.XPathFactory#newInstance(**)
+org.xml.sax.helpers.XMLReaderFactory#createXMLReader(**)
+]]></signatures>
</configuration>
<executions>
<execution>
- <phase>validate</phase>
<goals>
<goal>check</goal>
<goal>testCheck</goal>