diff options
| author | Davanum Srinivas <dims@linux.vnet.ibm.com> | 2013-02-20 11:06:21 -0500 |
|---|---|---|
| committer | Gerrit Code Review <review@openstack.org> | 2013-02-26 16:08:15 +0000 |
| commit | 14df42b15fabc70472bb23264cf73acf4bfbe83d (patch) | |
| tree | 28e3f4cd9498a9720fa3ab91a891be5553526edb | |
| parent | 2e771b103276884b18f9d3326279a29c1aba1919 (diff) | |
| download | nova-14df42b15fabc70472bb23264cf73acf4bfbe83d.tar.gz nova-14df42b15fabc70472bb23264cf73acf4bfbe83d.tar.xz nova-14df42b15fabc70472bb23264cf73acf4bfbe83d.zip | |
Additional tests for safe parsing with minidom
For nova, forbid_dtd is going to be true always, however
if someone picks up this code and tries forbid_dtd = False
then the existing code is not good enough. we need to protect
against external entities/dtd and not allow notations as well.
Added a few more handlers and test cases to cover that use
case.
Change-Id: If50f690e015f2bf837b403edf552e35d7af8c907
| -rw-r--r-- | nova/tests/test_utils.py | 45 | ||||
| -rw-r--r-- | nova/utils.py | 18 |
2 files changed, 61 insertions, 2 deletions
diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index 940ddf6ec..2c2c58db9 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -23,6 +23,7 @@ import os import os.path import StringIO import tempfile +from xml.dom import minidom import mox import netaddr @@ -1059,3 +1060,47 @@ class StringLengthTestCase(test.TestCase): self.assertRaises(exception.InvalidInput, utils.check_string_length, 'a' * 256, 'name', max_length=255) + + +class SafeParserTestCase(test.TestCase): + 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 = utils.ProtectedExpatParser(forbid_dtd=False, + 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 = utils.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 = utils.ProtectedExpatParser(forbid_dtd=False, + forbid_entities=True) + self.assertRaises(ValueError, + minidom.parseString, + xml_string, parser) diff --git a/nova/utils.py b/nova/utils.py index 2c7d0b427..764fa9070 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -672,19 +672,33 @@ class ProtectedExpatParser(expatreader.ExpatParser): def entity_decl(self, entityName, is_parameter_entity, value, base, systemId, publicId, notationName): - raise ValueError("<!ENTITY> forbidden") + raise ValueError("<!ENTITY> entity declaration forbidden") def unparsed_entity_decl(self, name, base, sysid, pubid, notation_name): # expat 1.2 - raise ValueError("<!ENTITY> forbidden") + 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): |