Remove unsafe XML parsing2013.1.3

Move to using xmlutils for all XML parsing. Resolves bug 1190229.
Grizzly backport of Mikal Still's master patch.

Change-Id: I43afb2e188bbea99ea30fe6cb2eb1aeedc4ddfd4

6 files changed, 84 insertions, 12 deletions

diff --git a/nova/api/openstack/compute/contrib/security_group_default_rules.py b/nova/api/openstack/compute/contrib/security_group_default_rules.py
index 751a4d4594..3052c8499a 100644
--- a/nova/api/openstack/compute/contrib/security_group_default_rules.py
+++ b/nova/api/openstack/compute/contrib/security_group_default_rules.py
@@ -14,8 +14,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from xml.dom import minidom
-
 import webob
 from webob import exc
 
@@ -26,6 +24,7 @@ from nova.api.openstack import xmlutil
 from nova import exception
 from nova.network.security_group import openstack_driver
 from nova.openstack.common import log as logging
+from nova.openstack.common import xmlutils
 
 
 LOG = logging.getLogger(__name__)
@@ -73,7 +72,7 @@ class SecurityGroupDefaultRuleTemplate(xmlutil.TemplateBuilder):
 
 class SecurityGroupDefaultRulesXMLDeserializer(wsgi.MetadataXMLDeserializer):
     def default(self, string):
-        dom = minidom.parseString(string)
+        dom = xmlutils.safe_minidom_parse_string(string)
         security_group_rule = self._extract_security_group_default_rule(dom)
         return {'body': {'security_group_default_rule': security_group_rule}}
 
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index f3d047c8c0..318a44b925 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -19,7 +19,6 @@
 import json
 import webob
 from webob import exc
-from xml.dom import minidom
 
 from nova.api.openstack import common
 from nova.api.openstack import extensions
@@ -31,6 +30,7 @@ from nova import exception
 from nova.network.security_group import openstack_driver
 from nova.network.security_group import quantum_driver
 from nova.openstack.common import log as logging
+from nova.openstack.common import xmlutils
 from nova.virt import netutils
 
 
@@ -500,7 +500,7 @@ class SecurityGroupsOutputController(wsgi.Controller):
                     servers[0][key] = req_obj['server'].get(
                         key, [{'name': 'default'}])
                 except ValueError:
-                    root = minidom.parseString(req.body)
+                    root = xmlutils.safe_minidom_parse_string(req.body)
                     sg_root = root.getElementsByTagName(key)
                     groups = []
                     if sg_root:
diff --git a/nova/openstack/common/xmlutils.py b/nova/openstack/common/xmlutils.py
new file mode 100644
index 0000000000..b131d3e2e9
--- /dev/null
+++ b/nova/openstack/common/xmlutils.py
@@ -0,0 +1,74 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 IBM Corp.
+#
+#    Licensed 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.
+
+from xml.dom import minidom
+from xml.parsers import expat
+from xml import sax
+from xml.sax import expatreader
+
+
+class ProtectedExpatParser(expatreader.ExpatParser):
+    """An expat parser which disables DTD's and entities by default."""
+
+    def __init__(self, forbid_dtd=True, forbid_entities=True,
+                 *args, **kwargs):
+        # Python 2.x old style class
+        expatreader.ExpatParser.__init__(self, *args, **kwargs)
+        self.forbid_dtd = forbid_dtd
+        self.forbid_entities = forbid_entities
+
+    def start_doctype_decl(self, name, sysid, pubid, has_internal_subset):
+        raise ValueError("Inline DTD forbidden")
+
+    def entity_decl(self, entityName, is_parameter_entity, value, base,
+                    systemId, publicId, notationName):
+        raise ValueError("<!ENTITY> entity declaration forbidden")
+
+    def unparsed_entity_decl(self, name, base, sysid, pubid, notation_name):
+        # expat 1.2
+        raise ValueError("<!ENTITY> unparsed entity forbidden")
+
+    def external_entity_ref(self, context, base, systemId, publicId):
+        raise ValueError("<!ENTITY> external entity forbidden")
+
+    def notation_decl(self, name, base, sysid, pubid):
+        raise ValueError("<!ENTITY> notation forbidden")
+
+    def reset(self):
+        expatreader.ExpatParser.reset(self)
+        if self.forbid_dtd:
+            self._parser.StartDoctypeDeclHandler = self.start_doctype_decl
+            self._parser.EndDoctypeDeclHandler = None
+        if self.forbid_entities:
+            self._parser.EntityDeclHandler = self.entity_decl
+            self._parser.UnparsedEntityDeclHandler = self.unparsed_entity_decl
+            self._parser.ExternalEntityRefHandler = self.external_entity_ref
+            self._parser.NotationDeclHandler = self.notation_decl
+            try:
+                self._parser.SkippedEntityHandler = None
+            except AttributeError:
+                # some pyexpat versions do not support SkippedEntity
+                pass
+
+
+def safe_minidom_parse_string(xml_string):
+    """Parse an XML string using minidom safely.
+
+    """
+    try:
+        return minidom.parseString(xml_string, parser=ProtectedExpatParser())
+    except sax.SAXParseException:
+        raise expat.ExpatError()
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 5a23592346..456eb80b82 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -59,7 +59,6 @@ from eventlet import tpool
 from eventlet import util as eventlet_util
 from lxml import etree
 from oslo.config import cfg
-from xml.dom import minidom
 
 from nova.api.metadata import base as instance_metadata
 from nova import block_device
@@ -76,6 +75,7 @@ from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
 from nova.openstack.common.notifier import api as notifier
+from nova.openstack.common import xmlutils
 from nova import utils
 from nova import version
 from nova.virt import configdrive
@@ -1627,8 +1627,7 @@ class LibvirtDriver(driver.ComputeDriver):
         def get_vnc_port_for_instance(instance_name):
             virt_dom = self._lookup_by_name(instance_name)
             xml = virt_dom.XMLDesc(0)
-            # TODO(sleepsonthefloor): use etree instead of minidom
-            dom = minidom.parseString(xml)
+            dom = xmlutils.safe_minidom_parse_string(xml)
 
             for graphic in dom.getElementsByTagName('graphics'):
                 if graphic.getAttribute('type') == 'vnc':
@@ -1645,7 +1644,7 @@ class LibvirtDriver(driver.ComputeDriver):
             virt_dom = self._lookup_by_name(instance_name)
             xml = virt_dom.XMLDesc(0)
             # TODO(sleepsonthefloor): use etree instead of minidom
-            dom = minidom.parseString(xml)
+            dom = xmlutils.safe_minidom_parse_string(xml)
 
             for graphic in dom.getElementsByTagName('graphics'):
                 if graphic.getAttribute('type') == 'spice':
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index 26bd9d268c..0991e13bd1 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -29,7 +29,6 @@ import time
 import urllib
 import urlparse
 import uuid
-from xml.dom import minidom
 from xml.parsers import expat
 
 from eventlet import greenthread
@@ -44,6 +43,7 @@ from nova import exception
 from nova.image import glance
 from nova.openstack.common import excutils
 from nova.openstack.common import log as logging
+from nova.openstack.common import xmlutils
 from nova import utils
 from nova.virt import configdrive
 from nova.virt.disk import api as disk
@@ -1437,7 +1437,7 @@ def compile_diagnostics(record):
         vm_uuid = record["uuid"]
         xml = _get_rrd(_get_rrd_server(), vm_uuid)
         if xml:
-            rrd = minidom.parseString(xml)
+            rrd = xmlutils.safe_minidom_parse_string(xml)
             for i, node in enumerate(rrd.firstChild.childNodes):
                 # Provide the last update of the information
                 if node.localName == 'lastupdate':
diff --git a/openstack-common.conf b/openstack-common.conf
index a2688fa459..fc7933d3a9 100644
--- a/openstack-common.conf
+++ b/openstack-common.conf
@@ -1,7 +1,7 @@
 [DEFAULT]
 
 # The list of modules to copy from openstack-common
-modules=cliutils,context,db,excutils,eventlet_backdoor,fileutils,gettextutils,importutils,jsonutils,local,lockutils,log,network_utils,notifier,plugin,policy,rootwrap,setup,timeutils,rpc,uuidutils,install_venv_common,flakes,version,processutils
+modules=cliutils,context,db,excutils,eventlet_backdoor,fileutils,gettextutils,importutils,jsonutils,local,lockutils,log,network_utils,notifier,plugin,policy,rootwrap,setup,timeutils,rpc,uuidutils,install_venv_common,flakes,version,processutils,xmlutils
 
 # The base module to hold the copy of openstack.common
 base=nova