Alon Bar-Lev has uploaded a new change for review. Change subject: utils: add SecureDocumentBuilderFactory ......................................................................
utils: add SecureDocumentBuilderFactory single place in which DocumentBuilderFactory is constructed to apply security settings. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1139000 Change-Id: Icf27db1ec13b6a16d9b7c77fd9710e8e6f6ec3c9 Signed-off-by: Alon Bar-Lev <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/SecureDocumentBuilderFactory.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/XmlUtils.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/xml/XmlDocument.java 4 files changed, 26 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/22/32622/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java index f254f06..d603e42 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java @@ -1,7 +1,6 @@ package org.ovirt.engine.core.bll; import java.io.StringReader; -import javax.xml.parsers.DocumentBuilderFactory; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.AuditLogType; @@ -10,6 +9,7 @@ import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; +import org.ovirt.engine.core.utils.SecureDocumentBuilderFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.InputSource; @@ -91,7 +91,7 @@ boolean error = false; Document doc = null; try { - doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new InputSource(new StringReader(message))); + doc = SecureDocumentBuilderFactory.newDocumentBuilderFactory().newDocumentBuilder().parse(new InputSource(new StringReader(message))); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/SecureDocumentBuilderFactory.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/SecureDocumentBuilderFactory.java new file mode 100644 index 0000000..0530ccc --- /dev/null +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/SecureDocumentBuilderFactory.java @@ -0,0 +1,21 @@ +package org.ovirt.engine.core.utils; + +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +public class SecureDocumentBuilderFactory { + + public static DocumentBuilderFactory newDocumentBuilderFactory() { + DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); + documentBuilderFactory.setExpandEntityReferences(false); + try { + documentBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl";, true); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities";, false); + documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities";, false); + } catch(ParserConfigurationException e) { + throw new RuntimeException(e); + } + return documentBuilderFactory; + } + +} diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/XmlUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/XmlUtils.java index ad12f4c..11f4062 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/XmlUtils.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/XmlUtils.java @@ -23,7 +23,7 @@ * @throws IOException */ public static Document loadXmlDoc(String xmlString) throws ParserConfigurationException, SAXException, IOException { - DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory docBuilderFactory = SecureDocumentBuilderFactory.newDocumentBuilderFactory(); DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder(); InputSource is = new InputSource(new StringReader(xmlString)); Document doc = docBuilder.parse(is); diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/xml/XmlDocument.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/xml/XmlDocument.java index e0a09fe..59cf8ac 100644 --- a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/xml/XmlDocument.java +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/xml/XmlDocument.java @@ -9,6 +9,7 @@ import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; +import org.ovirt.engine.core.utils.SecureDocumentBuilderFactory; import org.w3c.dom.Document; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -30,7 +31,7 @@ private void LoadXml(String ovfstring) throws Exception { // load doc - DocumentBuilderFactory fact = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory fact = SecureDocumentBuilderFactory.newDocumentBuilderFactory(); fact.setNamespaceAware(true); DocumentBuilder builder = fact.newDocumentBuilder(); doc = builder.parse(new InputSource(new StringReader(ovfstring))); -- To view, visit http://gerrit.ovirt.org/32622 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icf27db1ec13b6a16d9b7c77fd9710e8e6f6ec3c9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
