diff options
author | Davanum Srinivas <dims@linux.vnet.ibm.com> | 2013-02-26 11:26:24 -0500 |
---|---|---|
committer | Davanum Srinivas <dims@linux.vnet.ibm.com> | 2013-02-26 12:19:54 -0500 |
commit | 0d7417cff68e74f636d371529998e275e2765be8 (patch) | |
tree | 5666959a2785419754bb1e39a21273c8f2e8a8d9 | |
parent | bf6c0871d737da802fbb2e70d2481712ac94a1bd (diff) | |
download | oslo-0d7417cff68e74f636d371529998e275e2765be8.tar.gz oslo-0d7417cff68e74f636d371529998e275e2765be8.tar.xz oslo-0d7417cff68e74f636d371529998e275e2765be8.zip |
Port safe parsing with minidom patches from Nova
Prevent attacks through xml entity expansion etc.
Fixes LP# 1100282
Change-Id: I391531deac122697556c282184c8f8890ea66489
-rw-r--r-- | openstack/common/wsgi.py | 3 | ||||
-rw-r--r-- | openstack/common/xmlutils.py | 74 | ||||
-rw-r--r-- | tests/unit/test_xmlutils.py | 101 |
3 files changed, 177 insertions, 1 deletions
diff --git a/openstack/common/wsgi.py b/openstack/common/wsgi.py index 6ffbce1..fe35467 100644 --- a/openstack/common/wsgi.py +++ b/openstack/common/wsgi.py @@ -41,6 +41,7 @@ from openstack.common import jsonutils from openstack.common import log as logging from openstack.common import service from openstack.common import sslutils +from openstack.common import xmlutils socket_opts = [ cfg.IntOpt('backlog', @@ -743,7 +744,7 @@ class XMLDeserializer(TextDeserializer): plurals = set(self.metadata.get('plurals', {})) try: - node = minidom.parseString(datastring).childNodes[0] + node = xmlutils.safe_minidom_parse_string(datastring).childNodes[0] return {node.nodeName: self._from_xml_node(node, plurals)} except expat.ExpatError: msg = _("cannot understand XML") diff --git a/openstack/common/xmlutils.py b/openstack/common/xmlutils.py new file mode 100644 index 0000000..3370048 --- /dev/null +++ b/openstack/common/xmlutils.py @@ -0,0 +1,74 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013 IBM +# +# 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/tests/unit/test_xmlutils.py b/tests/unit/test_xmlutils.py new file mode 100644 index 0000000..c38e223 --- /dev/null +++ b/tests/unit/test_xmlutils.py @@ -0,0 +1,101 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013 IBM +# +# 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. + +import datetime +import StringIO +from xml.dom import minidom + +from openstack.common import xmlutils +from tests import utils + + +class XMLUtilsTestCase(utils.BaseTestCase): + def test_safe_parse_xml(self): + + normal_body = (""" + <?xml version="1.0" ?><foo> + <bar> + <v1>hey</v1> + <v2>there</v2> + </bar> + </foo>""").strip() + + def killer_body(): + return (("""<!DOCTYPE x [ + <!ENTITY a "%(a)s"> + <!ENTITY b "%(b)s"> + <!ENTITY c "%(c)s">]> + <foo> + <bar> + <v1>%(d)s</v1> + </bar> + </foo>""") % { + 'a': 'A' * 10, + 'b': '&a;' * 10, + 'c': '&b;' * 10, + 'd': '&c;' * 9999, + }).strip() + + dom = xmlutils.safe_minidom_parse_string(normal_body) + self.assertEqual(normal_body, str(dom.toxml())) + + self.assertRaises(ValueError, + xmlutils.safe_minidom_parse_string, + killer_body()) + + +class SafeParserTestCase(utils.BaseTestCase): + def test_external_dtd(self): + xml_string = ("""<?xml version="1.0" encoding="utf-8"?> + <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> + <html> + <head/> + <body>html with dtd</body> + </html>""") + + parser = xmlutils.ProtectedExpatParser(forbid_dtd=True, + forbid_entities=True) + self.assertRaises(ValueError, + minidom.parseString, + xml_string, parser) + + def test_external_file(self): + xml_string = """<!DOCTYPE external [ + <!ENTITY ee SYSTEM "file:///PATH/TO/root.xml"> + ]> + <root>ⅇ</root>""" + + parser = xmlutils.ProtectedExpatParser(forbid_dtd=False, + forbid_entities=True) + self.assertRaises(ValueError, + minidom.parseString, + xml_string, parser) + + def test_notation(self): + xml_string = """<?xml version="1.0" standalone="no"?> + <!-- comment data --> + <!DOCTYPE x [ + <!NOTATION notation SYSTEM "notation.jpeg"> + ]> + <root attr1="value1"> + </root>""" + + parser = xmlutils.ProtectedExpatParser(forbid_dtd=False, + forbid_entities=True) + self.assertRaises(ValueError, + minidom.parseString, + xml_string, parser) |