Merge pull request #673 from tomchristie/defusedxml

XML Security fixes

.travis.yml

@@ -13,6 +13,7 @@ env:
 
 install:
   - pip install $DJANGO
+  - pip install defusedxml==0.3
   - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install django-filter==0.5.4 --use-mirrors; fi"
   - "if [[ ${TRAVIS_PYTHON_VERSION::1} == '3' ]]; then pip install https://github.com/alex/django-filter/tarball/master; fi"
   - export PYTHONPATH=.

README.md

@@ -31,9 +31,10 @@ There is also a sandbox API you can use for testing purposes, [available here][s
 
 **Optional:**
 
-* [Markdown] - Markdown support for the self describing API.
-* [PyYAML] - YAML content type support.
-* [django-filter] - Filtering support.
+* [Markdown][markdown] - Markdown support for the self describing API.
+* [PyYAML][pyyaml] - YAML content type support.
+* [defusedxml][defusedxml] - XML content-type support.
+* [django-filter][django-filter] - Filtering support.
 
 # Installation
 
@@ -115,4 +116,5 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 [urlobject]: https://github.com/zacharyvoase/urlobject
 [markdown]: http://pypi.python.org/pypi/Markdown/
 [pyyaml]: http://pypi.python.org/pypi/PyYAML
+[defusedxml]: https://pypi.python.org/pypi/defusedxml
 [django-filter]: http://pypi.python.org/pypi/django-filter

docs/index.md

@@ -34,6 +34,7 @@ The following packages are optional:
 
 * [Markdown][markdown] (2.1.0+) - Markdown support for the browseable API.
 * [PyYAML][yaml] (3.10+) - YAML content-type support.
+* [defusedxml][defusedxml] (0.3+) - XML content-type support.
 * [django-filter][django-filter] (0.5.4+) - Filtering support.
 
 ## Installation
@@ -173,6 +174,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 [urlobject]: https://github.com/zacharyvoase/urlobject
 [markdown]: http://pypi.python.org/pypi/Markdown/
 [yaml]: http://pypi.python.org/pypi/PyYAML
+[defusedxml]: https://pypi.python.org/pypi/defusedxml
 [django-filter]: http://pypi.python.org/pypi/django-filter
 [0.4]: https://github.com/tomchristie/django-rest-framework/tree/0.4.X
 [image]: img/quickstart.png

optionals.txt

@@ -1,3 +1,4 @@
 markdown>=2.1.0
 PyYAML>=3.10
+defusedxml>=0.3
 django-filter>=0.5.4

rest_framework/compat.py

@@ -421,17 +421,8 @@ except ImportError:
     yaml = None
 
 
-# xml.etree.parse only throws ParseError for python >= 2.7
+# XML is optional
 try:
-    from xml.etree import ParseError as ETParseError
-except ImportError:  # python < 2.7
-    ETParseError = None
-
-
-# XMLParser only takes an encoding arg from >= 2.7
-def ET_XMLParser(encoding=None):
-    from xml.etree import ElementTree as ET
-    try:
-        return ET.XMLParser(encoding=encoding)
-    except TypeError:
-        return ET.XMLParser()
+    import defusedxml.ElementTree as etree
+except ImportError:
+    etree = None

rest_framework/parsers.py

@@ -9,11 +9,9 @@ from django.conf import settings
 from django.http import QueryDict
 from django.http.multipartparser import MultiPartParser as DjangoMultiPartParser
 from django.http.multipartparser import MultiPartParserError
-from rest_framework.compat import yaml, ETParseError, ET_XMLParser
+from rest_framework.compat import yaml, etree
 from rest_framework.exceptions import ParseError
 from rest_framework.compat import six
-from xml.etree import ElementTree as ET
-from xml.parsers.expat import ExpatError
 import json
 import datetime
 import decimal
@@ -80,6 +78,8 @@ class YAMLParser(BaseParser):
         `data` will be an object which is the parsed content of the response.
         `files` will always be `None`.
         """
+        assert yaml, 'YAMLParser requires pyyaml to be installed'
+
         parser_context = parser_context or {}
         encoding = parser_context.get('encoding', settings.DEFAULT_CHARSET)
 
@@ -146,12 +146,14 @@ class XMLParser(BaseParser):
     media_type = 'application/xml'
 
     def parse(self, stream, media_type=None, parser_context=None):
+        assert etree, 'XMLParser requires defusedxml to be installed'
+
         parser_context = parser_context or {}
         encoding = parser_context.get('encoding', settings.DEFAULT_CHARSET)
-        parser = ET_XMLParser(encoding=encoding)
+        parser = etree.DefusedXMLParser(encoding=encoding)
         try:
-            tree = ET.parse(stream, parser=parser)
-        except (ExpatError, ETParseError, ValueError) as exc:
+            tree = etree.parse(stream, parser=parser, forbid_dtd=True)
+        except (etree.ParseError, ValueError) as exc:
             raise ParseError('XML parse error - %s' % six.u(exc))
         data = self._xml_convert(tree.getroot())
 

rest_framework/tests/parsers.py

@@ -2,6 +2,8 @@ from __future__ import unicode_literals
 from rest_framework.compat import StringIO
 from django import forms
 from django.test import TestCase
+from django.utils import unittest
+from rest_framework.compat import etree
 from rest_framework.parsers import FormParser
 from rest_framework.parsers import XMLParser
 import datetime
@@ -69,11 +71,13 @@ class TestXMLParser(TestCase):
             ]
         }
 
+    @unittest.skipUnless(etree, 'defusedxml not installed')
     def test_parse(self):
         parser = XMLParser()
         data = parser.parse(self._input)
         self.assertEqual(data, self._data)
 
+    @unittest.skipUnless(etree, 'defusedxml not installed')
     def test_complex_data_parse(self):
         parser = XMLParser()
         data = parser.parse(self._complex_data_input)

rest_framework/tests/renderers.py

@@ -1,23 +1,21 @@
-import pickle
-import re
-
+from decimal import Decimal
 from django.core.cache import cache
 from django.test import TestCase
 from django.test.client import RequestFactory
-
+from django.utils import unittest
 from rest_framework import status, permissions
-from rest_framework.compat import yaml, patterns, url, include
+from rest_framework.compat import yaml, etree, patterns, url, include
 from rest_framework.response import Response
 from rest_framework.views import APIView
 from rest_framework.renderers import BaseRenderer, JSONRenderer, YAMLRenderer, \
     XMLRenderer, JSONPRenderer, BrowsableAPIRenderer
 from rest_framework.parsers import YAMLParser, XMLParser
 from rest_framework.settings import api_settings
-
 from rest_framework.compat import StringIO
 from rest_framework.compat import six
 import datetime
-from decimal import Decimal
+import pickle
+import re
 
 
 DUMMYSTATUS = status.HTTP_200_OK
@@ -410,6 +408,7 @@ class XMLRendererTestCase(TestCase):
         self.assertXMLContains(content, '<sub_name>first</sub_name>')
         self.assertXMLContains(content, '<sub_name>second</sub_name>')
 
+    @unittest.skipUnless(etree, 'defusedxml not installed')
     def test_render_and_parse_complex_data(self):
         """
         Test XML rendering.

tox.ini

@@ -9,11 +9,13 @@ commands = {envpython} rest_framework/runtests/runtests.py
 basepython = python3.3
 deps = https://www.djangoproject.com/download/1.5c1/tarball/
        https://github.com/alex/django-filter/archive/master.tar.gz
+       defusedxml==0.3
 
 [testenv:py3.2-django1.5]
 basepython = python3.2
 deps = https://www.djangoproject.com/download/1.5c1/tarball/
        https://github.com/alex/django-filter/archive/master.tar.gz
+       defusedxml==0.3
 
 [testenv:py2.7-django1.5]
 basepython = python2.7
@@ -24,23 +26,28 @@ deps = https://www.djangoproject.com/download/1.5c1/tarball/
 basepython = python2.6
 deps = https://www.djangoproject.com/download/1.5c1/tarball/
        django-filter==0.5.4
+       defusedxml==0.3
 
 [testenv:py2.7-django1.4]
 basepython = python2.7
 deps = django==1.4.3
        django-filter==0.5.4
+       defusedxml==0.3
 
 [testenv:py2.6-django1.4]
 basepython = python2.6
 deps = django==1.4.3
        django-filter==0.5.4
+       defusedxml==0.3
 
 [testenv:py2.7-django1.3]
 basepython = python2.7
 deps = django==1.3.5
        django-filter==0.5.4
+       defusedxml==0.3
 
 [testenv:py2.6-django1.3]
 basepython = python2.6
 deps = django==1.3.5
        django-filter==0.5.4
+       defusedxml==0.3