Fixed #21231 -- Enforced a max size for GET/POST values read into memory.

Thanks Tom Christie for review.

9 files changed, 387 insertions, 11 deletions

diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py
index 1f2df0f093..1b71b1b1b8 100644
--- a/django/conf/global_settings.py
+++ b/django/conf/global_settings.py
@@ -285,6 +285,14 @@ FILE_UPLOAD_HANDLERS = [
 # file system instead of into memory.
 FILE_UPLOAD_MAX_MEMORY_SIZE = 2621440  # i.e. 2.5 MB
 
+# Maximum size in bytes of request data (excluding file uploads) that will be
+# read before a SuspiciousOperation (RequestDataTooBig) is raised.
+DATA_UPLOAD_MAX_MEMORY_SIZE = 2621440  # i.e. 2.5 MB
+
+# Maximum number of GET/POST parameters that will be read before a
+# SuspiciousOperation (TooManyFieldsSent) is raised.
+DATA_UPLOAD_MAX_NUMBER_FIELDS = 1000
+
 # Directory in which upload streamed files will be temporarily saved. A value of
 # `None` will make Django use the operating system's default temporary directory
 # (i.e. "/tmp" on *nix systems).
diff --git a/django/core/exceptions.py b/django/core/exceptions.py
index c92e8d4f2a..5b125e6fd5 100644
--- a/django/core/exceptions.py
+++ b/django/core/exceptions.py
@@ -53,6 +53,22 @@ class DisallowedRedirect(SuspiciousOperation):
     pass
 
 
+class TooManyFieldsSent(SuspiciousOperation):
+    """
+    The number of fields in a POST request exceeded
+    settings.DATA_UPLOAD_MAX_NUMBER_FIELDS.
+    """
+    pass
+
+
+class RequestDataTooBig(SuspiciousOperation):
+    """
+    The size of the request (excluding any file uploads) exceeded
+    settings.DATA_UPLOAD_MAX_MEMORY_SIZE.
+    """
+    pass
+
+
 class PermissionDenied(Exception):
     """The user did not have permission to do that"""
     pass
diff --git a/django/http/multipartparser.py b/django/http/multipartparser.py
index b54eaca976..1babc72c98 100644
--- a/django/http/multipartparser.py
+++ b/django/http/multipartparser.py
@@ -12,7 +12,9 @@ import cgi
 import sys
 
 from django.conf import settings
-from django.core.exceptions import SuspiciousMultipartForm
+from django.core.exceptions import (
+    RequestDataTooBig, SuspiciousMultipartForm, TooManyFieldsSent,
+)
 from django.core.files.uploadhandler import (
     SkipFile, StopFutureHandlers, StopUpload,
 )
@@ -145,6 +147,13 @@ class MultiPartParser(object):
         old_field_name = None
         counters = [0] * len(handlers)
 
+        # Number of bytes that have been read.
+        num_bytes_read = 0
+        # To count the number of keys in the request.
+        num_post_keys = 0
+        # To limit the amount of data read from the request.
+        read_size = None
+
         try:
             for item_type, meta_data, field_stream in Parser(stream, self._boundary):
                 if old_field_name:
@@ -166,15 +175,37 @@ class MultiPartParser(object):
                 field_name = force_text(field_name, encoding, errors='replace')
 
                 if item_type == FIELD:
+                    # Avoid storing more than DATA_UPLOAD_MAX_NUMBER_FIELDS.
+                    num_post_keys += 1
+                    if (settings.DATA_UPLOAD_MAX_NUMBER_FIELDS is not None and
+                            settings.DATA_UPLOAD_MAX_NUMBER_FIELDS < num_post_keys):
+                        raise TooManyFieldsSent(
+                            'The number of GET/POST parameters exceeded '
+                            'settings.DATA_UPLOAD_MAX_NUMBER_FIELDS.'
+                        )
+
+                    # Avoid reading more than DATA_UPLOAD_MAX_MEMORY_SIZE.
+                    if settings.DATA_UPLOAD_MAX_MEMORY_SIZE is not None:
+                        read_size = settings.DATA_UPLOAD_MAX_MEMORY_SIZE - num_bytes_read
+
                     # This is a post field, we can just set it in the post
                     if transfer_encoding == 'base64':
-                        raw_data = field_stream.read()
+                        raw_data = field_stream.read(size=read_size)
+                        num_bytes_read += len(raw_data)
                         try:
                             data = base64.b64decode(raw_data)
                         except _BASE64_DECODE_ERROR:
                             data = raw_data
                     else:
-                        data = field_stream.read()
+                        data = field_stream.read(size=read_size)
+                        num_bytes_read += len(data)
+
+                    # Add two here to make the check consistent with the
+                    # x-www-form-urlencoded check that includes '&='.
+                    num_bytes_read += len(field_name) + 2
+                    if (settings.DATA_UPLOAD_MAX_MEMORY_SIZE is not None and
+                            num_bytes_read > settings.DATA_UPLOAD_MAX_MEMORY_SIZE):
+                        raise RequestDataTooBig('Request body exceeded settings.DATA_UPLOAD_MAX_MEMORY_SIZE.')
 
                     self._post.appendlist(field_name,
                                           force_text(data, encoding, errors='replace'))
diff --git a/django/http/request.py b/django/http/request.py
index d8f5cd7952..f3754ba2ff 100644
--- a/django/http/request.py
+++ b/django/http/request.py
@@ -8,7 +8,9 @@ from itertools import chain
 
 from django.conf import settings
 from django.core import signing
-from django.core.exceptions import DisallowedHost, ImproperlyConfigured
+from django.core.exceptions import (
+    DisallowedHost, ImproperlyConfigured, RequestDataTooBig,
+)
 from django.core.files import uploadhandler
 from django.http.multipartparser import MultiPartParser, MultiPartParserError
 from django.utils import six
@@ -16,9 +18,9 @@ from django.utils.datastructures import ImmutableList, MultiValueDict
 from django.utils.encoding import (
     escape_uri_path, force_bytes, force_str, force_text, iri_to_uri,
 )
-from django.utils.http import is_same_domain
+from django.utils.http import is_same_domain, limited_parse_qsl
 from django.utils.six.moves.urllib.parse import (
-    parse_qsl, quote, urlencode, urljoin, urlsplit,
+    quote, urlencode, urljoin, urlsplit,
 )
 
 RAISE_ERROR = object()
@@ -259,6 +261,12 @@ class HttpRequest(object):
         if not hasattr(self, '_body'):
             if self._read_started:
                 raise RawPostDataException("You cannot access body after reading from request's data stream")
+
+            # Limit the maximum request data size that will be handled in-memory.
+            if (settings.DATA_UPLOAD_MAX_MEMORY_SIZE is not None and
+                    int(self.META.get('CONTENT_LENGTH', 0)) > settings.DATA_UPLOAD_MAX_MEMORY_SIZE):
+                raise RequestDataTooBig('Request body exceeded settings.DATA_UPLOAD_MAX_MEMORY_SIZE.')
+
             try:
                 self._body = self.read()
             except IOError as e:
@@ -368,6 +376,12 @@ class QueryDict(MultiValueDict):
         if not encoding:
             encoding = settings.DEFAULT_CHARSET
         self.encoding = encoding
+        query_string = query_string or ''
+        parse_qsl_kwargs = {
+            'keep_blank_values': True,
+            'fields_limit': settings.DATA_UPLOAD_MAX_NUMBER_FIELDS,
+            'encoding': encoding,
+        }
         if six.PY3:
             if isinstance(query_string, bytes):
                 # query_string normally contains URL-encoded data, a subset of ASCII.
@@ -376,13 +390,10 @@ class QueryDict(MultiValueDict):
                 except UnicodeDecodeError:
                     # ... but some user agents are misbehaving :-(
                     query_string = query_string.decode('iso-8859-1')
-            for key, value in parse_qsl(query_string or '',
-                                        keep_blank_values=True,
-                                        encoding=encoding):
+            for key, value in limited_parse_qsl(query_string, **parse_qsl_kwargs):
                 self.appendlist(key, value)
         else:
-            for key, value in parse_qsl(query_string or '',
-                                        keep_blank_values=True):
+            for key, value in limited_parse_qsl(query_string, **parse_qsl_kwargs):
                 try:
                     value = value.decode(encoding)
                 except UnicodeDecodeError:
diff --git a/django/utils/http.py b/django/utils/http.py
index 31de726184..151e85de92 100644
--- a/django/utils/http.py
+++ b/django/utils/http.py
@@ -9,6 +9,7 @@ import unicodedata
 from binascii import Error as BinasciiError
 from email.utils import formatdate
 
+from django.core.exceptions import TooManyFieldsSent
 from django.utils import six
 from django.utils.datastructures import MultiValueDict
 from django.utils.encoding import force_bytes, force_str, force_text
@@ -34,6 +35,8 @@ ASCTIME_DATE = re.compile(r'^\w{3} %s %s %s %s$' % (__M, __D2, __T, __Y))
 RFC3986_GENDELIMS = str(":/?#[]@")
 RFC3986_SUBDELIMS = str("!$&'()*+,;=")
 
+FIELDS_MATCH = re.compile('[&;]')
+
 
 @keep_lazy_text
 def urlquote(url, safe='/'):
@@ -314,3 +317,60 @@ def _is_safe_url(url, host):
         return False
     return ((not url_info.netloc or url_info.netloc == host) and
             (not url_info.scheme or url_info.scheme in ['http', 'https']))
+
+
+def limited_parse_qsl(qs, keep_blank_values=False, encoding='utf-8',
+                      errors='replace', fields_limit=None):
+    """
+    Return a list of key/value tuples parsed from query string.
+
+    Copied from urlparse with an additional "fields_limit" argument.
+    Copyright (C) 2013 Python Software Foundation (see LICENSE.python).
+
+    Arguments:
+
+    qs: percent-encoded query string to be parsed
+
+    keep_blank_values: flag indicating whether blank values in
+        percent-encoded queries should be treated as blank strings. A
+        true value indicates that blanks should be retained as blank
+        strings. The default false value indicates that blank values
+        are to be ignored and treated as if they were  not included.
+
+    encoding and errors: specify how to decode percent-encoded sequences
+        into Unicode characters, as accepted by the bytes.decode() method.
+
+    fields_limit: maximum number of fields parsed or an exception
+        is raised. None means no limit and is the default.
+    """
+    if fields_limit:
+        pairs = FIELDS_MATCH.split(qs, fields_limit)
+        if len(pairs) > fields_limit:
+            raise TooManyFieldsSent(
+                'The number of GET/POST parameters exceeded '
+                'settings.DATA_UPLOAD_MAX_NUMBER_FIELDS.'
+            )
+    else:
+        pairs = FIELDS_MATCH.split(qs)
+    r = []
+    for name_value in pairs:
+        if not name_value:
+            continue
+        nv = name_value.split(str('='), 1)
+        if len(nv) != 2:
+            # Handle case of a control-name with no equal sign
+            if keep_blank_values:
+                nv.append('')
+            else:
+                continue
+        if len(nv[1]) or keep_blank_values:
+            if six.PY3:
+                name = nv[0].replace('+', ' ')
+                name = unquote(name, encoding=encoding, errors=errors)
+                value = nv[1].replace('+', ' ')
+                value = unquote(value, encoding=encoding, errors=errors)
+            else:
+                name = unquote(nv[0].replace(b'+', b' '))
+                value = unquote(nv[1].replace(b'+', b' '))
+            r.append((name, value))
+    return r
diff --git a/docs/ref/exceptions.txt b/docs/ref/exceptions.txt
index 37424fe7c6..c222010172 100644
--- a/docs/ref/exceptions.txt
+++ b/docs/ref/exceptions.txt
@@ -61,9 +61,11 @@ Django core exception classes are defined in ``django.core.exceptions``.
     * ``DisallowedModelAdminToField``
     * ``DisallowedRedirect``
     * ``InvalidSessionKey``
+    * ``RequestDataTooBig``
     * ``SuspiciousFileOperation``
     * ``SuspiciousMultipartForm``
     * ``SuspiciousSession``
+    * ``TooManyFieldsSent``
 
     If a ``SuspiciousOperation`` exception reaches the WSGI handler level it is
     logged at the ``Error`` level and results in
diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt
index 7423d7d360..838dde808a 100644
--- a/docs/ref/settings.txt
+++ b/docs/ref/settings.txt
@@ -880,6 +880,51 @@ This is an Oracle-specific setting.
 
 The maximum size that the DATAFILE_TMP is allowed to grow to.
 
+.. setting:: DATA_UPLOAD_MAX_MEMORY_SIZE
+
+DATA_UPLOAD_MAX_MEMORY_SIZE
+---------------------------
+
+.. versionadded:: 1.10
+
+Default: ``2621440`` (i.e. 2.5 MB).
+
+The maximum size in bytes that a request body may be before a
+:exc:`~django.core.exceptions.SuspiciousOperation` (``RequestDataTooBig``) is
+raised. The check is done when accessing ``request.body`` or ``request.POST``
+and is calculated against the total request size excluding any file upload
+data. You can set this to ``None`` to disable the check. Applications that are
+expected to receive unusually large form posts should tune this setting.
+
+The amount of request data is correlated to the amount of memory needed to
+process the request and populate the GET and POST dictionaries. Large requests
+could be used as a denial-of-service attack vector if left unchecked. Since web
+servers don't typically perform deep request inspection, it's not possible to
+perform a similar check at that level.
+
+See also :setting:`FILE_UPLOAD_MAX_MEMORY_SIZE`.
+
+.. setting:: DATA_UPLOAD_MAX_NUMBER_FIELDS
+
+DATA_UPLOAD_MAX_NUMBER_FIELDS
+-----------------------------
+
+.. versionadded:: 1.10
+
+Default: ``1000``
+
+The maximum number of parameters that may be received via GET or POST before a
+:exc:`~django.core.exceptions.SuspiciousOperation` (``TooManyFields``) is
+raised. You can set this to ``None`` to disable the check. Applications that
+are expected to receive an unusually large number of form fields should tune
+this setting.
+
+The number of request parameters is correlated to the amount of time needed to
+process the request and populate the GET and POST dictionaries. Large requests
+could be used as a denial-of-service attack vector if left unchecked. Since web
+servers don't typically perform deep request inspection, it's not possible to
+perform a similar check at that level.
+
 .. setting:: DATABASE_ROUTERS
 
 ``DATABASE_ROUTERS``
@@ -1325,6 +1370,8 @@ Default: ``2621440`` (i.e. 2.5 MB).
 The maximum size (in bytes) that an upload will be before it gets streamed to
 the file system. See :doc:`/topics/files` for details.
 
+See also :setting:`DATA_UPLOAD_MAX_MEMORY_SIZE`.
+
 .. setting:: FILE_UPLOAD_DIRECTORY_PERMISSIONS
 
 ``FILE_UPLOAD_DIRECTORY_PERMISSIONS``
@@ -3258,6 +3305,8 @@ Globalization (``i18n``/``l10n``)
 
 HTTP
 ----
+* :setting:`DATA_UPLOAD_MAX_MEMORY_SIZE`
+* :setting:`DATA_UPLOAD_MAX_NUMBER_FIELDS`
 * :setting:`DEFAULT_CHARSET`
 * :setting:`DEFAULT_CONTENT_TYPE`
 * :setting:`DISALLOWED_USER_AGENTS`
diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt
index 410180f838..29af587eec 100644
--- a/docs/releases/1.10.txt
+++ b/docs/releases/1.10.txt
@@ -719,6 +719,19 @@ custom lookup for it. For example::
   ``POINT (23.0000000000000000 5.5000000000000000)``, you'll get
   ``POINT (23 5.5)``.
 
+Maximum size of a request body and the number of GET/POST parameters is limited
+-------------------------------------------------------------------------------
+
+Two new settings help mitigate denial-of-service attacks via large requests:
+
+* :setting:`DATA_UPLOAD_MAX_MEMORY_SIZE` limits the size that a request body
+  may be. File uploads don't count towards this limit.
+* :setting:`DATA_UPLOAD_MAX_NUMBER_FIELDS` limits the number of GET/POST
+  parameters that are parsed.
+
+Applications that receive unusually large form posts may need to tune these
+settings.
+
 Miscellaneous
 -------------
 
diff --git a/tests/requests/test_data_upload_settings.py b/tests/requests/test_data_upload_settings.py
new file mode 100644
index 0000000000..8855e13336
--- /dev/null
+++ b/tests/requests/test_data_upload_settings.py
@@ -0,0 +1,186 @@
+from io import BytesIO
+
+from django.core.exceptions import RequestDataTooBig, TooManyFieldsSent
+from django.core.handlers.wsgi import WSGIRequest
+from django.test import SimpleTestCase
+from django.test.client import FakePayload
+
+TOO_MANY_FIELDS_MSG = 'The number of GET/POST parameters exceeded settings.DATA_UPLOAD_MAX_NUMBER_FIELDS.'
+TOO_MUCH_DATA_MSG = 'Request body exceeded settings.DATA_UPLOAD_MAX_MEMORY_SIZE.'
+
+
+class DataUploadMaxMemorySizeFormPostTests(SimpleTestCase):
+    def setUp(self):
+        payload = FakePayload('a=1&a=2;a=3\r\n')
+        self.request = WSGIRequest({
+            'REQUEST_METHOD': 'POST',
+            'CONTENT_TYPE': 'application/x-www-form-urlencoded',
+            'CONTENT_LENGTH': len(payload),
+            'wsgi.input': payload,
+        })
+
+    def test_size_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=12):
+            with self.assertRaisesMessage(RequestDataTooBig, TOO_MUCH_DATA_MSG):
+                self.request._load_post_and_files()
+
+    def test_size_not_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=13):
+            self.request._load_post_and_files()
+
+    def test_no_limit(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=None):
+            self.request._load_post_and_files()
+
+
+class DataUploadMaxMemorySizeMultipartPostTests(SimpleTestCase):
+    def setUp(self):
+        payload = FakePayload("\r\n".join([
+            '--boundary',
+            'Content-Disposition: form-data; name="name"',
+            '',
+            'value',
+            '--boundary--'
+            ''
+        ]))
+        self.request = WSGIRequest({
+            'REQUEST_METHOD': 'POST',
+            'CONTENT_TYPE': 'multipart/form-data; boundary=boundary',
+            'CONTENT_LENGTH': len(payload),
+            'wsgi.input': payload,
+        })
+
+    def test_size_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=10):
+            with self.assertRaisesMessage(RequestDataTooBig, TOO_MUCH_DATA_MSG):
+                self.request._load_post_and_files()
+
+    def test_size_not_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=11):
+            self.request._load_post_and_files()
+
+    def test_no_limit(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=None):
+            self.request._load_post_and_files()
+
+    def test_file_passes(self):
+        payload = FakePayload("\r\n".join([
+            '--boundary',
+            'Content-Disposition: form-data; name="file1"; filename="test.file"',
+            '',
+            'value',
+            '--boundary--'
+            ''
+        ]))
+        request = WSGIRequest({
+            'REQUEST_METHOD': 'POST',
+            'CONTENT_TYPE': 'multipart/form-data; boundary=boundary',
+            'CONTENT_LENGTH': len(payload),
+            'wsgi.input': payload,
+        })
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=1):
+            request._load_post_and_files()
+            self.assertIn('file1', request.FILES, "Upload file not present")
+
+
+class DataUploadMaxMemorySizeGetTests(SimpleTestCase):
+    def setUp(self):
+        self.request = WSGIRequest({
+            'REQUEST_METHOD': 'GET',
+            'wsgi.input': BytesIO(b''),
+            'CONTENT_LENGTH': 3,
+        })
+
+    def test_data_upload_max_memory_size_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=2):
+            with self.assertRaisesMessage(RequestDataTooBig, TOO_MUCH_DATA_MSG):
+                self.request.body
+
+    def test_size_not_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=3):
+            self.request.body
+
+    def test_no_limit(self):
+        with self.settings(DATA_UPLOAD_MAX_MEMORY_SIZE=None):
+            self.request.body
+
+
+class DataUploadMaxNumberOfFieldsGet(SimpleTestCase):
+
+    def test_get_max_fields_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=1):
+            with self.assertRaisesMessage(TooManyFieldsSent, TOO_MANY_FIELDS_MSG):
+                request = WSGIRequest({
+                    'REQUEST_METHOD': 'GET',
+                    'wsgi.input': BytesIO(b''),
+                    'QUERY_STRING': 'a=1&a=2;a=3',
+                })
+                request.GET['a']
+
+    def test_get_max_fields_not_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=3):
+            request = WSGIRequest({
+                'REQUEST_METHOD': 'GET',
+                'wsgi.input': BytesIO(b''),
+                'QUERY_STRING': 'a=1&a=2;a=3',
+            })
+            request.GET['a']
+
+
+class DataUploadMaxNumberOfFieldsMultipartPost(SimpleTestCase):
+    def setUp(self):
+        payload = FakePayload("\r\n".join([
+            '--boundary',
+            'Content-Disposition: form-data; name="name1"',
+            '',
+            'value1',
+            '--boundary',
+            'Content-Disposition: form-data; name="name2"',
+            '',
+            'value2',
+            '--boundary--'
+            ''
+        ]))
+        self.request = WSGIRequest({
+            'REQUEST_METHOD': 'POST',
+            'CONTENT_TYPE': 'multipart/form-data; boundary=boundary',
+            'CONTENT_LENGTH': len(payload),
+            'wsgi.input': payload,
+        })
+
+    def test_number_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=1):
+            with self.assertRaisesMessage(TooManyFieldsSent, TOO_MANY_FIELDS_MSG):
+                self.request._load_post_and_files()
+
+    def test_number_not_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=2):
+            self.request._load_post_and_files()
+
+    def test_no_limit(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=None):
+            self.request._load_post_and_files()
+
+
+class DataUploadMaxNumberOfFieldsFormPost(SimpleTestCase):
+    def setUp(self):
+        payload = FakePayload("\r\n".join(['a=1&a=2;a=3', '']))
+        self.request = WSGIRequest({
+            'REQUEST_METHOD': 'POST',
+            'CONTENT_TYPE': 'application/x-www-form-urlencoded',
+            'CONTENT_LENGTH': len(payload),
+            'wsgi.input': payload,
+        })
+
+    def test_number_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=2):
+            with self.assertRaisesMessage(TooManyFieldsSent, TOO_MANY_FIELDS_MSG):
+                self.request._load_post_and_files()
+
+    def test_number_not_exceeded(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=3):
+            self.request._load_post_and_files()
+
+    def test_no_limit(self):
+        with self.settings(DATA_UPLOAD_MAX_NUMBER_FIELDS=None):
+            self.request._load_post_and_files()