Fixed: Enhance XML processing security by disabling external entity resolution across multiple classes

jacopoc · jacopoc · commit 618de331c196 · 2026-04-27T17:39:01.000+02:00
diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/eway/GatewayResponse.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/thirdparty/eway/GatewayResponse.java
@@ -23,6 +23,7 @@
 import java.math.RoundingMode;
 import java.util.Locale;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 
@@ -165,14 +166,13 @@ public double getBeagleScore() {
     public GatewayResponse(InputStream xmlstream, GatewayRequest req) throws Exception {
 
         DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance();
-        // Below is an answer to a codeQL action on GH reporting a possible XXE
-        // I have a doubt about "load-external-dtd" feature because I did not test the changes.
         builderFactory.setValidating(true);
         builderFactory.setNamespaceAware(true);
 
         builderFactory.setAttribute("http://xml.org/sax/features/validation", true);
         builderFactory.setAttribute("http://apache.org/xml/features/validation/schema", true);
 
+        builderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
         builderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
         builderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
         builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHtml.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHtml.java
@@ -69,6 +69,8 @@ public static List<ParseError> validateHtmlFragmentWithJSoup(String content) {
      */
     public static List<String> hasUnclosedTag(String content) {
         XMLInputFactory inputFactory = XMLInputFactory.newInstance();
+        inputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
         XMLEventReader eventReader = null;
         List<String> errorList = new ArrayList<>();
         try {
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilXml.java
@@ -37,6 +37,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -226,6 +227,9 @@ public static Transformer createOutputTransformer(String encoding, boolean omitX
         sb.append("</xsl:template>\n</xsl:stylesheet>\n");
         ByteArrayInputStream bis = new ByteArrayInputStream(sb.toString().getBytes());
         TransformerFactory transformerFactory = TransformerFactory.newInstance();
+        transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
         return transformerFactory.newTransformer(new StreamSource(bis));
     }
 
@@ -460,6 +464,7 @@ public static Document readXmlDocument(InputStream is, boolean validate, String
         factory.setAttribute("http://xml.org/sax/features/validation", validate);
         factory.setAttribute("http://apache.org/xml/features/validation/schema", validate);
 
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
         factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
         factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
         factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
@@ -589,6 +594,9 @@ public void ignorableWhitespace(XMLString text, Augmentations augs) throws XNIEx
         parser.setFeature("http://xml.org/sax/features/validation", validate);
         parser.setFeature("http://apache.org/xml/features/validation/schema", validate);
         parser.setFeature("http://apache.org/xml/features/dom/defer-node-expansion", false);
+        parser.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        parser.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        parser.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
 
         // with a SchemaUrl, a URL object
         if (validate) {
@@ -617,9 +625,9 @@ public static Document makeEmptyXmlDocument() {
     public static Document makeEmptyXmlDocument(String rootElementName) {
         Document document = null;
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-
         factory.setValidating(true);
         try {
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             DocumentBuilder builder = factory.newDocumentBuilder();
 
             document = builder.newDocument();
@@ -1273,6 +1281,9 @@ public static String convertDocumentToXmlString(Document document) {
         TransformerFactory tf = TransformerFactory.newInstance();
         Transformer transformer;
         try {
+            tf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            tf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            tf.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
             transformer = tf.newTransformer();
             transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8");
             transformer.setOutputProperty(OutputKeys.INDENT, "yes");
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/string/UelFunctions.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/string/UelFunctions.java
@@ -37,6 +37,7 @@
 
 import jakarta.el.FunctionMapper;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -368,6 +369,7 @@ public static Document readHtmlDocument(String str) {
             URL url = FlexibleLocation.resolveLocation(str);
             if (url != null) {
                 DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+                factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
                 factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
                 factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
                 factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
@@ -443,6 +445,9 @@ public static String toHtmlString(Node node, String encoding, boolean indent, in
             sb.append("</xsl:template>\n</xsl:stylesheet>\n");
             ByteArrayInputStream bis = new ByteArrayInputStream(sb.toString().getBytes(StandardCharsets.UTF_8));
             TransformerFactory transformerFactory = TransformerFactory.newInstance();
+            transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
             try (ByteArrayOutputStream os = new ByteArrayOutputStream();) {
                 UtilXml.transformDomDocument(transformerFactory.newTransformer(new StreamSource(bis)), node, os);
                 return os.toString();
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/template/XslTransform.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/XslTransform.java
@@ -60,6 +60,10 @@ public static String renderTemplate(String template, String data) throws Transfo
             pfactory.setXIncludeAware(true);
             XMLReader reader = null;
             try {
+                pfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+                pfactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+                pfactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+                pfactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
                 reader = pfactory.newSAXParser().getXMLReader();
             } catch (Exception e) {
                 throw new TransformerException("Error creating SAX parser/reader", e);
diff --git a/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntitySaxReader.java b/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntitySaxReader.java
@@ -35,6 +35,7 @@
 import java.util.List;
 import java.util.Map;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
@@ -265,7 +266,13 @@ public long parse(URL location) throws SAXException, java.io.IOException {
     private long parse(InputStream is, String docDescription) throws SAXException, java.io.IOException {
         SAXParser parser;
         try {
-            parser = SAXParserFactory.newInstance().newSAXParser();
+            SAXParserFactory factory = SAXParserFactory.newInstance();
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            factory.setXIncludeAware(false);
+            parser = factory.newSAXParser();
         } catch (ParserConfigurationException pce) {
             throw new SAXException("Unable to create the SAX parser", pce);
         }
diff --git a/framework/minilang/src/main/java/org/apache/ofbiz/minilang/MiniLangUtil.java b/framework/minilang/src/main/java/org/apache/ofbiz/minilang/MiniLangUtil.java
@@ -29,6 +29,7 @@
 import java.util.Set;
 import java.util.TimeZone;
 
+import javax.xml.XMLConstants;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.stream.StreamSource;
@@ -282,6 +283,9 @@ public static void writeMiniLangDocument(URL xmlURL, Document document) {
         try {
             styleSheetURL = FlexibleLocation.resolveLocation("component://minilang/config/MiniLang.xslt");
             TransformerFactory transformerFactory = TransformerFactory.newInstance();
+            transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
             try (InputStream styleSheetInStream = styleSheetURL.openStream()) {
                 transformer = transformerFactory.newTransformer(new StreamSource(styleSheetInStream));
             }
diff --git a/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java b/framework/security/src/main/java/org/apache/ofbiz/security/SecuredUpload.java
@@ -61,6 +61,7 @@
 import javax.imageio.ImageIO;
 import javax.imageio.ImageReader;
 import javax.imageio.stream.ImageInputStream;
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -1332,9 +1333,12 @@ private static boolean isSafeSvgContent(String fileName) {
         try {
             DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
             // Harden against XXE and DOCTYPE-based injection attacks
+            dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
             dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
             dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            dbf.setXIncludeAware(false);
             dbf.setExpandEntityReferences(false);
             dbf.setNamespaceAware(true);
             DocumentBuilder db = dbf.newDocumentBuilder();
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java b/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java
@@ -60,6 +60,7 @@
 import javax.wsdl.extensions.soap.SOAPBody;
 import javax.wsdl.extensions.soap.SOAPOperation;
 import javax.wsdl.factory.WSDLFactory;
+import javax.xml.XMLConstants;
 import javax.xml.namespace.QName;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -1993,6 +1994,7 @@ public void getWSDL(Definition def, String locationURI) throws WSDLException {
         DocumentBuilder builder = null;
         Document document = null;
         try {
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
             builder = factory.newDocumentBuilder();
             document = builder.newDocument();
         } catch (Exception e) {
diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/engine/SOAPClientEngine.java b/framework/service/src/main/java/org/apache/ofbiz/service/engine/SOAPClientEngine.java
@@ -136,7 +136,10 @@ private Map<String, Object> serviceInvoker(ModelService modelService, Map<String
 
         try {
             String xmlParameters = SoapSerializer.serialize(parameterMap);
-            XMLStreamReader reader = XMLInputFactory.newInstance().createXMLStreamReader(new StringReader(xmlParameters));
+            XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+            xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+            xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+            XMLStreamReader reader = xmlInputFactory.createXMLStreamReader(new StringReader(xmlParameters));
             OMXMLParserWrapper builder = OMXMLBuilderFactory.createStAXOMBuilder(reader);
             parameterSer = builder.getDocumentElement();
         } catch (Exception e) {
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/SOAPEventHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/SOAPEventHandler.java
@@ -237,7 +237,10 @@ private static void createAndSendSOAPResponse(Map<String, Object> serviceResults
             }
             String xmlResults = SoapSerializer.serialize(serviceResults);
             //Debug.logInfo("xmlResults ==================" + xmlResults, MODULE);
-            XMLStreamReader reader = XMLInputFactory.newInstance().createXMLStreamReader(new StringReader(xmlResults));
+            XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+            xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+            xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+            XMLStreamReader reader = xmlInputFactory.createXMLStreamReader(new StringReader(xmlResults));
             OMXMLParserWrapper resultsBuilder = OMXMLBuilderFactory.createStAXOMBuilder(OMAbstractFactory.getOMFactory(),
                     reader);
             OMElement resultSer = resultsBuilder.getDocumentElement();
@@ -290,7 +293,10 @@ private static void sendError(HttpServletResponse res, Object object, String ser
             // setup the response
             res.setContentType("text/xml");
             String xmlResults = SoapSerializer.serialize(object);
-            XMLStreamReader xmlReader = XMLInputFactory.newInstance().createXMLStreamReader(new StringReader(xmlResults));
+            XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+            xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+            xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+            XMLStreamReader xmlReader = xmlInputFactory.createXMLStreamReader(new StringReader(xmlResults));
             OMXMLParserWrapper resultsBuilder = OMXMLBuilderFactory.createStAXOMBuilder(OMAbstractFactory.getOMFactory(), xmlReader);
             OMElement resultSer = resultsBuilder.getDocumentElement();
 
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/view/ApacheFopWorker.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/view/ApacheFopWorker.java
@@ -26,6 +26,7 @@
 import java.io.OutputStream;
 import java.net.URL;
 
+import javax.xml.XMLConstants;
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
 import javax.xml.transform.Transformer;
@@ -165,6 +166,9 @@ public static void transform(StreamSource src, StreamSource stylesheet, Fop fop)
         Result res = new SAXResult(fop.getDefaultHandler());
         try {
             TransformerFactory factory = TransformerFactory.newInstance();
+            factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+            factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
             Transformer transformer;
             if (stylesheet == null) {
                 transformer = factory.newTransformer();
diff --git a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
@@ -44,6 +44,7 @@
 import java.util.Set;
 import java.util.TreeSet;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -166,6 +167,7 @@ public static Map<String, Object> entityImport(DispatchContext dctx, Map<String,
                 factory.setAttribute("http://xml.org/sax/features/validation", true);
                 factory.setAttribute("http://apache.org/xml/features/validation/schema", true);
 
+                factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
                 factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
                 factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
                 factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
