From 02525361dd3b1ad3887a7b2235bcb0a3a126bbb8 Mon Sep 17 00:00:00 2001 From: Radoslav Dimitrov Date: Sat, 3 Jan 2026 20:25:28 +0200 Subject: [PATCH] Handle URL-encoded NUL bytes (%00) in URL path and query Signed-off-by: Radoslav Dimitrov --- internal/api/server.go | 45 +++++++++++++++---- internal/api/server_test.go | 88 +++++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 8 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index 4b4f9bde..e795a1e2 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -2,6 +2,7 @@ package api import ( "context" + "encoding/json" "log" "net/http" "strings" @@ -17,19 +18,21 @@ import ( "github.com/modelcontextprotocol/registry/internal/telemetry" ) -// NulByteValidationMiddleware rejects requests containing NUL bytes in URL path or query parameters -// This prevents PostgreSQL encoding errors and returns a proper 400 Bad Request +// NulByteValidationMiddleware rejects requests containing NUL bytes in URL path or query parameters. +// This prevents PostgreSQL encoding errors (SQLSTATE 22021) and returns a proper 400 Bad Request. +// Checks for both literal NUL bytes (\x00) and URL-encoded form (%00). func NulByteValidationMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Check URL path for NUL bytes - if strings.ContainsRune(r.URL.Path, '\x00') { - http.Error(w, "Invalid request: URL path contains null bytes", http.StatusBadRequest) + // Check URL path for literal NUL bytes or URL-encoded %00 + // Path needs %00 check because handlers call url.PathUnescape() which would decode it + if containsNulByte(r.URL.Path) { + writeErrorResponse(w, http.StatusBadRequest, "Invalid request: URL path contains null bytes") return } - // Check raw query string for NUL bytes - if strings.ContainsRune(r.URL.RawQuery, '\x00') { - http.Error(w, "Invalid request: query parameters contain null bytes", http.StatusBadRequest) + // Check raw query string for literal NUL bytes or URL-encoded %00 + if containsNulByte(r.URL.RawQuery) { + writeErrorResponse(w, http.StatusBadRequest, "Invalid request: query parameters contain null bytes") return } @@ -37,6 +40,32 @@ func NulByteValidationMiddleware(next http.Handler) http.Handler { }) } +// writeErrorResponse writes a JSON error response using huma's ErrorModel format +// for consistency with the rest of the API. +func writeErrorResponse(w http.ResponseWriter, status int, detail string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + + errModel := &huma.ErrorModel{ + Title: http.StatusText(status), + Status: status, + Detail: detail, + } + _ = json.NewEncoder(w).Encode(errModel) +} + +// containsNulByte checks if a string contains a NUL byte, either as a literal \x00 +// or URL-encoded as %00. +func containsNulByte(s string) bool { + // Check for literal NUL byte + if strings.ContainsRune(s, '\x00') { + return true + } + // Check for URL-encoded NUL byte (%00) + // Using Contains directly since %00 has no case variation (both hex digits are 0) + return strings.Contains(s, "%00") +} + // TrailingSlashMiddleware redirects requests with trailing slashes to their canonical form func TrailingSlashMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/api/server_test.go b/internal/api/server_test.go index c7d189ea..dc313b03 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -52,6 +52,10 @@ func TestNulByteValidationMiddleware(t *testing.T) { if !strings.Contains(w.Body.String(), "URL path contains null bytes") { t.Errorf("expected body to contain error message, got %q", w.Body.String()) } + // Verify JSON response format + if w.Header().Get("Content-Type") != "application/json" { + t.Errorf("expected Content-Type application/json, got %q", w.Header().Get("Content-Type")) + } }) t.Run("query with NUL byte should return 400", func(t *testing.T) { @@ -79,6 +83,90 @@ func TestNulByteValidationMiddleware(t *testing.T) { t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code) } }) + + t.Run("query with URL-encoded NUL byte (%00) should return 400", func(t *testing.T) { + // This is the exact case from issue #862: ?cursor=%00 + req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%00", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code) + } + if !strings.Contains(w.Body.String(), "query parameters contain null bytes") { + t.Errorf("expected body to contain error message, got %q", w.Body.String()) + } + }) + + t.Run("query with URL-encoded NUL byte followed by text should return 400", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%00test", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code) + } + }) + + t.Run("query with embedded URL-encoded NUL byte should return 400", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=abc%00def", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code) + } + }) + + t.Run("query with double-encoded NUL byte (%2500) should pass through", func(t *testing.T) { + // %2500 decodes to %00 (literal string), not a NUL byte + // This is intentionally allowed - double-decoding is the caller's responsibility + req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%2500", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + // This should pass - %2500 is not a NUL byte injection attempt + // When decoded once: %2500 -> %00 (the string "%00", not a NUL byte) + if w.Code != http.StatusOK { + t.Errorf("expected status %d, got %d (double-encoded should pass)", http.StatusOK, w.Code) + } + }) + + t.Run("query with valid percent-encoding should pass through", func(t *testing.T) { + // Ensure we don't false-positive on valid encodings like %20 (space) + req := httptest.NewRequest(http.MethodGet, "/v0/servers?search=hello%20world", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected status %d, got %d", http.StatusOK, w.Code) + } + }) + + t.Run("path with URL-encoded NUL byte (%00) should return 400", func(t *testing.T) { + // Handlers call url.PathUnescape() which would decode %00 to \x00 + req := httptest.NewRequest(http.MethodGet, "/v0/servers/%00/versions", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code) + } + if !strings.Contains(w.Body.String(), "URL path contains null bytes") { + t.Errorf("expected body to contain error message, got %q", w.Body.String()) + } + }) + + t.Run("path with URL-encoded NUL byte among other encodings should return 400", func(t *testing.T) { + // %0a is newline, %00 is NUL - should still catch the NUL + req := httptest.NewRequest(http.MethodGet, "/v0/servers/test%0a%00name/versions", nil) + w := httptest.NewRecorder() + middleware.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code) + } + }) } func TestTrailingSlashMiddleware(t *testing.T) {