Merge pull request #379 from fruechel/master

Fix XXE in XML parsing (related to #366)

setup.py

@@ -18,6 +18,7 @@ install_requires = [
     'pytz',
     'pyOpenSSL',
     'python-dateutil',
+    'defusedxml',
     'six'
 ]
 

src/saml2/__init__.py

@@ -36,6 +36,7 @@ except ImportError:
         import cElementTree as ElementTree
     except ImportError:
         from elementtree import ElementTree
+import defusedxml.ElementTree
 
 root_logger = logging.getLogger(__name__)
 root_logger.level = logging.NOTSET
@@ -87,7 +88,7 @@ def create_class_from_xml_string(target_class, xml_string):
     """
     if not isinstance(xml_string, six.binary_type):
         xml_string = xml_string.encode('utf-8')
-    tree = ElementTree.fromstring(xml_string)
+    tree = defusedxml.ElementTree.fromstring(xml_string)
     return create_class_from_element_tree(target_class, tree)
 
 
@@ -269,7 +270,7 @@ class ExtensionElement(object):
 
 
 def extension_element_from_string(xml_string):
-    element_tree = ElementTree.fromstring(xml_string)
+    element_tree = defusedxml.ElementTree.fromstring(xml_string)
     return _extension_element_from_element_tree(element_tree)
 
 

src/saml2/pack.py

@@ -37,6 +37,7 @@ except ImportError:
         import cElementTree as ElementTree
     except ImportError:
         from elementtree import ElementTree
+import defusedxml.ElementTree
 
 NAMESPACE = "http://schemas.xmlsoap.org/soap/envelope/"
 FORM_SPEC = """<form method="post" action="%s">
@@ -235,7 +236,7 @@ def parse_soap_enveloped_saml(text, body_class, header_class=None):
     :param text: The SOAP object as XML
     :return: header parts and body as saml.samlbase instances
     """
-    envelope = ElementTree.fromstring(text)
+    envelope = defusedxml.ElementTree.fromstring(text)
     assert envelope.tag == '{%s}Envelope' % NAMESPACE
 
     # print(len(envelope))

src/saml2/soap.py

@@ -19,6 +19,7 @@ except ImportError:
     except ImportError:
         #noinspection PyUnresolvedReferences
         from elementtree import ElementTree
+import defusedxml.ElementTree
 
 
 logger = logging.getLogger(__name__)
@@ -133,7 +134,7 @@ def parse_soap_enveloped_saml_thingy(text, expected_tags):
     :param expected_tags: What the tag of the SAML thingy is expected to be.
     :return: SAML thingy as a string
     """
-    envelope = ElementTree.fromstring(text)
+    envelope = defusedxml.ElementTree.fromstring(text)
 
     # Make sure it's a SOAP message
     assert envelope.tag == '{%s}Envelope' % soapenv.NAMESPACE
@@ -183,7 +184,7 @@ def class_instances_from_soap_enveloped_saml_thingies(text, modules):
     :return: The body and headers as class instances
     """
     try:
-        envelope = ElementTree.fromstring(text)
+        envelope = defusedxml.ElementTree.fromstring(text)
     except Exception as exc:
         raise XmlParseError("%s" % exc)
 
@@ -209,7 +210,7 @@ def open_soap_envelope(text):
     :return: dictionary with two keys "body"/"header"
     """
     try:
-        envelope = ElementTree.fromstring(text)
+        envelope = defusedxml.ElementTree.fromstring(text)
     except Exception as exc:
         raise XmlParseError("%s" % exc)
 

tests/test_03_saml2.py

@@ -17,6 +17,7 @@ except ImportError:
         import cElementTree as ElementTree
     except ImportError:
         from elementtree import ElementTree
+from defusedxml.common import EntitiesForbidden
 
 ITEMS = {
     NameID: ["""<?xml version="1.0" encoding="utf-8"?>
@@ -166,6 +167,19 @@ def test_create_class_from_xml_string_wrong_class_spec():
     assert kl == None
 
 
+def test_create_class_from_xml_string_xxe():
+    xml = """<?xml version="1.0"?>
+    <!DOCTYPE lolz [
+    <!ENTITY lol "lol">
+    <!ELEMENT lolz (#PCDATA)>
+    <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+    ]>
+    <lolz>&lol1;</lolz>
+    """
+    with raises(EntitiesForbidden) as err:
+        create_class_from_xml_string(NameID, xml)
+
+
 def test_ee_1():
     ee = saml2.extension_element_from_string(
         """<?xml version='1.0' encoding='UTF-8'?><foo>bar</foo>""")
@@ -454,6 +468,19 @@ def test_ee_7():
     assert nid.text.strip() == "http://federationX.org"
 
 
+def test_ee_xxe():
+    xml = """<?xml version="1.0"?>
+    <!DOCTYPE lolz [
+    <!ENTITY lol "lol">
+    <!ELEMENT lolz (#PCDATA)>
+    <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+    ]>
+    <lolz>&lol1;</lolz>
+    """
+    with raises(EntitiesForbidden):
+        saml2.extension_element_from_string(xml)
+
+
 def test_extension_element_loadd():
     ava = {'attributes': {},
            'tag': 'ExternalEntityAttributeAuthority',

tests/test_43_soap.py

@@ -12,9 +12,13 @@ except ImportError:
         import cElementTree as ElementTree
     except ImportError:
         from elementtree import ElementTree
+from defusedxml.common import EntitiesForbidden
+
+from pytest import raises
 
 import saml2.samlp as samlp
 from saml2.samlp import NAMESPACE as SAMLP_NAMESPACE
+from saml2 import soap
 
 NAMESPACE = "http://schemas.xmlsoap.org/soap/envelope/"
 
@@ -66,3 +70,42 @@ def test_make_soap_envelope():
     assert len(body) == 1
     saml_part = body[0]
     assert saml_part.tag == '{%s}AuthnRequest' % SAMLP_NAMESPACE
+
+
+def test_parse_soap_enveloped_saml_thingy_xxe():
+    xml = """<?xml version="1.0"?>
+    <!DOCTYPE lolz [
+    <!ENTITY lol "lol">
+    <!ELEMENT lolz (#PCDATA)>
+    <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+    ]>
+    <lolz>&lol1;</lolz>
+    """
+    with raises(EntitiesForbidden):
+        soap.parse_soap_enveloped_saml_thingy(xml, None)
+
+
+def test_class_instances_from_soap_enveloped_saml_thingies_xxe():
+    xml = """<?xml version="1.0"?>
+    <!DOCTYPE lolz [
+    <!ENTITY lol "lol">
+    <!ELEMENT lolz (#PCDATA)>
+    <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+    ]>
+    <lolz>&lol1;</lolz>
+    """
+    with raises(soap.XmlParseError):
+        soap.class_instances_from_soap_enveloped_saml_thingies(xml, None)
+
+
+def test_open_soap_envelope_xxe():
+    xml = """<?xml version="1.0"?>
+    <!DOCTYPE lolz [
+    <!ENTITY lol "lol">
+    <!ELEMENT lolz (#PCDATA)>
+    <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+    ]>
+    <lolz>&lol1;</lolz>
+    """
+    with raises(soap.XmlParseError):
+        soap.open_soap_envelope(xml)

tests/test_51_client.py

@@ -7,6 +7,7 @@ import six
 from future.backports.urllib.parse import parse_qs
 from future.backports.urllib.parse import urlencode
 from future.backports.urllib.parse import urlparse
+from pytest import raises
 
 from saml2.argtree import add_path
 from saml2.cert import OpenSSLWrapper
@@ -25,6 +26,7 @@ from saml2.assertion import Assertion
 from saml2.authn_context import INTERNETPROTOCOLPASSWORD
 from saml2.client import Saml2Client
 from saml2.config import SPConfig
+from saml2.pack import parse_soap_enveloped_saml
 from saml2.response import LogoutResponse
 from saml2.saml import NAMEID_FORMAT_PERSISTENT, EncryptedAssertion, Advice
 from saml2.saml import NAMEID_FORMAT_TRANSIENT
@@ -38,6 +40,8 @@ from saml2.s_utils import do_attribute_statement
 from saml2.s_utils import factory
 from saml2.time_util import in_a_while, a_while_ago
 
+from defusedxml.common import EntitiesForbidden
+
 from fakeIDP import FakeIDP
 from fakeIDP import unpack_form
 from pathutils import full_path
@@ -1552,6 +1556,17 @@ class TestClientWithDummy():
                'http://www.example.com/login'
         assert ac.authn_context_class_ref.text == INTERNETPROTOCOLPASSWORD
 
+def test_parse_soap_enveloped_saml_xxe():
+    xml = """<?xml version="1.0"?>
+    <!DOCTYPE lolz [
+    <!ENTITY lol "lol">
+    <!ELEMENT lolz (#PCDATA)>
+    <!ENTITY lol1 "&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;">
+    ]>
+    <lolz>&lol1;</lolz>
+    """
+    with raises(EntitiesForbidden):
+        parse_soap_enveloped_saml(xml, None)
 
 # if __name__ == "__main__":
 #     tc = TestClient()