CVE-2016-10127: Improper Restriction of XML External Entity Reference (CWE-611)
value: PySAML2 allows remote attackers to conduct XML external entity (XXE) attacks via a crafted SAML XML request or response.
CWE description: The product processes an XML document that can contain XML entities with URIs that resolve to documents outside of the intended sphere of control, causing the product to embed incorrect documents into its output.
CVSS v3 base score: 9
Published: 2017-03-03T15:59Z
Repository: https://github.com/rohe/pysaml2
Language: Python
Files changed: setup.py, src/saml2/__init__.py, src/saml2/pack.py, src/saml2/soap.py, tests/test_03_saml2.py, tests/test_43_soap.py, tests/test_51_client.py
Commit date: 2016-10-31 11:56:48 +1100
Commit message: Fix XXE in XML parsing (related to #366)

This fixes XXE issues on anything where pysaml2 parses XML directly as part of
issue #366. It doesn't address the xmlsec issues discussed on that ticket as
they are out of reach of a direct fix and need the underlying library to fix
this issue.

Diff: diff --git a/setup.py b/setup.py
index 8d39207..de96cae 100755
--- a/setup.py
+++ b/setup.py
@@ -16,10 +16,11 @@ install_requires = [
     'repoze.who',
     'pycryptodomex',
     'pytz',
     'pyOpenSSL',
     'python-dateutil',
+    'defusedxml',
     'six'
 ]
 
 version = ''
 with open('src/saml2/__init__.py', 'r') as fd:
diff --git a/src/saml2/__init__.py b/src/saml2/__init__.py
index 6833d7e..b246caa 100644
--- a/src/saml2/__init__.py
+++ b/src/saml2/__init__.py
@@ -34,10 +34,11 @@ try:
 except ImportError:
     try:
         import cElementTree as ElementTree
     except ImportError:
         from elementtree import ElementTree
+import defusedxml.ElementTree
 
 root_logger = logging.getLogger(__name__)
 root_logger.level = logging.NOTSET
 
 NAMESPACE = 'urn:oasis:names:tc:SAML:2.0:assertion'
@@ -85,11 +86,11 @@ def create_class_from_xml_string(target_class, xml_string):
         the contents of the XML - or None if the root XML tag and namespace did
         not match those of the target class.
     """
     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)
 
 
 def create_class_from_element_tree(target_class, tree, namespace=None,
                                    tag=None):
@@ -267,11 +268,11 @@ class ExtensionElement(object):
 
         return self
 
 
 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)
 
 
 def _extension_element_from_element_tree(element_tree):
     elementc_tag = element_tree.tag
diff --git a/src/saml2/pack.py b/src/saml2/pack.py
index e4c1462..728a516 100644
--- a/src/saml2/pack.py
+++ b/src/saml2/pack.py
@@ -35,10 +35,11 @@ try:
 except ImportError:
     try:
         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">
    <input type="hidden" name="%s" value="%s" />
    <input type="hidden" name="RelayState" value="%s" />
@@ -233,11 +234,11 @@ def parse_soap_enveloped_saml(text, body_class, header_class=None):
     """Parses a SOAP enveloped SAML thing and returns header parts and body
 
     :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))
     body = None
     header = {}
diff --git a/src/saml2/soap.py b/src/saml2/soap.py
index c1be544..055c690 100644
--- a/src/saml2/soap.py
+++ b/src/saml2/soap.py
@@ -17,10 +17,11 @@ except ImportError:
     try:
         import cElementTree as ElementTree
     except ImportError:
         #noinspection PyUnresolvedReferences
         from elementtree import ElementTree
+import defusedxml.ElementTree
 
 
 logger = logging.getLogger(__name__)
 
 
@@ -131,11 +132,11 @@ def parse_soap_enveloped_saml_thingy(text, expected_tags):
 
     :param text: The SOAP object as XML string
     :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
 
     assert len(envelope) >= 1
@@ -181,11 +182,11 @@ def class_instances_from_soap_enveloped_saml_thingies(text, modules):
     :param text: The SOAP object as XML
     :param modules: modules representing xsd schemas
     :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)
 
     assert envelope.tag == '{%s}Envelope' % soapenv.NAMESPACE
     assert len(envelope) >= 1
@@ -207,11 +208,11 @@ def open_soap_envelope(text):
 
     :param text: SOAP message
     :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)
 
     assert envelope.tag == '{%s}Envelope' % soapenv.NAMESPACE
     assert len(envelope) >= 1
diff --git a/tests/test_03_saml2.py b/tests/test_03_saml2.py
index 136161a..a71eb3c 100644
--- a/tests/test_03_saml2.py
+++ b/tests/test_03_saml2.py
@@ -15,10 +15,11 @@ try:
 except ImportError:
     try:
         import cElementTree as ElementTree
     except ImportError:
         from elementtree import ElementTree
+from defusedxml.common import EntitiesForbidden
 
 ITEMS = {
     NameID: ["""<?xml version="1.0" encoding="utf-8"?>
 <NameID xmlns="urn:oasis:names:tc:SAML:2.0:assertion"
   Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
@@ -164,10 +165,23 @@ def test_create_class_from_xml_string_wrong_class_spec():
     kl = create_class_from_xml_string(SubjectConfirmationData,
                                       ITEMS[SubjectConfirmation])
     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>""")
     assert ee != None
     print(ee.__dict__)
@@ -452,10 +466,23 @@ def test_ee_7():
     assert len(nid.children) == 0
     assert _eq(nid.attributes.keys(), ["Format"])
     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',
            'namespace': 'urn:oasis:names:tc:SAML:metadata:dynamicsaml',
            'children': [{
diff --git a/tests/test_43_soap.py b/tests/test_43_soap.py
index 4cde3d6..bf66a1d 100755
--- a/tests/test_43_soap.py
+++ b/tests/test_43_soap.py
@@ -10,13 +10,17 @@ try:
 except ImportError:
     try:
         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/"
 
 example = """<Envelope xmlns="http://schemas.xmlsoap.org/soap/envelope/">
     <Body>
@@ -64,5 +68,44 @@ def test_make_soap_envelope():
     body = envelope[0]
     assert body.tag == '{%s}Body' % NAMESPACE
     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)
diff --git a/tests/test_51_client.py b/tests/test_51_client.py
index f695816..13cef7c 100644
--- a/tests/test_51_client.py
+++ b/tests/test_51_client.py
@@ -5,10 +5,11 @@ import base64
 import uuid
 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
 from saml2.xmldsig import SIG_RSA_SHA256
 from saml2 import BINDING_HTTP_POST
@@ -23,10 +24,11 @@ from saml2 import s_utils
 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
 from saml2.saml import NameID
 from saml2.samlp import SessionIndex
@@ -36,10 +38,12 @@ from saml2.sigver import rm_xmltag
 from saml2.sigver import verify_redirect_signature
 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
 
 AUTHN = {
@@ -1550,10 +1554,21 @@ class TestClientWithDummy():
         ac = resp.assertion.authn_statement[0].authn_context
         assert ac.authenticating_authority[0].text == \
                '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()
 #     tc.setup_class()
 #     tc.test_response()

Vulnerable code:     tree = ElementTree.fromstring(xml_string)
    element_tree = ElementTree.fromstring(xml_string)
    envelope = ElementTree.fromstring(text)
    envelope = ElementTree.fromstring(text)
        envelope = ElementTree.fromstring(text)
        envelope = ElementTree.fromstring(text)
Fixed code:     'defusedxml',
import defusedxml.ElementTree
    tree = defusedxml.ElementTree.fromstring(xml_string)
    element_tree = defusedxml.ElementTree.fromstring(xml_string)
import defusedxml.ElementTree
    envelope = defusedxml.ElementTree.fromstring(text)
import defusedxml.ElementTree
    envelope = defusedxml.ElementTree.fromstring(text)
        envelope = defusedxml.ElementTree.fromstring(text)
        envelope = defusedxml.ElementTree.fromstring(text)
from defusedxml.common import EntitiesForbidden
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_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)


from defusedxml.common import EntitiesForbidden

from pytest import raises
from saml2 import soap


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)
from pytest import raises
from saml2.pack import parse_soap_enveloped_saml
from defusedxml.common import EntitiesForbidden

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)