Prevent XML External Entity injection and DTD expansion (Coverity)

Kitodo-API/src/main/java/org/kitodo/api/externaldatamanagement/XmlResponseHandler.java

@@ -13,7 +13,6 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.lang.reflect.UndeclaredThrowableException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.LinkedList;
@@ -34,6 +33,7 @@
 import org.kitodo.exceptions.CatalogException;
 import org.kitodo.exceptions.ConfigException;
 import org.kitodo.exceptions.NoRecordFoundException;
+import org.kitodo.utils.XMLSecurity;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
@@ -42,17 +42,17 @@
 
 public class XmlResponseHandler {
 
-    private static final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+    private static final DocumentBuilderFactory documentBuilderFactory;
     private static final XMLOutputter xmlOutputter = new XMLOutputter();
     private static final XPath xPath = XPathFactory.newInstance().newXPath();
 
     static {
-        documentBuilderFactory.setNamespaceAware(true);
         try {
-            documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-        } catch (ParserConfigurationException parserConfigurationException) {
-            throw new UndeclaredThrowableException(parserConfigurationException);
+            documentBuilderFactory = XMLSecurity.newDocumentBuilderFactory();
+        } catch (ParserConfigurationException e) {
+            throw new ExceptionInInitializerError(e);
         }
+        documentBuilderFactory.setNamespaceAware(true);
         xmlOutputter.setFormat(Format.getPrettyFormat());
     }
 

Kitodo-API/src/main/java/org/kitodo/utils/XMLSecurity.java

@@ -0,0 +1,155 @@
+/*
+ * (c) Kitodo. Key to digital objects e. V. <contact@kitodo.org>
+ *
+ * This file is part of the Kitodo project.
+ *
+ * It is licensed under GNU General Public License version 3 or later.
+ *
+ * For the full copyright and license information, please read the
+ * GPL3-License.txt file that was distributed with this source code.
+ */
+
+package org.kitodo.utils;
+
+import java.io.InputStream;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.sax.SAXSource;
+import javax.xml.validation.SchemaFactory;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+import org.xml.sax.XMLReader;
+
+/**
+ * Provides factory instances that are hardened against XML External Entity
+ * (XXE) injection and unrestricted document type definitions.
+ */
+public final class XMLSecurity {
+
+    private static final Logger logger = LogManager.getLogger(XMLSecurity.class);
+
+    private static final String DISALLOW_DOCTYPE_DECL = "http://apache.org/xml/features/disallow-doctype-decl";
+    private static final String EXTERNAL_GENERAL_ENTITIES = "http://xml.org/sax/features/external-general-entities";
+    private static final String EXTERNAL_PARAMETER_ENTITIES = "http://xml.org/sax/features/external-parameter-entities";
+
+    private XMLSecurity() {
+    }
+
+    /**
+     * Create and return a DocumentBuilderFactory that rejects DOCTYPE declarations and
+     * external entity resolution.
+     *
+     * @return hardened DocumentBuilderFactory
+     * @throws ParserConfigurationException if a feature cannot be set
+     */
+    public static DocumentBuilderFactory newDocumentBuilderFactory() throws ParserConfigurationException {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature(DISALLOW_DOCTYPE_DECL, true);
+        factory.setFeature(EXTERNAL_GENERAL_ENTITIES, false);
+        factory.setFeature(EXTERNAL_PARAMETER_ENTITIES, false);
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setXIncludeAware(false);
+        factory.setExpandEntityReferences(false);
+        return factory;
+    }
+
+    /**
+     * Create and return a TransformerFactory that restricts access to external DTDs
+     * and stylesheets to prevent XML External Entity (XXE) injection.
+     *
+     * @return hardened TransformerFactory
+     */
+    public static TransformerFactory newTransformerFactory() {
+        TransformerFactory factory = TransformerFactory.newInstance();
+        try {
+            factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+        } catch (IllegalArgumentException e) {
+            logger.warn("Unable to restrict external access on TransformerFactory '{}': {}",
+                    factory.getClass().getName(), e.getMessage());
+        }
+        return factory;
+    }
+
+    /**
+     * Create and return a SchemaFactory that rejects external DTD access to prevent
+     * XML External Entity (XXE) injection during XML validation.
+     *
+     * @return hardened SchemaFactory
+     */
+    public static SchemaFactory newSchemaFactory() {
+        SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+        try {
+            factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        } catch (IllegalArgumentException | SAXNotRecognizedException | SAXNotSupportedException e) {
+            logger.warn("Unable to restrict external access on SchemaFactory '{}': {}",
+                    factory.getClass().getName(), e.getMessage());
+        }
+        return factory;
+    }
+
+    /**
+     * Create and return an XMLInputFactory with DTD support and external entity
+     * resolution disabled to prevent XML External Entity (XXE) injection.
+     *
+     * @return hardened XMLInputFactory
+     */
+    public static XMLInputFactory newXmlInputFactory() {
+        XMLInputFactory factory = XMLInputFactory.newInstance();
+        factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+        factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        return factory;
+    }
+
+    /**
+     * Create and return a SAXParserFactory that rejects DOCTYPE declarations and
+     * external entity resolution to prevent XML External Entity (XXE) injection.
+     *
+     * @return hardened SAXParserFactory
+     */
+    public static SAXParserFactory newSaxParserFactory() {
+        SAXParserFactory factory = SAXParserFactory.newInstance();
+        factory.setValidating(false);
+        factory.setNamespaceAware(true);
+        try {
+            factory.setFeature(DISALLOW_DOCTYPE_DECL, true);
+            factory.setFeature(EXTERNAL_GENERAL_ENTITIES, false);
+            factory.setFeature(EXTERNAL_PARAMETER_ENTITIES, false);
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        } catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) {
+            throw new IllegalStateException("Unable to harden SAXParserFactory", e);
+        }
+        return factory;
+    }
+
+    /**
+     * Create and return a SAXSource that rejects DOCTYPE declarations and external
+     * entity resolution to prevent XML External Entity (XXE) injection during
+     * transformation of the given input stream.
+     *
+     * @param inputStream input stream containing the XML document to transform
+     * @return hardened SAXSource
+     * @throws ParserConfigurationException if a feature cannot be set
+     * @throws SAXException if the SAX parser cannot be created
+     */
+    public static SAXSource newSecureSource(InputStream inputStream) throws ParserConfigurationException, SAXException {
+        SAXParserFactory factory = SAXParserFactory.newInstance();
+        factory.setNamespaceAware(true);
+        factory.setFeature(DISALLOW_DOCTYPE_DECL, true);
+        factory.setFeature(EXTERNAL_GENERAL_ENTITIES, false);
+        factory.setFeature(EXTERNAL_PARAMETER_ENTITIES, false);
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        XMLReader reader = factory.newSAXParser().getXMLReader();
+        return new SAXSource(reader, new InputSource(inputStream));
+    }
+}

Kitodo-DataEditor/src/main/java/org/kitodo/dataeditor/JaxbXmlUtils.java

@@ -26,9 +26,10 @@
 import javax.xml.datatype.DatatypeConfigurationException;
 import javax.xml.datatype.DatatypeFactory;
 import javax.xml.datatype.XMLGregorianCalendar;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
 
@@ -38,6 +39,8 @@
 import org.kitodo.dataformat.metskitodo.KitodoType;
 import org.kitodo.dataformat.metskitodo.MdSecType;
 import org.kitodo.serviceloader.KitodoServiceLoader;
+import org.kitodo.utils.XMLSecurity;
+import org.xml.sax.SAXException;
 
 /**
  * Provides methods for handling jaxb generated java objects and xml files.
@@ -61,15 +64,16 @@ private JaxbXmlUtils() {
      */
     static String transformXmlByXslt(URI xmlFile, URI xslFile) throws TransformerException, IOException {
         FileManagementInterface fileManagementModule = new KitodoServiceLoader<>(FileManagementInterface.class).loadModule();
-        TransformerFactory factory = TransformerFactory.newInstance();
         StreamSource styleSource = new StreamSource(xslFile.getPath());
-        Transformer transformer = factory.newTransformer(styleSource);
+        Transformer transformer = XMLSecurity.newTransformerFactory().newTransformer(styleSource);
         try (InputStream inputStream = fileManagementModule.read(xmlFile);
                 StringWriter stringWriter = new StringWriter()) {
-            StreamSource source = new StreamSource(inputStream);
+            Source source = XMLSecurity.newSecureSource(inputStream);
             StreamResult result = new StreamResult(stringWriter);
             transformer.transform(source, result);
             return stringWriter.toString();
+        } catch (ParserConfigurationException | SAXException e) {
+            throw new IOException(e);
         }
     }
 

Kitodo-Docket/src/main/java/org/kitodo/docket/ExportDocket.java

@@ -17,10 +17,11 @@
 import java.io.IOException;
 import java.io.OutputStream;
 
+import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.Result;
+import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.sax.SAXResult;
 import javax.xml.transform.stream.StreamSource;
 
@@ -30,6 +31,8 @@
 import org.apache.fop.apps.FopFactoryBuilder;
 import org.apache.fop.apps.MimeConstants;
 import org.kitodo.api.docket.DocketData;
+import org.kitodo.utils.XMLSecurity;
+import org.xml.sax.SAXException;
 
 /**
  * This class provides generating a run note based on the generated xml log.
@@ -90,22 +93,26 @@ void startExport(Iterable<DocketData> docketDataList, OutputStream os) throws IO
 
     private byte[] generatePdfBytes(ByteArrayOutputStream out) throws IOException {
         // generate pdf file
-        StreamSource source = new StreamSource(new ByteArrayInputStream(out.toByteArray()));
         StreamSource transformSource = new StreamSource(xsltFile);
         FopFactoryBuilder builder = new FopFactoryBuilder(new File(".").toURI());
         builder.setStrictFOValidation(false);
         FopFactory fopFactory = builder.build();
         ByteArrayOutputStream outStream = new ByteArrayOutputStream();
         // transform xml
         try {
-            Transformer xslTransformer = TransformerFactory.newInstance().newTransformer(transformSource);
+            Transformer xslTransformer = XMLSecurity.newTransformerFactory().newTransformer(transformSource);
             Fop fop = fopFactory.newFop(MimeConstants.MIME_PDF, outStream);
             Result res = new SAXResult(fop.getDefaultHandler());
+            Source source = XMLSecurity.newSecureSource(new ByteArrayInputStream(out.toByteArray()));
             xslTransformer.transform(source, res);
         } catch (FOPException e) {
             throw new IOException("FOPException occurred", e);
         } catch (TransformerException e) {
             throw new IOException("TransformerException occurred", e);
+        } catch (ParserConfigurationException e) {
+            throw new IOException("ParserConfigurationException occurred", e);
+        } catch (SAXException e) {
+            throw new IOException("SAXException occurred", e);
         }
 
         // write the content to output stream

Kitodo-Query-URL-Import/src/main/java/org/kitodo/queryurlimport/QueryURLImport.java

@@ -33,13 +33,10 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
-import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 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;
 
@@ -76,6 +73,7 @@
 import org.kitodo.exceptions.CatalogException;
 import org.kitodo.exceptions.ConfigException;
 import org.kitodo.exceptions.NoRecordFoundException;
+import org.kitodo.utils.XMLSecurity;
 import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
@@ -428,16 +426,13 @@ private String createSearchFieldString(SearchInterfaceType interfaceType, Linked
 
     private Document stringToDocument(String xmlContent) throws ParserConfigurationException, IOException,
             SAXException {
-        DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
-        documentBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-        DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder();
+        DocumentBuilder documentBuilder = XMLSecurity.newDocumentBuilderFactory().newDocumentBuilder();
         return documentBuilder.parse(new InputSource(new StringReader(xmlContent)));
     }
 
     private String nodeToString(Node node) throws TransformerException {
         StringWriter writer = new StringWriter();
-        Transformer transformer = TransformerFactory.newInstance().newTransformer();
+        Transformer transformer = XMLSecurity.newTransformerFactory().newTransformer();
         transformer.transform(new DOMSource(node), new StreamResult(writer));
         return writer.toString();
     }

Kitodo-Validation/src/main/java/org/kitodo/validation/filestructure/FileStructureValidation.java

@@ -32,6 +32,7 @@
 import org.kitodo.api.validation.State;
 import org.kitodo.api.validation.ValidationResult;
 import org.kitodo.api.validation.filestructure.FileStructureValidationInterface;
+import org.kitodo.utils.XMLSecurity;
 import org.xml.sax.SAXException;
 import org.xml.sax.SAXParseException;
 
@@ -87,13 +88,18 @@ private ValidationResult validateStreamSource(StreamSource source, Validator val
 
     private Validator initializeXmlValidator(Collection<URI> xsdFilePaths) throws SAXException {
         FileStructureValidationErrorHandler xmlValidationErrorHandler = new FileStructureValidationErrorHandler();
-        SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+        SchemaFactory schemaFactory = XMLSecurity.newSchemaFactory();
         Source[] sources = new Source[xsdFilePaths.size()];
         for (int i = 0; i < xsdFilePaths.size(); i++) {
             sources[i] = new StreamSource(new File(xsdFilePaths.toArray(new URI[0])[i]));
         }
         Schema schema = schemaFactory.newSchema(sources);
         Validator xmlValidator = schema.newValidator();
+        try {
+            xmlValidator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        } catch (IllegalArgumentException e) {
+            logger.warn("Unable to restrict external access on Validator: {}", e.getMessage());
+        }
         xmlValidator.setErrorHandler(xmlValidationErrorHandler);
         return xmlValidator;
     }

Kitodo/src/main/java/org/kitodo/export/ExportMets.java

@@ -53,6 +53,7 @@
 import org.kitodo.production.helper.tasks.EmptyTask;
 import org.kitodo.production.services.ServiceManager;
 import org.kitodo.production.services.file.FileService;
+import org.kitodo.utils.XMLSecurity;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.xml.sax.SAXException;
@@ -248,7 +249,7 @@ private void updateInternalLabelsIfNeeded(URI metaFile, byte[] xmlBytes, Process
     private Map<String, String> extractLabels(byte[] xmlBytes) {
         Map<String, String> labels = new HashMap<>();
         try {
-            DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+            DocumentBuilderFactory factory = XMLSecurity.newDocumentBuilderFactory();
             factory.setNamespaceAware(true);
             Document doc = factory.newDocumentBuilder().parse(new ByteArrayInputStream(xmlBytes));
             XPath xpath = XPathFactory.newInstance().newXPath();