Skip to content

Commit ad207f2

Browse files
authored
Handle URL-encoded NUL bytes (%00) in URL path and query (#877)
<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> Follow up on #869 ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> ## Breaking Changes <!-- Will users need to update their code or configurations? --> ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> Signed-off-by: Radoslav Dimitrov <[email protected]>
1 parent c6856a8 commit ad207f2

File tree

2 files changed

+125
-8
lines changed

2 files changed

+125
-8
lines changed

internal/api/server.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package api
22

33
import (
44
"context"
5+
"encoding/json"
56
"log"
67
"net/http"
78
"strings"
@@ -17,26 +18,54 @@ import (
1718
"github.com/modelcontextprotocol/registry/internal/telemetry"
1819
)
1920

20-
// NulByteValidationMiddleware rejects requests containing NUL bytes in URL path or query parameters
21-
// This prevents PostgreSQL encoding errors and returns a proper 400 Bad Request
21+
// NulByteValidationMiddleware rejects requests containing NUL bytes in URL path or query parameters.
22+
// This prevents PostgreSQL encoding errors (SQLSTATE 22021) and returns a proper 400 Bad Request.
23+
// Checks for both literal NUL bytes (\x00) and URL-encoded form (%00).
2224
func NulByteValidationMiddleware(next http.Handler) http.Handler {
2325
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
24-
// Check URL path for NUL bytes
25-
if strings.ContainsRune(r.URL.Path, '\x00') {
26-
http.Error(w, "Invalid request: URL path contains null bytes", http.StatusBadRequest)
26+
// Check URL path for literal NUL bytes or URL-encoded %00
27+
// Path needs %00 check because handlers call url.PathUnescape() which would decode it
28+
if containsNulByte(r.URL.Path) {
29+
writeErrorResponse(w, http.StatusBadRequest, "Invalid request: URL path contains null bytes")
2730
return
2831
}
2932

30-
// Check raw query string for NUL bytes
31-
if strings.ContainsRune(r.URL.RawQuery, '\x00') {
32-
http.Error(w, "Invalid request: query parameters contain null bytes", http.StatusBadRequest)
33+
// Check raw query string for literal NUL bytes or URL-encoded %00
34+
if containsNulByte(r.URL.RawQuery) {
35+
writeErrorResponse(w, http.StatusBadRequest, "Invalid request: query parameters contain null bytes")
3336
return
3437
}
3538

3639
next.ServeHTTP(w, r)
3740
})
3841
}
3942

43+
// writeErrorResponse writes a JSON error response using huma's ErrorModel format
44+
// for consistency with the rest of the API.
45+
func writeErrorResponse(w http.ResponseWriter, status int, detail string) {
46+
w.Header().Set("Content-Type", "application/json")
47+
w.WriteHeader(status)
48+
49+
errModel := &huma.ErrorModel{
50+
Title: http.StatusText(status),
51+
Status: status,
52+
Detail: detail,
53+
}
54+
_ = json.NewEncoder(w).Encode(errModel)
55+
}
56+
57+
// containsNulByte checks if a string contains a NUL byte, either as a literal \x00
58+
// or URL-encoded as %00.
59+
func containsNulByte(s string) bool {
60+
// Check for literal NUL byte
61+
if strings.ContainsRune(s, '\x00') {
62+
return true
63+
}
64+
// Check for URL-encoded NUL byte (%00)
65+
// Using Contains directly since %00 has no case variation (both hex digits are 0)
66+
return strings.Contains(s, "%00")
67+
}
68+
4069
// TrailingSlashMiddleware redirects requests with trailing slashes to their canonical form
4170
func TrailingSlashMiddleware(next http.Handler) http.Handler {
4271
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

internal/api/server_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ func TestNulByteValidationMiddleware(t *testing.T) {
5252
if !strings.Contains(w.Body.String(), "URL path contains null bytes") {
5353
t.Errorf("expected body to contain error message, got %q", w.Body.String())
5454
}
55+
// Verify JSON response format
56+
if w.Header().Get("Content-Type") != "application/json" {
57+
t.Errorf("expected Content-Type application/json, got %q", w.Header().Get("Content-Type"))
58+
}
5559
})
5660

5761
t.Run("query with NUL byte should return 400", func(t *testing.T) {
@@ -79,6 +83,90 @@ func TestNulByteValidationMiddleware(t *testing.T) {
7983
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
8084
}
8185
})
86+
87+
t.Run("query with URL-encoded NUL byte (%00) should return 400", func(t *testing.T) {
88+
// This is the exact case from issue #862: ?cursor=%00
89+
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%00", nil)
90+
w := httptest.NewRecorder()
91+
middleware.ServeHTTP(w, req)
92+
93+
if w.Code != http.StatusBadRequest {
94+
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
95+
}
96+
if !strings.Contains(w.Body.String(), "query parameters contain null bytes") {
97+
t.Errorf("expected body to contain error message, got %q", w.Body.String())
98+
}
99+
})
100+
101+
t.Run("query with URL-encoded NUL byte followed by text should return 400", func(t *testing.T) {
102+
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%00test", nil)
103+
w := httptest.NewRecorder()
104+
middleware.ServeHTTP(w, req)
105+
106+
if w.Code != http.StatusBadRequest {
107+
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
108+
}
109+
})
110+
111+
t.Run("query with embedded URL-encoded NUL byte should return 400", func(t *testing.T) {
112+
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=abc%00def", nil)
113+
w := httptest.NewRecorder()
114+
middleware.ServeHTTP(w, req)
115+
116+
if w.Code != http.StatusBadRequest {
117+
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
118+
}
119+
})
120+
121+
t.Run("query with double-encoded NUL byte (%2500) should pass through", func(t *testing.T) {
122+
// %2500 decodes to %00 (literal string), not a NUL byte
123+
// This is intentionally allowed - double-decoding is the caller's responsibility
124+
req := httptest.NewRequest(http.MethodGet, "/v0/servers?cursor=%2500", nil)
125+
w := httptest.NewRecorder()
126+
middleware.ServeHTTP(w, req)
127+
128+
// This should pass - %2500 is not a NUL byte injection attempt
129+
// When decoded once: %2500 -> %00 (the string "%00", not a NUL byte)
130+
if w.Code != http.StatusOK {
131+
t.Errorf("expected status %d, got %d (double-encoded should pass)", http.StatusOK, w.Code)
132+
}
133+
})
134+
135+
t.Run("query with valid percent-encoding should pass through", func(t *testing.T) {
136+
// Ensure we don't false-positive on valid encodings like %20 (space)
137+
req := httptest.NewRequest(http.MethodGet, "/v0/servers?search=hello%20world", nil)
138+
w := httptest.NewRecorder()
139+
middleware.ServeHTTP(w, req)
140+
141+
if w.Code != http.StatusOK {
142+
t.Errorf("expected status %d, got %d", http.StatusOK, w.Code)
143+
}
144+
})
145+
146+
t.Run("path with URL-encoded NUL byte (%00) should return 400", func(t *testing.T) {
147+
// Handlers call url.PathUnescape() which would decode %00 to \x00
148+
req := httptest.NewRequest(http.MethodGet, "/v0/servers/%00/versions", nil)
149+
w := httptest.NewRecorder()
150+
middleware.ServeHTTP(w, req)
151+
152+
if w.Code != http.StatusBadRequest {
153+
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
154+
}
155+
if !strings.Contains(w.Body.String(), "URL path contains null bytes") {
156+
t.Errorf("expected body to contain error message, got %q", w.Body.String())
157+
}
158+
})
159+
160+
t.Run("path with URL-encoded NUL byte among other encodings should return 400", func(t *testing.T) {
161+
// %0a is newline, %00 is NUL - should still catch the NUL
162+
req := httptest.NewRequest(http.MethodGet, "/v0/servers/test%0a%00name/versions", nil)
163+
w := httptest.NewRecorder()
164+
middleware.ServeHTTP(w, req)
165+
166+
if w.Code != http.StatusBadRequest {
167+
t.Errorf("expected status %d, got %d", http.StatusBadRequest, w.Code)
168+
}
169+
})
82170
}
83171

84172
func TestTrailingSlashMiddleware(t *testing.T) {

0 commit comments

Comments
 (0)