From 8f6d431b08cbb418d9144b976e7b972546607851 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Tue, 16 Feb 2021 10:14:17 +0000 Subject: [PATCH] [3.1.x] Fixed CVE-2021-23336 -- Fixed web cache poisoning via django.utils.http.limited_parse_qsl(). --- django/utils/http.py | 2 +- docs/releases/2.2.19.txt | 16 +++++++ docs/releases/3.0.13.txt | 16 +++++++ docs/releases/3.1.7.txt | 13 +++++- docs/releases/index.txt | 2 + tests/handlers/test_exception.py | 2 +- tests/requests/test_data_upload_settings.py | 8 ++-- tests/utils_tests/test_http.py | 52 +++++++++++++++++++-- 8 files changed, 100 insertions(+), 11 deletions(-) create mode 100644 docs/releases/2.2.19.txt create mode 100644 docs/releases/3.0.13.txt diff --git a/django/utils/http.py b/django/utils/http.py index c1005458e7..bc6aade3ef 100644 --- a/django/utils/http.py +++ b/django/utils/http.py @@ -42,7 +42,7 @@ ASCTIME_DATE = _lazy_re_compile(r'^\w{3} %s %s %s %s$' % (__M, __D2, __T, __Y)) RFC3986_GENDELIMS = ":/?#[]@" RFC3986_SUBDELIMS = "!$&'()*+,;=" -FIELDS_MATCH = _lazy_re_compile('[&;]') +FIELDS_MATCH = _lazy_re_compile('&') @keep_lazy_text diff --git a/docs/releases/2.2.19.txt b/docs/releases/2.2.19.txt new file mode 100644 index 0000000000..feaffd996c --- /dev/null +++ b/docs/releases/2.2.19.txt @@ -0,0 +1,16 @@ +=========================== +Django 2.2.19 release notes +=========================== + +*February 19, 2021* + +Django 2.2.19 fixes a security issue in 2.2.18. + +CVE-2021-23336: Web cache poisoning via ``django.utils.http.limited_parse_qsl()`` +================================================================================= + +Django contains a copy of :func:`urllib.parse.parse_qsl` which was added to +backport some security fixes. A further security fix has been issued recently +such that ``parse_qsl()`` no longer allows using ``;`` as a query parameter +separator by default. Django now includes this fix. See :bpo:`42967` for +further details. diff --git a/docs/releases/3.0.13.txt b/docs/releases/3.0.13.txt new file mode 100644 index 0000000000..c78b8a04fd --- /dev/null +++ b/docs/releases/3.0.13.txt @@ -0,0 +1,16 @@ +=========================== +Django 3.0.13 release notes +=========================== + +*February 19, 2021* + +Django 3.0.13 fixes a security issue in 3.0.12. + +CVE-2021-23336: Web cache poisoning via ``django.utils.http.limited_parse_qsl()`` +================================================================================= + +Django contains a copy of :func:`urllib.parse.parse_qsl` which was added to +backport some security fixes. A further security fix has been issued recently +such that ``parse_qsl()`` no longer allows using ``;`` as a query parameter +separator by default. Django now includes this fix. See :bpo:`42967` for +further details. diff --git a/docs/releases/3.1.7.txt b/docs/releases/3.1.7.txt index 1ef16b76c7..e58fc23e80 100644 --- a/docs/releases/3.1.7.txt +++ b/docs/releases/3.1.7.txt @@ -2,9 +2,18 @@ Django 3.1.7 release notes ========================== -*Expected March 1, 2021* +*February 19, 2021* -Django 3.1.7 fixes several bugs in 3.1.6. +Django 3.1.7 fixes a security issue and a bug in 3.1.6. + +CVE-2021-23336: Web cache poisoning via ``django.utils.http.limited_parse_qsl()`` +================================================================================= + +Django contains a copy of :func:`urllib.parse.parse_qsl` which was added to +backport some security fixes. A further security fix has been issued recently +such that ``parse_qsl()`` no longer allows using ``;`` as a query parameter +separator by default. Django now includes this fix. See :bpo:`42967` for +further details. Bugfixes ======== diff --git a/docs/releases/index.txt b/docs/releases/index.txt index dd246f00a3..8bd30dc7b4 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -39,6 +39,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 3.0.13 3.0.12 3.0.11 3.0.10 @@ -58,6 +59,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 2.2.19 2.2.18 2.2.17 2.2.16 diff --git a/tests/handlers/test_exception.py b/tests/handlers/test_exception.py index 7afd4acc6b..0c1e763990 100644 --- a/tests/handlers/test_exception.py +++ b/tests/handlers/test_exception.py @@ -6,7 +6,7 @@ from django.test.client import FakePayload class ExceptionHandlerTests(SimpleTestCase): def get_suspicious_environ(self): - payload = FakePayload('a=1&a=2;a=3\r\n') + payload = FakePayload('a=1&a=2&a=3\r\n') return { 'REQUEST_METHOD': 'POST', 'CONTENT_TYPE': 'application/x-www-form-urlencoded', diff --git a/tests/requests/test_data_upload_settings.py b/tests/requests/test_data_upload_settings.py index f60f1850ea..44897cc9fa 100644 --- a/tests/requests/test_data_upload_settings.py +++ b/tests/requests/test_data_upload_settings.py @@ -11,7 +11,7 @@ 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') + payload = FakePayload('a=1&a=2&a=3\r\n') self.request = WSGIRequest({ 'REQUEST_METHOD': 'POST', 'CONTENT_TYPE': 'application/x-www-form-urlencoded', @@ -117,7 +117,7 @@ class DataUploadMaxNumberOfFieldsGet(SimpleTestCase): request = WSGIRequest({ 'REQUEST_METHOD': 'GET', 'wsgi.input': BytesIO(b''), - 'QUERY_STRING': 'a=1&a=2;a=3', + 'QUERY_STRING': 'a=1&a=2&a=3', }) request.GET['a'] @@ -126,7 +126,7 @@ class DataUploadMaxNumberOfFieldsGet(SimpleTestCase): request = WSGIRequest({ 'REQUEST_METHOD': 'GET', 'wsgi.input': BytesIO(b''), - 'QUERY_STRING': 'a=1&a=2;a=3', + 'QUERY_STRING': 'a=1&a=2&a=3', }) request.GET['a'] @@ -168,7 +168,7 @@ class DataUploadMaxNumberOfFieldsMultipartPost(SimpleTestCase): class DataUploadMaxNumberOfFieldsFormPost(SimpleTestCase): def setUp(self): - payload = FakePayload("\r\n".join(['a=1&a=2;a=3', ''])) + payload = FakePayload("\r\n".join(['a=1&a=2&a=3', ''])) self.request = WSGIRequest({ 'REQUEST_METHOD': 'POST', 'CONTENT_TYPE': 'application/x-www-form-urlencoded', diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py index aa9f194a8a..0ec14a6819 100644 --- a/tests/utils_tests/test_http.py +++ b/tests/utils_tests/test_http.py @@ -3,14 +3,16 @@ import unittest from datetime import datetime from unittest import mock +from django.core.exceptions import TooManyFieldsSent from django.test import SimpleTestCase, ignore_warnings from django.utils.datastructures import MultiValueDict from django.utils.deprecation import RemovedInDjango40Warning from django.utils.http import ( base36_to_int, escape_leading_slashes, http_date, int_to_base36, - is_safe_url, is_same_domain, parse_etags, parse_http_date, quote_etag, - url_has_allowed_host_and_scheme, urlencode, urlquote, urlquote_plus, - urlsafe_base64_decode, urlsafe_base64_encode, urlunquote, urlunquote_plus, + is_safe_url, is_same_domain, limited_parse_qsl, parse_etags, + parse_http_date, quote_etag, url_has_allowed_host_and_scheme, urlencode, + urlquote, urlquote_plus, urlsafe_base64_decode, urlsafe_base64_encode, + urlunquote, urlunquote_plus, ) @@ -359,3 +361,47 @@ class EscapeLeadingSlashesTests(unittest.TestCase): for url, expected in tests: with self.subTest(url=url): self.assertEqual(escape_leading_slashes(url), expected) + + +# Backport of unit tests for urllib.parse.parse_qsl() from Python 3.8.8. +# Copyright (C) 2021 Python Software Foundation (see LICENSE.python). +class ParseQSLBackportTests(unittest.TestCase): + def test_parse_qsl(self): + tests = [ + ('', []), + ('&', []), + ('&&', []), + ('=', [('', '')]), + ('=a', [('', 'a')]), + ('a', [('a', '')]), + ('a=', [('a', '')]), + ('&a=b', [('a', 'b')]), + ('a=a+b&b=b+c', [('a', 'a b'), ('b', 'b c')]), + ('a=1&a=2', [('a', '1'), ('a', '2')]), + (';a=b', [(';a', 'b')]), + ('a=a+b;b=b+c', [('a', 'a b;b=b c')]), + ] + for original, expected in tests: + with self.subTest(original): + result = limited_parse_qsl(original, keep_blank_values=True) + self.assertEqual(result, expected, 'Error parsing %r' % original) + expect_without_blanks = [v for v in expected if len(v[1])] + result = limited_parse_qsl(original, keep_blank_values=False) + self.assertEqual(result, expect_without_blanks, 'Error parsing %r' % original) + + def test_parse_qsl_encoding(self): + result = limited_parse_qsl('key=\u0141%E9', encoding='latin-1') + self.assertEqual(result, [('key', '\u0141\xE9')]) + result = limited_parse_qsl('key=\u0141%C3%A9', encoding='utf-8') + self.assertEqual(result, [('key', '\u0141\xE9')]) + result = limited_parse_qsl('key=\u0141%C3%A9', encoding='ascii') + self.assertEqual(result, [('key', '\u0141\ufffd\ufffd')]) + result = limited_parse_qsl('key=\u0141%E9-', encoding='ascii') + self.assertEqual(result, [('key', '\u0141\ufffd-')]) + result = limited_parse_qsl('key=\u0141%E9-', encoding='ascii', errors='ignore') + self.assertEqual(result, [('key', '\u0141-')]) + + def test_parse_qsl_field_limit(self): + with self.assertRaises(TooManyFieldsSent): + limited_parse_qsl('&'.join(['a=a'] * 11), fields_limit=10) + limited_parse_qsl('&'.join(['a=a'] * 10), fields_limit=10)