From 929684d6ee0efb5afad51dc584489d0437d2451b Mon Sep 17 00:00:00 2001 From: Andre Cruz Date: Wed, 7 Jan 2015 18:41:29 +0000 Subject: [PATCH] Fixed #21231 -- Enforced a max size for GET/POST values read into memory. Thanks Tom Christie for review. --- django/conf/global_settings.py | 8 + django/core/exceptions.py | 16 ++ django/http/multipartparser.py | 37 +++- django/http/request.py | 27 ++- django/utils/http.py | 60 +++++++ docs/ref/exceptions.txt | 2 + docs/ref/settings.txt | 49 ++++++ docs/releases/1.10.txt | 13 ++ tests/requests/test_data_upload_settings.py | 186 ++++++++++++++++++++ 9 files changed, 387 insertions(+), 11 deletions(-) create mode 100644 tests/requests/test_data_upload_settings.py 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()