Log injection

PoC (exploit vector for log injection via response logging): Prerequisites: A Django app running a version prior to the fix where response logging does not escape untrusted values. Attack vector: craft a request whose path contains a CRLF sequence to inject a fake log line in the server logs. Steps: 1) Start the server (vulnerable version). 2) Send a crafted HTTP request that triggers a 405 Method Not Allowed for a path containing a CRLF, e.g. GET /example/%0D%0AINJECTED HTTP/1.1 3) Check the Django log (e.g., django.request) and observe a log line containing the injected content as a separate line due to the unescaped CRLF. Example Python (attack against vulnerable behavior): import requests url = 'http://localhost:8000/example/%0D%0AINJECTED' resp = requests.get(url) print(resp.status_code) Post-fix behavior (as committed): logs are produced via log_response() which escapes arguments, so the injected CRLF would be sanitized and not create a new log line.

diff --git a/django/views/generic/base.py b/django/views/generic/base.py index 8f8f9397e8c0..8412288be1ce 100644 --- a/django/views/generic/base.py +++ b/django/views/generic/base.py @@ -14,6 +14,7 @@ from django.urls import reverse from django.utils.decorators import classonlymethod from django.utils.functional import classproperty +from django.utils.log import log_response logger = logging.getLogger("django.request") @@ -143,13 +144,14 @@ def dispatch(self, request, *args, **kwargs): return handler(request, *args, **kwargs) def http_method_not_allowed(self, request, *args, **kwargs): - logger.warning( + response = HttpResponseNotAllowed(self._allowed_methods()) + log_response( "Method Not Allowed (%s): %s", request.method, request.path, - extra={"status_code": 405, "request": request}, + response=response, + request=request, ) - response = HttpResponseNotAllowed(self._allowed_methods()) if self.view_is_async: @@ -261,10 +263,9 @@ def get(self, request, *args, **kwargs): else: return HttpResponseRedirect(url) else: - logger.warning( - "Gone: %s", request.path, extra={"status_code": 410, "request": request} - ) - return HttpResponseGone() + response = HttpResponseGone() + log_response("Gone: %s", request.path, response=response, request=request) + return response def head(self, request, *args, **kwargs): return self.get(request, *args, **kwargs) diff --git a/docs/releases/4.2.23.txt b/docs/releases/4.2.23.txt new file mode 100644 index 000000000000..e4232f9beaad --- /dev/null +++ b/docs/releases/4.2.23.txt @@ -0,0 +1,14 @@ +=========================== +Django 4.2.23 release notes +=========================== + +*June 10, 2025* + +Django 4.2.23 fixes a potential log injection issue in 4.2.22. + +Bugfixes +======== + +* Fixed a log injection possibility by migrating remaining response logging + to ``django.utils.log.log_response()``, which safely escapes arguments such + as the request path to prevent unsafe log output (:cve:`2025-48432`). diff --git a/docs/releases/5.1.11.txt b/docs/releases/5.1.11.txt new file mode 100644 index 000000000000..de44dc0e679e --- /dev/null +++ b/docs/releases/5.1.11.txt @@ -0,0 +1,14 @@ +=========================== +Django 5.1.11 release notes +=========================== + +*June 10, 2025* + +Django 5.1.11 fixes a potential log injection issue in 5.1.10. + +Bugfixes +======== + +* Fixed a log injection possibility by migrating remaining response logging + to ``django.utils.log.log_response()``, which safely escapes arguments such + as the request path to prevent unsafe log output (:cve:`2025-48432`). diff --git a/docs/releases/5.2.3.txt b/docs/releases/5.2.3.txt index fa0b4163c0b5..5aaa7fd2dd1b 100644 --- a/docs/releases/5.2.3.txt +++ b/docs/releases/5.2.3.txt @@ -2,7 +2,7 @@ Django 5.2.3 release notes ========================== -*Expected July 2, 2025* +*June 10, 2025* Django 5.2.3 fixes several bugs in 5.2.2. Also, the latest string translations from Transifex are incorporated. @@ -10,4 +10,6 @@ from Transifex are incorporated. Bugfixes ======== -* ... +* Fixed a log injection possibility by migrating remaining response logging + to ``django.utils.log.log_response()``, which safely escapes arguments such + as the request path to prevent unsafe log output (:cve:`2025-48432`). diff --git a/docs/releases/index.txt b/docs/releases/index.txt index 8e6c94161f6d..6cbf2306bb88 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -42,6 +42,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 5.1.11 5.1.10 5.1.9 5.1.8 @@ -81,6 +82,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 4.2.23 4.2.22 4.2.21 4.2.20 diff --git a/tests/generic_views/test_base.py b/tests/generic_views/test_base.py index 5f3941196b26..acd938935ae6 100644 --- a/tests/generic_views/test_base.py +++ b/tests/generic_views/test_base.py @@ -1,5 +1,8 @@ +import logging import time +from logging_tests.tests import LoggingAssertionMixin + from django.core.exceptions import ImproperlyConfigured from django.http import HttpResponse from django.test import RequestFactory, SimpleTestCase, override_settings @@ -63,7 +66,7 @@ def get(self, request): return self -class ViewTest(SimpleTestCase): +class ViewTest(LoggingAssertionMixin, SimpleTestCase): rf = RequestFactory() def _assert_simple(self, response): @@ -297,6 +300,25 @@ def test_direct_instantiation(self): response = view.dispatch(self.rf.head("/")) self.assertEqual(response.status_code, 405) + def test_method_not_allowed_response_logged(self): + for path, escaped in [ + ("/foo/", "/foo/"), + (r"/%1B[1;31mNOW IN RED!!!1B[0m/", r"/\x1b[1;31mNOW IN RED!!!1B[0m/"), + ]: + with self.subTest(path=path): + request = self.rf.get(path, REQUEST_METHOD="BOGUS") + with self.assertLogs("django.request", "WARNING") as handler: + response = SimpleView.as_view()(request) + + self.assertLogRecord( + handler, + f"Method Not Allowed (BOGUS): {escaped}", + logging.WARNING, + 405, + request, + ) + self.assertEqual(response.status_code, 405) + @override_settings(ROOT_URLCONF="generic_views.urls") class TemplateViewTest(SimpleTestCase): @@ -425,7 +447,7 @@ def test_extra_context(self): @override_settings(ROOT_URLCONF="generic_views.urls") -class RedirectViewTest(SimpleTestCase): +class RedirectViewTest(LoggingAssertionMixin, SimpleTestCase): rf = RequestFactory() def test_no_url(self): @@ -549,6 +571,20 @@ def test_direct_instantiation(self): response = view.dispatch(self.rf.head("/foo/")) self.assertEqual(response.status_code, 410) + def test_gone_response_logged(self): + for path, escaped in [ + ("/foo/", "/foo/"), + (r"/%1B[1;31mNOW IN RED!!!1B[0m/", r"/\x1b[1;31mNOW IN RED!!!1B[0m/"), + ]: + with self.subTest(path=path): + request = self.rf.get(path) + with self.assertLogs("django.request", "WARNING") as handler: + RedirectView().dispatch(request) + + self.assertLogRecord( + handler, f"Gone: {escaped}", logging.WARNING, 410, request + ) + class GetContextDataTest(SimpleTestCase): def test_get_context_data_super(self):