From 6446b4ecd2e7f2dd28cf5b54ee281f70429b31c3 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 26 Feb 2013 15:43:50 -0500 Subject: [PATCH] Prevent DoS through XML entity expansion Add a ProtectedXMLParser that overrides the doctype declaration handler. The handler simply throws an exception and prevents any further parsing of the incoming xml. Fixes LP Bug #1100282 Change-Id: I6488e1a6a52326006e7e7927ece5b5939b72e83e --- quantum/tests/unit/test_wsgi.py | 21 +++++++++++++++++++++ quantum/wsgi.py | 19 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/quantum/tests/unit/test_wsgi.py b/quantum/tests/unit/test_wsgi.py index 0c09c1cd4d..7e66c0a91a 100644 --- a/quantum/tests/unit/test_wsgi.py +++ b/quantum/tests/unit/test_wsgi.py @@ -175,6 +175,27 @@ class XMLDeserializerTest(testtools.TestCase): self.assertRaises( exception.MalformedRequestBody, deserializer.default, data_string) + def test_entity_expansion(self): + def killer_body(): + return ((""" + + ]> + + + %(d)s + + """) % { + 'a': 'A' * 10, + 'b': '&a;' * 10, + 'c': '&b;' * 10, + 'd': '&c;' * 9999, + }).strip() + + deserializer = wsgi.XMLDeserializer() + self.assertRaises( + ValueError, deserializer.default, killer_body()) + class JSONDeserializerTest(testtools.TestCase): def test_default_raise_Maiformed_Exception(self): diff --git a/quantum/wsgi.py b/quantum/wsgi.py index a78c88ffe0..58b9d156a4 100644 --- a/quantum/wsgi.py +++ b/quantum/wsgi.py @@ -452,6 +452,18 @@ class JSONDeserializer(TextDeserializer): return {'body': self._from_json(datastring)} +class ProtectedXMLParser(etree.XMLParser): + def __init__(self, *args, **kwargs): + etree.XMLParser.__init__(self, *args, **kwargs) + self._parser.StartDoctypeDeclHandler = self.start_doctype_decl + + def start_doctype_decl(self, name, sysid, pubid, internal): + raise ValueError(_("Inline DTD forbidden")) + + def doctype(self, name, pubid, system): + raise ValueError(_("Inline DTD forbidden")) + + class XMLDeserializer(TextDeserializer): def __init__(self, metadata=None): @@ -493,12 +505,17 @@ class XMLDeserializer(TextDeserializer): node.remove(link) return link_list and {link_key: link_list} or {} + def _parseXML(self, text): + parser = ProtectedXMLParser() + parser.feed(text) + return parser.close() + def _from_xml(self, datastring): if datastring is None: return None plurals = set(self.metadata.get('plurals', {})) try: - node = etree.fromstring(datastring) + node = self._parseXML(datastring) root_tag = self._get_key(node.tag) # Deserialize link node was needed by unit test for verifying # the request's response