From 9e0a6a877b7b9811cd69a6d32b9fc9657bad4fec Mon Sep 17 00:00:00 2001
From: Claudio Salazar <csalazar@spect.cl>
Date: Tue, 1 Apr 2014 23:54:04 +0800
Subject: [PATCH 1/4] Fixed XXE flaw in sitemap reader

---
 scrapy/utils/sitemap.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scrapy/utils/sitemap.py b/scrapy/utils/sitemap.py
index 24a540531..bbf37bc28 100644
--- a/scrapy/utils/sitemap.py
+++ b/scrapy/utils/sitemap.py
@@ -12,7 +12,7 @@ class Sitemap(object):
     (type=sitemapindex) files"""
 
     def __init__(self, xmltext):
-        xmlp = lxml.etree.XMLParser(recover=True, remove_comments=True)
+        xmlp = lxml.etree.XMLParser(recover=True, remove_comments=True, resolve_entities=False)
         self._root = lxml.etree.fromstring(xmltext, parser=xmlp)
         rt = self._root.tag
         self.type = self._root.tag.split('}', 1)[1] if '}' in rt else rt

From 259f66305bfb26d4e57b759d4f110481fe6fdcc6 Mon Sep 17 00:00:00 2001
From: Claudio Salazar <csalazar@spect.cl>
Date: Sat, 5 Apr 2014 00:13:27 +0800
Subject: [PATCH 2/4] Fixed XML selector against XXE attacks

---
 scrapy/selector/unified.py    |  7 ++++++-
 scrapy/tests/test_selector.py | 10 ++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py
index 790937af5..3fa5fd767 100644
--- a/scrapy/selector/unified.py
+++ b/scrapy/selector/unified.py
@@ -15,11 +15,16 @@ from .csstranslator import ScrapyHTMLTranslator, ScrapyGenericTranslator
 
 __all__ = ['Selector', 'SelectorList']
 
+
+class SafeXMLParser(etree.XMLParser):
+    def __init__(self, *args, **kwargs):
+        super(SafeXMLParser, self).__init__(*args, resolve_entities=False, **kwargs)
+
 _ctgroup = {
     'html': {'_parser': etree.HTMLParser,
              '_csstranslator': ScrapyHTMLTranslator(),
              '_tostring_method': 'html'},
-    'xml': {'_parser': etree.XMLParser,
+    'xml': {'_parser': SafeXMLParser,
             '_csstranslator': ScrapyGenericTranslator(),
             '_tostring_method': 'xml'},
 }
diff --git a/scrapy/tests/test_selector.py b/scrapy/tests/test_selector.py
index d84e4bd47..710f1c28e 100644
--- a/scrapy/tests/test_selector.py
+++ b/scrapy/tests/test_selector.py
@@ -297,6 +297,16 @@ class SelectorTestCase(unittest.TestCase):
         sel.remove_namespaces()
         self.assertEqual(len(sel.xpath("//link/@type")), 2)
 
+    def test_xml_entity_expansion(self):
+        malicious_xml = '<?xml version="1.0" encoding="ISO-8859-1"?>'\
+            '<!DOCTYPE foo [ <!ELEMENT foo ANY > <!ENTITY xxe SYSTEM '\
+            '"file:///etc/passwd" >]><foo>&xxe;</foo>'
+
+        response = XmlResponse('http://example.com', body=malicious_xml)
+        sel = self.sscls(response=response)
+
+        self.assertEqual(sel.extract(), '<foo>&xxe;</foo>')
+
 
 class DeprecatedXpathSelectorTest(unittest.TestCase):
 

From 912e4f7dbd8a1b81ff1fcb841cb250bcd89257fd Mon Sep 17 00:00:00 2001
From: Claudio Salazar <csalazar@spect.cl>
Date: Sat, 5 Apr 2014 00:22:36 +0800
Subject: [PATCH 3/4] Added test against XXE attacks for Sitemap

---
 scrapy/tests/test_utils_sitemap.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/scrapy/tests/test_utils_sitemap.py b/scrapy/tests/test_utils_sitemap.py
index 0049e5c40..56585143f 100644
--- a/scrapy/tests/test_utils_sitemap.py
+++ b/scrapy/tests/test_utils_sitemap.py
@@ -188,13 +188,28 @@ Disallow: /forum/active/
             <xhtml:link rel="alternate" hreflang="en"/><!-- wrong tag without href -->
         </url>
     </urlset>""")
-        
+
         self.assertEqual(list(s), [
             {'loc': 'http://www.example.com/english/',
              'alternate': ['http://www.example.com/deutsch/', 'http://www.example.com/schweiz-deutsch/', 'http://www.example.com/english/']
             }
         ])
 
+    def test_xml_entity_expansion(self):
+        s = Sitemap("""<?xml version="1.0" encoding="utf-8"?>
+          <!DOCTYPE foo [
+          <!ELEMENT foo ANY >
+          <!ENTITY xxe SYSTEM "file:///etc/passwd" >
+          ]>
+          <urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
+            <url>
+              <loc>http://127.0.0.1:8000/&xxe;</loc>
+            </url>
+          </urlset>
+        """)
+
+        self.assertEqual(list(s), [{'loc': 'http://127.0.0.1:8000/'}])
+
 
 if __name__ == '__main__':
     unittest.main()

From 99c8c209f9be7b445609b2707e6fc1a04d1af576 Mon Sep 17 00:00:00 2001
From: Claudio Salazar <csalazar@spect.cl>
Date: Sat, 5 Apr 2014 00:40:41 +0800
Subject: [PATCH 4/4] Added resolve_entities to kwargs in SafeXMLParser

---
 scrapy/selector/unified.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py
index 3fa5fd767..bb3164582 100644
--- a/scrapy/selector/unified.py
+++ b/scrapy/selector/unified.py
@@ -18,7 +18,8 @@ __all__ = ['Selector', 'SelectorList']
 
 class SafeXMLParser(etree.XMLParser):
     def __init__(self, *args, **kwargs):
-        super(SafeXMLParser, self).__init__(*args, resolve_entities=False, **kwargs)
+        kwargs.setdefault('resolve_entities', False)
+        super(SafeXMLParser, self).__init__(*args, **kwargs)
 
 _ctgroup = {
     'html': {'_parser': etree.HTMLParser,