Security: avoid requests with NULL characters (0x00) on GET (#9350)

Author: humitos

readthedocs/core/middleware.py

@@ -1,6 +1,6 @@
-import structlog
 import time
 
+import structlog
 from django.conf import settings
 from django.contrib.sessions.backends.base import SessionBase, UpdateError
 from django.contrib.sessions.middleware import SessionMiddleware
@@ -11,7 +11,6 @@
 )
 from django.utils.cache import patch_vary_headers
 from django.utils.http import http_date
-from django.utils.translation import gettext_lazy as _
 
 log = structlog.get_logger(__name__)
 
@@ -192,3 +191,27 @@ def __call__(self, request):
         response = self.get_response(request)
         response['Referrer-Policy'] = settings.SECURE_REFERRER_POLICY
         return response
+
+
+class NullCharactersMiddleware:
+
+    """
+    Block all requests that contains NULL characters (0x00) on their GET attributes.
+
+    Requests containing NULL characters make our code to break. In particular,
+    when trying to save the content containing a NULL character into the
+    database, producing a 500 and creating an event in Sentry.
+
+    NULL characters are also used as an explotation technique, known as "Null Byte Injection".
+    """
+
+    def __init__(self, get_response):
+        self.get_response = get_response
+
+    def __call__(self, request):
+        for key, value in request.GET.items():
+            if "\x00" in value:
+                raise SuspiciousOperation(
+                    f"There are NULL (0x00) characters in at least one of the parameters ({key}) passed to the request."  # noqa
+                )
+        return self.get_response(request)

readthedocs/rtd_tests/tests/test_middleware.py

@@ -2,13 +2,17 @@
 
 from corsheaders.middleware import CorsMiddleware
 from django.conf import settings
+from django.core.exceptions import SuspiciousOperation
 from django.http import HttpResponse
 from django.test import TestCase, override_settings
 from django.test.client import RequestFactory
 from django_dynamic_fixture import get
 
 from readthedocs.builds.constants import LATEST
-from readthedocs.core.middleware import ReadTheDocsSessionMiddleware
+from readthedocs.core.middleware import (
+    NullCharactersMiddleware,
+    ReadTheDocsSessionMiddleware,
+)
 from readthedocs.projects.constants import PRIVATE, PUBLIC
 from readthedocs.projects.models import Domain, Project, ProjectRelationship
 from readthedocs.rtd_tests.utils import create_user
@@ -259,3 +263,13 @@ def test_main_cookie_samesite_lax(self):
 
         self.assertEqual(response.cookies[settings.SESSION_COOKIE_NAME]['samesite'], 'Lax')
         self.assertTrue(self.test_main_cookie_samesite_none not in response.cookies)
+
+
+class TestNullCharactersMiddleware(TestCase):
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.middleware = NullCharactersMiddleware(None)
+
+    def test_request_with_null_chars(self):
+        request = self.factory.get("/?language=en\x00es&project_slug=myproject")
+        self.assertRaises(SuspiciousOperation, lambda: self.middleware(request))

readthedocs/settings/base.py

@@ -236,6 +236,7 @@ def USE_PROMOS(self):  # noqa
         return 'readthedocsext.donate' in self.INSTALLED_APPS
 
     MIDDLEWARE = (
+        'readthedocs.core.middleware.NullCharactersMiddleware',
         'readthedocs.core.middleware.ReadTheDocsSessionMiddleware',
         'django.middleware.locale.LocaleMiddleware',
         'django.middleware.common.CommonMiddleware',