commit 4534c514f89aba99a57b2f4250e0f7789fa0ead6 Author: Michael Still Date: Thu Jul 4 15:48:08 2013 +1000 Remove unsafe XML parsing. Move to using xmlutils for all XML parsing. Resolves bug 1190229. Change-Id: I43afb2e188bbea99ea30fe6cb2eb1aeedc4ddfd4 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 751a4d4..3052c84 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,7 +24,8 @@ from nova.api.openstack import xmlutil from nova import exception from nova.network.security_group import openstack_driver from nova.openstack.common.gettextutils import _ from nova.openstack.common import log as logging +from nova.openstack.common import xmlutils LOG = logging.getLogger(__name__) @@ -74,7 +73,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 e2862be..0e8a9d6 100644 --- a/nova/api/openstack/compute/contrib/security_groups.py +++ b/nova/api/openstack/compute/contrib/security_groups.py @@ -20,7 +20,6 @@ import contextlib 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,7 +30,8 @@ from nova.compute import api as compute_api from nova import exception from nova.network.security_group import neutron_driver from nova.network.security_group import openstack_driver from nova.openstack.common.gettextutils import _ +from nova.openstack.common import xmlutils from nova.virt import netutils @@ -546,7 +546,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 0000000..b131d3e --- /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 declaration forbidden") + + def unparsed_entity_decl(self, name, base, sysid, pubid, notation_name): + # expat 1.2 + raise ValueError(" unparsed entity forbidden") + + def external_entity_ref(self, context, base, systemId, publicId): + raise ValueError(" external entity forbidden") + + def notation_decl(self, name, base, sysid, pubid): + raise ValueError(" 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 f20a279..3c211d5 100755 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -60,7 +60,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 @@ -79,6 +78,7 @@ from nova.openstack.common import log as logging from nova.openstack.common import loopingcall from nova.openstack.common.notifier import api as notifier from nova.openstack.common import processutils +from nova.openstack.common import xmlutils from nova import utils from nova import version from nova.virt import configdrive @@ -1693,8 +1693,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': @@ -1711,7 +1710,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 d6ce2ee..c41f833 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 @@ -47,6 +46,7 @@ from nova.openstack.common import log as logging from nova.openstack.common import processutils from nova.openstack.common import strutils from nova.openstack.common import timeutils +from nova.openstack.common import xmlutils from nova import utils from nova.virt import configdrive from nova.virt.disk import api as disk @@ -1579,7 +1579,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 9edac2d..e4b6ec8 100644 --- a/openstack-common.conf +++ b/openstack-common.conf @@ -34,6 +34,7 @@ module=strutils module=threadgroup module=timeutils module=uuidutils +module=xmlutils # The base module to hold the copy of openstack.common base=nova