From 441edf3ca722c778233c6ce169fd47cf1b87cb53 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 27 Jul 2022 14:31:51 +0530 Subject: [PATCH] utils: use safer parsing utility across codebase (#6562) This addresses SonarQube/SonarCloud quality checks to use safer xml parser to resist potential XXE attacks. https://sonarcloud.io/organizations/apache/rules?open=java%3AS2755&rule_key=java%3AS2755 Signed-off-by: Rohit Yadav --- .../cloud/agent/api/storage/OVFHelper.java | 27 ++++----- .../cloud/agent/api/storage/OVFParser.java | 20 ++++--- .../resource/LibvirtComputingResource.java | 12 ++-- .../kvm/resource/LibvirtDomainXMLParser.java | 7 ++- .../resource/LibvirtStoragePoolXMLParser.java | 4 +- .../LibvirtStorageVolumeXMLParser.java | 4 +- .../kvm/resource/LibvirtXMLParser.java | 4 +- .../wrapper/LibvirtMigrateCommandWrapper.java | 15 ++--- .../resource/CitrixResourceBase.java | 6 +- .../cisco/CiscoVnmcConnectionImpl.java | 5 +- .../network/resource/JuniperSrxResource.java | 6 +- .../network/resource/PaloAltoResource.java | 12 ++-- .../GetServiceProviderMetaDataCmd.java | 47 ++++++++-------- .../java/com/cloud/test/DatabaseConfig.java | 54 +++++++++--------- .../java/com/cloud/vm/UserVmManagerImpl.java | 4 +- .../main/java/com/cloud/utils/UriUtils.java | 10 ++-- .../utils/cisco/n1kv/vsm/VsmResponse.java | 5 +- .../utils/xmlobject/XmlObjectParser.java | 4 +- .../utils/security/ParserUtils.java | 31 +++++++++++ .../utils/security/ParserUtilsTest.java | 55 +++++++++++++++++++ .../vmware/mo/HypervisorHostHelper.java | 5 +- 21 files changed, 216 insertions(+), 121 deletions(-) create mode 100644 utils/src/test/java/org/apache/cloudstack/utils/security/ParserUtilsTest.java diff --git a/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java b/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java index f93c3aa5578..c8313ccd664 100644 --- a/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java +++ b/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java @@ -35,19 +35,10 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; -import com.cloud.agent.api.to.deployasis.OVFConfigurationTO; -import com.cloud.agent.api.to.deployasis.OVFEulaSectionTO; -import com.cloud.agent.api.to.deployasis.OVFPropertyTO; -import com.cloud.agent.api.to.deployasis.OVFVirtualHardwareItemTO; -import com.cloud.agent.api.to.deployasis.OVFVirtualHardwareSectionTO; -import com.cloud.configuration.Resource.ResourceType; -import com.cloud.exception.InternalErrorException; -import com.cloud.utils.Pair; -import com.cloud.utils.compression.CompressionUtil; -import com.cloud.agent.api.to.deployasis.OVFNetworkTO; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang.math.NumberUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -56,10 +47,20 @@ import org.w3c.dom.NodeList; import org.w3c.dom.traversal.DocumentTraversal; import org.w3c.dom.traversal.NodeFilter; import org.w3c.dom.traversal.NodeIterator; +import org.xml.sax.SAXException; import com.cloud.agent.api.to.DatadiskTO; +import com.cloud.agent.api.to.deployasis.OVFConfigurationTO; +import com.cloud.agent.api.to.deployasis.OVFEulaSectionTO; +import com.cloud.agent.api.to.deployasis.OVFNetworkTO; +import com.cloud.agent.api.to.deployasis.OVFPropertyTO; +import com.cloud.agent.api.to.deployasis.OVFVirtualHardwareItemTO; +import com.cloud.agent.api.to.deployasis.OVFVirtualHardwareSectionTO; +import com.cloud.configuration.Resource.ResourceType; +import com.cloud.exception.InternalErrorException; +import com.cloud.utils.Pair; +import com.cloud.utils.compression.CompressionUtil; import com.cloud.utils.exception.CloudRuntimeException; -import org.xml.sax.SAXException; public class OVFHelper { private static final Logger s_logger = Logger.getLogger(OVFHelper.class); @@ -496,7 +497,7 @@ public class OVFHelper { final StringWriter writer = new StringWriter(); final StreamResult result = new StreamResult(writer); - final TransformerFactory tf = TransformerFactory.newInstance(); + final TransformerFactory tf = ParserUtils.getSaferTransformerFactory(); final Transformer transformer = tf.newTransformer(); final DOMSource domSource = new DOMSource(doc); transformer.transform(domSource, result); diff --git a/api/src/main/java/com/cloud/agent/api/storage/OVFParser.java b/api/src/main/java/com/cloud/agent/api/storage/OVFParser.java index b4ca8161c5d..b66fbe418d7 100644 --- a/api/src/main/java/com/cloud/agent/api/storage/OVFParser.java +++ b/api/src/main/java/com/cloud/agent/api/storage/OVFParser.java @@ -16,6 +16,16 @@ // under the License. package com.cloud.agent.api.storage; +import java.io.File; +import java.io.IOException; +import java.io.StringReader; +import java.util.Map; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.w3c.dom.Document; @@ -25,14 +35,6 @@ import org.w3c.dom.NodeList; import org.xml.sax.InputSource; import org.xml.sax.SAXException; -import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; -import java.io.File; -import java.io.IOException; -import java.io.StringReader; -import java.util.Map; - public class OVFParser { private static final Logger s_logger = Logger.getLogger(OVFParser.class); @@ -47,7 +49,7 @@ public class OVFParser { public OVFParser() { try { - DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory documentBuilderFactory = ParserUtils.getSaferDocumentBuilderFactory(); documentBuilderFactory.setNamespaceAware(true); documentBuilder = documentBuilderFactory.newDocumentBuilder(); } catch (ParserConfigurationException e) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index ba175c44573..db1f06290c9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -43,28 +43,29 @@ import java.util.regex.Pattern; import javax.naming.ConfigurationException; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; -import com.cloud.configuration.Config; import org.apache.cloudstack.storage.configdrive.ConfigDrive; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.hypervisor.HypervisorUtils; import org.apache.cloudstack.utils.linux.CPUStat; import org.apache.cloudstack.utils.linux.KVMHostInfo; import org.apache.cloudstack.utils.linux.MemStat; import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; -import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.apache.cloudstack.utils.security.KeyStoreUtils; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.io.FileUtils; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.BooleanUtils; import org.apache.commons.lang.math.NumberUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.joda.time.Duration; import org.libvirt.Connect; @@ -118,6 +119,7 @@ import com.cloud.agent.properties.AgentPropertiesFileHandler; import com.cloud.agent.resource.virtualnetwork.VRScripts; import com.cloud.agent.resource.virtualnetwork.VirtualRouterDeployer; import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource; +import com.cloud.configuration.Config; import com.cloud.dc.Vlan; import com.cloud.exception.InternalErrorException; import com.cloud.host.Host.Type; @@ -187,8 +189,6 @@ import com.cloud.utils.ssh.SshHelper; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.PowerState; import com.cloud.vm.VmDetailConstants; -import org.apache.commons.lang3.StringUtils; -import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; /** * LibvirtComputingResource execute requests on the computing/routing host using @@ -4597,7 +4597,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv snapshotCurrent.free(); DocumentBuilder builder; try { - builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + builder = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder(); InputSource is = new InputSource(); is.setCharacterStream(new StringReader(snapshotXML)); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java index 15ed16e21b6..f3a177a9e0c 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java @@ -22,10 +22,11 @@ import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; import java.util.List; + import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.w3c.dom.Document; @@ -42,8 +43,8 @@ import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef.NicModel; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.RngDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.RngDef.RngBackendModel; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef; -import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogModel; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogAction; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogModel; public class LibvirtDomainXMLParser { private static final Logger s_logger = Logger.getLogger(LibvirtDomainXMLParser.class); @@ -58,7 +59,7 @@ public class LibvirtDomainXMLParser { public boolean parseDomainXML(String domXML) { DocumentBuilder builder; try { - builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + builder = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder(); InputSource is = new InputSource(); is.setCharacterStream(new StringReader(domXML)); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java index d88d68d54b1..7f0bf8c0bc8 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java @@ -20,9 +20,9 @@ import java.io.IOException; import java.io.StringReader; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.w3c.dom.Document; @@ -38,7 +38,7 @@ public class LibvirtStoragePoolXMLParser { public LibvirtStoragePoolDef parseStoragePoolXML(String poolXML) { DocumentBuilder builder; try { - builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + builder = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder(); InputSource is = new InputSource(); is.setCharacterStream(new StringReader(poolXML)); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java index 1c89f81327c..c4132ca1ba3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtStorageVolumeXMLParser.java @@ -20,9 +20,9 @@ import java.io.IOException; import java.io.StringReader; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.log4j.Logger; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -37,7 +37,7 @@ public class LibvirtStorageVolumeXMLParser { public LibvirtStorageVolumeDef parseStorageVolumeXML(String volXML) { DocumentBuilder builder; try { - builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + builder = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder(); InputSource is = new InputSource(); is.setCharacterStream(new StringReader(volXML)); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java index 9d347816d86..48a379c278f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtXMLParser.java @@ -23,6 +23,7 @@ import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.log4j.Logger; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -32,8 +33,7 @@ public class LibvirtXMLParser extends DefaultHandler { private static final Logger s_logger = Logger.getLogger(LibvirtXMLParser.class); protected static final SAXParserFactory s_spf; static { - s_spf = SAXParserFactory.newInstance(); - + s_spf = ParserUtils.getSaferSAXParserFactory(); } protected SAXParser _sp; protected boolean _initialized = false; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 5c1c99aaabc..de942412333 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -43,16 +43,15 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; -import com.cloud.agent.api.to.DiskTO; -import com.cloud.agent.api.to.DpdkTO; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; -import org.libvirt.DomainJobInfo; import org.libvirt.DomainInfo.DomainState; +import org.libvirt.DomainJobInfo; import org.libvirt.LibvirtException; import org.libvirt.StorageVol; import org.w3c.dom.Document; @@ -66,6 +65,8 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateAnswer; import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo; +import com.cloud.agent.api.to.DiskTO; +import com.cloud.agent.api.to.DpdkTO; import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.agent.properties.AgentProperties; import com.cloud.agent.properties.AgentPropertiesFileHandler; @@ -340,7 +341,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper dpdkPortsMapping) throws TransformerException, ParserConfigurationException, IOException, SAXException { InputStream in = IOUtils.toInputStream(xmlDesc); - DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); Document doc = docBuilder.parse(in); @@ -488,7 +489,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper\n%s\n", decodedUrl))); Document doc = builder.parse(src); diff --git a/utils/src/main/java/com/cloud/utils/UriUtils.java b/utils/src/main/java/com/cloud/utils/UriUtils.java index 62ed1de2803..fd7adf38d54 100644 --- a/utils/src/main/java/com/cloud/utils/UriUtils.java +++ b/utils/src/main/java/com/cloud/utils/UriUtils.java @@ -43,6 +43,7 @@ import java.util.function.Predicate; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.httpclient.Credentials; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpException; @@ -53,6 +54,7 @@ import org.apache.commons.httpclient.auth.AuthScope; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.httpclient.methods.HeadMethod; import org.apache.commons.httpclient.util.URIUtil; +import org.apache.commons.lang3.StringUtils; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URIBuilder; import org.apache.http.client.utils.URLEncodedUtils; @@ -64,12 +66,10 @@ import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; import org.w3c.dom.NodeList; -import org.apache.commons.lang3.StringUtils; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; - import com.cloud.utils.crypt.DBEncryptionUtil; import com.cloud.utils.exception.CloudRuntimeException; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; public class UriUtils { @@ -376,7 +376,7 @@ public class UriUtils { protected static Map> getMultipleValuesFromXML(InputStream is, String[] tagNames) { Map> returnValues = new HashMap>(); try { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory factory = ParserUtils.getSaferDocumentBuilderFactory(); DocumentBuilder docBuilder = factory.newDocumentBuilder(); Document doc = docBuilder.parse(is); Element rootElement = doc.getDocumentElement(); diff --git a/utils/src/main/java/com/cloud/utils/cisco/n1kv/vsm/VsmResponse.java b/utils/src/main/java/com/cloud/utils/cisco/n1kv/vsm/VsmResponse.java index d82b0dcf982..eb7f0246575 100644 --- a/utils/src/main/java/com/cloud/utils/cisco/n1kv/vsm/VsmResponse.java +++ b/utils/src/main/java/com/cloud/utils/cisco/n1kv/vsm/VsmResponse.java @@ -26,6 +26,7 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.log4j.Logger; import org.w3c.dom.DOMException; import org.w3c.dom.Document; @@ -93,7 +94,7 @@ public abstract class VsmResponse { protected void initialize() { try { - DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); docFactory.setNamespaceAware(true); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); _docResponse = docBuilder.parse(new InputSource(new StringReader(_xmlResponse))); @@ -210,7 +211,7 @@ public abstract class VsmResponse { // Helper routine to check for the response received. protected void printResponse() { try { - DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory docFactory = ParserUtils.getSaferDocumentBuilderFactory(); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); DOMImplementationLS ls = (DOMImplementationLS)docBuilder.getDOMImplementation(); LSSerializer lss = ls.createLSSerializer(); diff --git a/utils/src/main/java/com/cloud/utils/xmlobject/XmlObjectParser.java b/utils/src/main/java/com/cloud/utils/xmlobject/XmlObjectParser.java index f0e8ce31261..ea631704820 100644 --- a/utils/src/main/java/com/cloud/utils/xmlobject/XmlObjectParser.java +++ b/utils/src/main/java/com/cloud/utils/xmlobject/XmlObjectParser.java @@ -20,6 +20,8 @@ package com.cloud.utils.xmlobject; import com.cloud.utils.exception.CloudRuntimeException; + +import org.apache.cloudstack.utils.security.ParserUtils; import org.xml.sax.Attributes; import org.xml.sax.SAXException; import org.xml.sax.helpers.DefaultHandler; @@ -115,7 +117,7 @@ public class XmlObjectParser { } private XmlObject parse() { - SAXParserFactory spfactory = SAXParserFactory.newInstance(); + SAXParserFactory spfactory = ParserUtils.getSaferSAXParserFactory(); try { SAXParser saxParser = spfactory.newSAXParser(); XmlHandler handler = new XmlHandler(); diff --git a/utils/src/main/java/org/apache/cloudstack/utils/security/ParserUtils.java b/utils/src/main/java/org/apache/cloudstack/utils/security/ParserUtils.java index 0e8e9d61740..7f1cc62d308 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/security/ParserUtils.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/security/ParserUtils.java @@ -20,8 +20,17 @@ package org.apache.cloudstack.utils.security; import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParserFactory; +import javax.xml.transform.TransformerConfigurationException; +import javax.xml.transform.TransformerFactory; + +import org.apache.log4j.Logger; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; public class ParserUtils { + private static final Logger LOGGER = Logger.getLogger(ParserUtils.class); + public static DocumentBuilderFactory getSaferDocumentBuilderFactory() throws ParserConfigurationException { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); @@ -38,4 +47,26 @@ public class ParserUtils { return factory; } + + public static TransformerFactory getSaferTransformerFactory() { + TransformerFactory factory = TransformerFactory.newInstance(); + try { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (final TransformerConfigurationException e) { + LOGGER.warn("Failed to set safer feature on TransformerFactory: ", e); + } + return factory; + } + + public static SAXParserFactory getSaferSAXParserFactory() { + SAXParserFactory factory = SAXParserFactory.newInstance(); + try { + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + } catch (final ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) { + LOGGER.warn("Failed to set safer feature on SAXParserFactory: ", e); + } + return factory; + } } diff --git a/utils/src/test/java/org/apache/cloudstack/utils/security/ParserUtilsTest.java b/utils/src/test/java/org/apache/cloudstack/utils/security/ParserUtilsTest.java new file mode 100644 index 00000000000..6b6715795ed --- /dev/null +++ b/utils/src/test/java/org/apache/cloudstack/utils/security/ParserUtilsTest.java @@ -0,0 +1,55 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.utils.security; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParserFactory; +import javax.xml.transform.TransformerFactory; + +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; + +import junit.framework.TestCase; + +public class ParserUtilsTest extends TestCase { + + public void testGetSaferDocumentBuilderFactory() throws ParserConfigurationException { + final DocumentBuilderFactory factory = ParserUtils.getSaferDocumentBuilderFactory(); + assertTrue(factory.getFeature(XMLConstants.FEATURE_SECURE_PROCESSING)); + assertTrue(factory.getFeature("http://apache.org/xml/features/disallow-doctype-decl")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-general-entities")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-parameter-entities")); + assertFalse(factory.getFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd")); + assertFalse(factory.isXIncludeAware()); + assertFalse(factory.isExpandEntityReferences()); + } + + public void testGetSaferTransformerFactory() { + final TransformerFactory factory = ParserUtils.getSaferTransformerFactory(); + assertTrue(factory.getFeature(XMLConstants.FEATURE_SECURE_PROCESSING)); + } + + public void testGetSaferSAXParserFactory() throws SAXNotSupportedException, SAXNotRecognizedException, ParserConfigurationException { + final SAXParserFactory factory = ParserUtils.getSaferSAXParserFactory(); + assertTrue(factory.getFeature("http://apache.org/xml/features/disallow-doctype-decl")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-general-entities")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-parameter-entities")); + } +} diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java index 9f950024f13..7271725e8f4 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java @@ -40,6 +40,7 @@ import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.utils.security.ParserUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; @@ -1832,7 +1833,7 @@ public class HypervisorHostHelper { return ovfString; } try { - final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + final DocumentBuilderFactory factory = ParserUtils.getSaferDocumentBuilderFactory(); final Document doc = factory.newDocumentBuilder().parse(new ByteArrayInputStream(ovfString.getBytes())); final DocumentTraversal traversal = (DocumentTraversal) doc; final NodeIterator iterator = traversal.createNodeIterator(doc.getDocumentElement(), NodeFilter.SHOW_ELEMENT, null, true); @@ -1851,7 +1852,7 @@ public class HypervisorHostHelper { final DOMSource domSource = new DOMSource(doc); final StringWriter writer = new StringWriter(); final StreamResult result = new StreamResult(writer); - final TransformerFactory tf = TransformerFactory.newInstance(); + final TransformerFactory tf = ParserUtils.getSaferTransformerFactory(); final Transformer transformer = tf.newTransformer(); transformer.transform(domSource, result); return writer.toString();