diff --git a/readthedocs/core/middleware.py b/readthedocs/core/middleware.py index 12f68973670..789d9a348b0 100644 --- a/readthedocs/core/middleware.py +++ b/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) diff --git a/readthedocs/rtd_tests/tests/test_middleware.py b/readthedocs/rtd_tests/tests/test_middleware.py index 3c143e1b900..a25f1201734 100644 --- a/readthedocs/rtd_tests/tests/test_middleware.py +++ b/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)) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index f1cad709f97..c911304f086 100644 --- a/readthedocs/settings/base.py +++ b/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',