From 14df42b15fabc70472bb23264cf73acf4bfbe83d Mon Sep 17 00:00:00 2001
From: Davanum Srinivas
Date: Wed, 20 Feb 2013 11:06:21 -0500
Subject: 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
---
nova/tests/test_utils.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
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 = ("""
+
+
+
+ html with dtd
+ """)
+
+ parser = utils.ProtectedExpatParser(forbid_dtd=False,
+ forbid_entities=True)
+ self.assertRaises(ValueError,
+ minidom.parseString,
+ xml_string, parser)
+
+ def test_external_file(self):
+ xml_string = """
+ ]>
+ ⅇ"""
+
+ parser = utils.ProtectedExpatParser(forbid_dtd=False,
+ forbid_entities=True)
+ self.assertRaises(ValueError,
+ minidom.parseString,
+ xml_string, parser)
+
+ def test_notation(self):
+ xml_string = """
+
+
+ ]>
+
+ """
+
+ 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(" forbidden")
+ raise ValueError(" entity declaration forbidden")
def unparsed_entity_decl(self, name, base, sysid, pubid, notation_name):
# expat 1.2
- raise ValueError(" forbidden")
+ 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):
--
cgit