From 8afe7d2a8e0c534852c89eb8edb15c5b074c50a2 Mon Sep 17 00:00:00 2001 From: EffortlessSteven <15812269+EffortlessSteven@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:46:15 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20Request=20Smuggling=20via=20chunked=20encoding=20bypas?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 4 ++++ crates/bitnet-server/src/lib.rs | 25 ++----------------------- crates/bitnet-server/src/security.rs | 10 ++++++++++ test_body_limit.rs | 4 ++++ test_chunked.rs | 1 + 5 files changed, 21 insertions(+), 23 deletions(-) create mode 100644 test_body_limit.rs create mode 100644 test_chunked.rs diff --git a/.jules/sentinel.md b/.jules/sentinel.md index b098b5f558..70109e22d3 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -23,3 +23,7 @@ **Vulnerability:** The `validate_model_request` function bypassed directory checks entirely when `allowed_model_directories` was empty. This allowed attackers to specify absolute paths (e.g., `/etc/passwd.gguf`) and bypass path restrictions. **Learning:** Checking for `..` and `~` is insufficient for path safety because absolute paths don't contain them. When an allowlist is intentionally empty, it is critical to explicitly deny absolute paths or deny all requests, rather than allowing any path. **Prevention:** Explicitly deny absolute paths if no specific allowed directories are configured. +## 2025-10-25 - Chunked Encoding Request Smuggling +**Vulnerability:** The Axum middleware relied solely on checking the `content-length` header to enforce request body size limits. This allowed an attacker to bypass the limit using chunked encoding (`Transfer-Encoding: chunked`), potentially leading to request smuggling and DoS via memory exhaustion. +**Learning:** Manually reading headers like `content-length` is insufficient for enforcing HTTP limits since standard HTTP clients can utilize chunked encoding which obscures the total size until fully consumed. +**Prevention:** Always enforce global body limits directly through the web framework's native capabilities (e.g., Axum's `DefaultBodyLimit`), which correctly handles all encoding types. When writing custom sanitization middleware, explicitly reject ambiguous or complex encodings if they aren't strictly required. diff --git a/crates/bitnet-server/src/lib.rs b/crates/bitnet-server/src/lib.rs index c282e5f104..7efa25908f 100644 --- a/crates/bitnet-server/src/lib.rs +++ b/crates/bitnet-server/src/lib.rs @@ -678,10 +678,8 @@ impl BitNetServer { // Add comprehensive middleware stack app = app .layer(middleware::from_fn(security_headers_middleware)) - .layer(middleware::from_fn_with_state( - self.security_validator.clone(), - request_validation_middleware, - )) + // 🛡️ Sentinel: Enforce global body limit to protect against chunked request smuggling + .layer(axum::extract::DefaultBodyLimit::max(self.config.security.max_prompt_length * 2)) .layer(middleware::from_fn_with_state( self.config.security.clone(), security::ip_blocking_middleware, @@ -1942,25 +1940,6 @@ async fn enhanced_metrics_middleware(request: Request, next: Next) -> Response { response } -/// Request validation middleware -async fn request_validation_middleware( - State(validator): State>, - request: Request, - next: Next, -) -> Result { - // Check request size limits - if let Some(content_length) = request.headers().get("content-length") - && let Ok(length_str) = content_length.to_str() - && let Ok(length) = length_str.parse::() - && length > validator.config().max_prompt_length * 2 - { - warn!(content_length = length, "Request payload too large"); - return Err(StatusCode::PAYLOAD_TOO_LARGE); - } - - Ok(next.run(request).await) -} - /// Utility functions /// Calculate tokens per second from token count and duration fn calculate_tokens_per_second(tokens: u64, duration: Duration) -> f64 { diff --git a/crates/bitnet-server/src/security.rs b/crates/bitnet-server/src/security.rs index a6f76f07df..9e39d6be34 100644 --- a/crates/bitnet-server/src/security.rs +++ b/crates/bitnet-server/src/security.rs @@ -493,6 +493,16 @@ pub async fn request_sanitization_middleware( // For inference requests, we'll validate in the handler // This middleware focuses on general request sanitization + // 🛡️ Sentinel: Reject chunked requests to prevent bypasses of our size checks, + // avoiding HTTP desync / request smuggling vulnerabilities. + if let Some(te) = request.headers().get("transfer-encoding") + && let Ok(te_str) = te.to_str() + && te_str.contains("chunked") + { + warn!("Chunked requests are not permitted"); + return Err(StatusCode::BAD_REQUEST); + } + // Check request size if let Some(content_length) = request.headers().get("content-length") && let Ok(length_str) = content_length.to_str() diff --git a/test_body_limit.rs b/test_body_limit.rs new file mode 100644 index 0000000000..aa5e91e338 --- /dev/null +++ b/test_body_limit.rs @@ -0,0 +1,4 @@ +use axum::{Router, extract::DefaultBodyLimit}; +fn test() { + let app: Router = Router::new().layer(DefaultBodyLimit::max(1024)); +} diff --git a/test_chunked.rs b/test_chunked.rs new file mode 100644 index 0000000000..193a380dde --- /dev/null +++ b/test_chunked.rs @@ -0,0 +1 @@ +fn test() {}