Alon Bar-Lev has uploaded a new change for review. Change subject: core: fix CVE-2014-3573 ......................................................................
core: fix CVE-2014-3573 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 M backend/manager/modules/restapi/interface/definition/pom.xml M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java M backend/manager/modules/restapi/interface/definition/src/main/modules/org/ovirt/engine/api/restapi-definition/main/module.xml 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 A backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/xml/SecureDocumentBuilderFactory.java 7 files changed, 36 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/63/32563/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..4dfa01e 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.uutils.xml.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/restapi/interface/definition/pom.xml b/backend/manager/modules/restapi/interface/definition/pom.xml index 82497d0..517ea60 100644 --- a/backend/manager/modules/restapi/interface/definition/pom.xml +++ b/backend/manager/modules/restapi/interface/definition/pom.xml @@ -40,6 +40,12 @@ <dependency> <groupId>org.ovirt.engine.core</groupId> + <artifactId>uutils</artifactId> + <version>${engine.version}</version> + </dependency> + + <dependency> + <groupId>org.ovirt.engine.core</groupId> <artifactId>common</artifactId> <version>${engine.version}</version> </dependency> diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java index 806faa6..06715b4 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/rsdl/RsdlManager.java @@ -9,7 +9,6 @@ import javax.xml.bind.JAXB; import javax.xml.bind.JAXBElement; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; @@ -20,6 +19,7 @@ import org.ovirt.engine.api.model.RSDL; import org.ovirt.engine.api.utils.ApiRootLinksCreator; import org.ovirt.engine.core.common.mode.ApplicationMode; +import org.ovirt.engine.core.uutils.xml.SecureDocumentBuilderFactory; import org.w3c.dom.Document; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -94,7 +94,7 @@ // as parameter: Document document; try { - DocumentBuilder parser = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + DocumentBuilder parser = SecureDocumentBuilderFactory.newDocumentBuilderFactory().newDocumentBuilder(); try (InputStream in = RsdlIOManager.loadAsStream(fileName)) { document = parser.parse(in); } diff --git a/backend/manager/modules/restapi/interface/definition/src/main/modules/org/ovirt/engine/api/restapi-definition/main/module.xml b/backend/manager/modules/restapi/interface/definition/src/main/modules/org/ovirt/engine/api/restapi-definition/main/module.xml index aa8d01f..a69455f 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/modules/org/ovirt/engine/api/restapi-definition/main/module.xml +++ b/backend/manager/modules/restapi/interface/definition/src/main/modules/org/ovirt/engine/api/restapi-definition/main/module.xml @@ -34,6 +34,7 @@ <module name="org.codehaus.jackson.jackson-mapper-asl"/> <module name="org.codehaus.jackson.jackson-xc"/> <module name="org.ovirt.engine.core.common"/> + <module name="org.ovirt.engine.core.uutils"/> <module name="org.slf4j"/> <module name="org.yaml.snakeyaml"/> </dependencies> 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..f872a7f 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 @@ -7,6 +7,7 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.ovirt.engine.core.uutils.xml.SecureDocumentBuilderFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.xml.sax.InputSource; @@ -23,7 +24,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..8d61549 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.uutils.xml.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))); diff --git a/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/xml/SecureDocumentBuilderFactory.java b/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/xml/SecureDocumentBuilderFactory.java new file mode 100644 index 0000000..d628229 --- /dev/null +++ b/backend/manager/modules/uutils/src/main/java/org/ovirt/engine/core/uutils/xml/SecureDocumentBuilderFactory.java @@ -0,0 +1,21 @@ +package org.ovirt.engine.core.uutils.xml; + +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; + } + +} -- To view, visit http://gerrit.ovirt.org/32563 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icf27db1ec13b6a16d9b7c77fd9710e8e6f6ec3c9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
