This is an automated email from the ASF dual-hosted git repository. jacopoc pushed a commit to branch release24.09 in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
commit 23bac922618e647ed3d427d8793e981640475411 Author: Jacopo Cappellato <[email protected]> AuthorDate: Mon Apr 27 16:23:01 2026 +0200 Fixed: Enhance XML processing security by disabling external entity resolution across multiple classes (cherry picked from commit 618de331c19686fb2ea3edcf1db0417a4b1721fc) --- .../ofbiz/accounting/thirdparty/eway/GatewayResponse.java | 4 ++-- .../src/main/java/org/apache/ofbiz/base/util/UtilHtml.java | 2 ++ .../src/main/java/org/apache/ofbiz/base/util/UtilXml.java | 13 ++++++++++++- .../org/apache/ofbiz/base/util/string/UelFunctions.java | 6 ++++++ .../org/apache/ofbiz/base/util/template/XslTransform.java | 4 ++++ .../java/org/apache/ofbiz/entity/util/EntitySaxReader.java | 9 ++++++++- .../main/java/org/apache/ofbiz/minilang/MiniLangUtil.java | 4 ++++ .../main/java/org/apache/ofbiz/security/SecuredUpload.java | 4 ++++ .../main/java/org/apache/ofbiz/service/ModelService.java | 2 ++ .../org/apache/ofbiz/service/engine/SOAPClientEngine.java | 5 ++++- .../org/apache/ofbiz/webapp/event/SOAPEventHandler.java | 10 ++++++++-- .../java/org/apache/ofbiz/webapp/view/ApacheFopWorker.java | 4 ++++ .../java/org/apache/ofbiz/webtools/WebToolsServices.java | 2 ++ 13 files changed, 62 insertions(+), 7 deletions(-) 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 index 27c3ac24ec..8fbbcc62af 100644 --- 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.BigDecimal; 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 class GatewayResponse { 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 index 2d6288496d..f16db4b957 100644 --- 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 final class UtilHtml { */ 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 index 085d1db45e..c6e0440b01 100644 --- 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.Set; 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 final class UtilXml { 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 final class UtilXml { 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 final class UtilXml { 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 final class UtilXml { 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 final class UtilXml { 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 index 273127bac7..e3f355bada 100644 --- 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 @@ -36,6 +36,8 @@ import java.util.Map; import java.util.TimeZone; import javax.el.FunctionMapper; + +import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -367,6 +369,7 @@ public class UelFunctions { 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); @@ -442,6 +445,9 @@ public class UelFunctions { 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 index 20a93092f4..53cba52653 100644 --- 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 final class XslTransform { 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 index 1ae45cdd90..740c91cc87 100644 --- 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.LinkedList; 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 class EntitySaxReader extends DefaultHandler { 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 index bdaef4033a..719802ff5a 100644 --- 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.Locale; 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 final class MiniLangUtil { 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 index 5b711676be..c1a1a20dea 100644 --- 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 java.util.zip.Inflater; 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; @@ -1311,9 +1312,12 @@ public class SecuredUpload { 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 index 8b8ae0bf58..c5fa289c29 100644 --- a/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java +++ b/framework/service/src/main/java/org/apache/ofbiz/service/ModelService.java @@ -58,6 +58,7 @@ import javax.wsdl.extensions.soap.SOAPBinding; 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; @@ -1999,6 +2000,7 @@ public class ModelService extends AbstractMap<String, Object> implements Seriali 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 index ab5e5d1f0d..d3db5eb641 100644 --- 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 @@ public final class SOAPClientEngine extends GenericAsyncEngine { 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 index 1716340bfe..ea7be549b4 100644 --- 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 @@ public class SOAPEventHandler implements EventHandler { } 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 @@ public class SOAPEventHandler implements EventHandler { // 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 index 1056ddf898..0bd0071f0b 100644 --- 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.InputStream; 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 final class ApacheFopWorker { 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 index 84a60eec73..e77f79e810 100644 --- 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.ResourceBundle; 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 class WebToolsServices { 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);
