From 1a88f24c4d7d24cc05440b5efefbab3fa7dc4f67 Mon Sep 17 00:00:00 2001 From: Deep Santoshwar Date: Sat, 1 Aug 2026 23:44:58 +0530 Subject: [PATCH 1/2] fix(API): reject NUL bytes in query parameters instead of crashing with 500 Any view that passes a query param straight into a Postgres string query (e.g. environments/identities/views.py's identifier lookup) raised an unhandled ValueError when the value contained a NUL byte, since psycopg rejects NUL characters in string literals. Reject such requests centrally in middleware instead of patching every call site individually. --- api/app/settings/common.py | 1 + api/core/middleware/query_params.py | 24 +++++++++++ .../test_unit_core_middleware_query_params.py | 43 +++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 api/core/middleware/query_params.py create mode 100644 api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py diff --git a/api/app/settings/common.py b/api/app/settings/common.py index d948e14b5a9d..c85ee8d44173 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -379,6 +379,7 @@ MIDDLEWARE = [ "common.core.middleware.APIResponseVersionHeaderMiddleware", "common.gunicorn.middleware.RouteLoggerMiddleware", + "core.middleware.query_params.RejectNulByteQueryParamsMiddleware", "django.middleware.security.SecurityMiddleware", "whitenoise.middleware.WhiteNoiseMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", diff --git a/api/core/middleware/query_params.py b/api/core/middleware/query_params.py new file mode 100644 index 000000000000..063fc730b0ed --- /dev/null +++ b/api/core/middleware/query_params.py @@ -0,0 +1,24 @@ +from collections.abc import Callable + +from django.http import HttpRequest, HttpResponse, HttpResponseBadRequest + + +class RejectNulByteQueryParamsMiddleware: + """ + Reject requests whose query parameters contain a NUL (0x00) character. + + Passing one through to a query against the string field of a Postgres + row raises an unhandled `ValueError: A string literal cannot contain + NUL (0x00) characters`, so reject it here, before any view can pass it + to the ORM. + """ + + def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]) -> None: + self.get_response = get_response + + def __call__(self, request: HttpRequest) -> HttpResponse: + if any("\x00" in value for value in request.GET.values()): + return HttpResponseBadRequest( + "Query parameters must not contain NUL characters." + ) + return self.get_response(request) diff --git a/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py b/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py new file mode 100644 index 000000000000..bf50140cee53 --- /dev/null +++ b/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py @@ -0,0 +1,43 @@ +from django.http import HttpResponse +from django.test import RequestFactory + +from core.middleware.query_params import RejectNulByteQueryParamsMiddleware + + +def test_reject_nul_byte_query_params_middleware__nul_byte_in_query_param__returns_bad_request( # type: ignore[no-untyped-def] # noqa: E501 + mocker, rf: RequestFactory +): + # Given + mocked_get_response = mocker.MagicMock() + request = rf.get( + "/api/v1/environments/some-key/identities/", {"identifier": "foo\x00bar"} + ) + + middleware = RejectNulByteQueryParamsMiddleware(mocked_get_response) + + # When + response = middleware(request) + + # Then + assert response.status_code == 400 + mocked_get_response.assert_not_called() + + +def test_reject_nul_byte_query_params_middleware__no_nul_byte__calls_get_response( # type: ignore[no-untyped-def] # noqa: E501 + mocker, rf: RequestFactory +): + # Given + a_response = HttpResponse() + mocked_get_response = mocker.MagicMock(return_value=a_response) + request = rf.get( + "/api/v1/environments/some-key/identities/", {"identifier": "foobar"} + ) + + middleware = RejectNulByteQueryParamsMiddleware(mocked_get_response) + + # When + response = middleware(request) + + # Then + assert response is a_response + mocked_get_response.assert_called_once_with(request) From 69cde38ad9ce0e331bdf1285f93c9626d5dddd0a Mon Sep 17 00:00:00 2001 From: Deep Santoshwar Date: Sun, 2 Aug 2026 00:09:39 +0530 Subject: [PATCH 2/2] fix(API): address CodeRabbit review on NUL-byte query param middleware - Run the middleware after CorsMiddleware so a short-circuited 400 response still gets CORS headers, instead of the request bypassing CorsMiddleware entirely. - Use QueryDict.lists() instead of .values(), which only yields the last value per key and let a NUL byte in an earlier value of a repeated query key slip through undetected. --- api/app/settings/common.py | 4 +++- api/core/middleware/query_params.py | 7 ++++++- .../test_unit_core_middleware_query_params.py | 20 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/api/app/settings/common.py b/api/app/settings/common.py index c85ee8d44173..4f3690288b85 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -379,11 +379,13 @@ MIDDLEWARE = [ "common.core.middleware.APIResponseVersionHeaderMiddleware", "common.gunicorn.middleware.RouteLoggerMiddleware", - "core.middleware.query_params.RejectNulByteQueryParamsMiddleware", "django.middleware.security.SecurityMiddleware", "whitenoise.middleware.WhiteNoiseMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "corsheaders.middleware.CorsMiddleware", + # Must come after CorsMiddleware: it can short-circuit with a response of + # its own, and CorsMiddleware needs to wrap it to add CORS headers to that. + "core.middleware.query_params.RejectNulByteQueryParamsMiddleware", "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", diff --git a/api/core/middleware/query_params.py b/api/core/middleware/query_params.py index 063fc730b0ed..8eafa65f679f 100644 --- a/api/core/middleware/query_params.py +++ b/api/core/middleware/query_params.py @@ -17,7 +17,12 @@ def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]) -> None: self.get_response = get_response def __call__(self, request: HttpRequest) -> HttpResponse: - if any("\x00" in value for value in request.GET.values()): + # `.values()` only yields the last value per key: a repeated key + # (`?a=x&a=y`) would let a NUL byte in an earlier value slip through. + # `.lists()` yields every value for every key. + if any( + "\x00" in value for _, values in request.GET.lists() for value in values + ): return HttpResponseBadRequest( "Query parameters must not contain NUL characters." ) diff --git a/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py b/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py index bf50140cee53..a663d21289d1 100644 --- a/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py +++ b/api/tests/unit/core/middleware/test_unit_core_middleware_query_params.py @@ -41,3 +41,23 @@ def test_reject_nul_byte_query_params_middleware__no_nul_byte__calls_get_respons # Then assert response is a_response mocked_get_response.assert_called_once_with(request) + + +def test_reject_nul_byte_query_params_middleware__nul_byte_in_repeated_key__returns_bad_request( # type: ignore[no-untyped-def] # noqa: E501 + mocker, rf: RequestFactory +): + # Given - `identifier` is repeated; `QueryDict.values()` would only see + # the last ("foobar"), silently missing the NUL byte in the first. + mocked_get_response = mocker.MagicMock() + request = rf.get( + "/api/v1/environments/some-key/identities/?identifier=foo%00bar&identifier=foobar" + ) + + middleware = RejectNulByteQueryParamsMiddleware(mocked_get_response) + + # When + response = middleware(request) + + # Then + assert response.status_code == 400 + mocked_get_response.assert_not_called()