From 14c07511843fed7117ace177b0b5b18081391112 Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Sun, 29 Mar 2026 14:58:19 -0500 Subject: [PATCH 1/3] fix(l7): reject duplicate Content-Length headers to prevent request smuggling Both parse_body_length() in rest.rs and try_parse_http_request() in inference.rs silently accepted multiple Content-Length headers, overwriting with the last value seen. Per RFC 7230 Section 3.3.3, a message with multiple Content-Length headers with differing values must be rejected to prevent HTTP request smuggling (CWE-444). An attacker could send conflicting Content-Length values causing the proxy and downstream server to disagree on message boundaries. Fix: - rest.rs: detect duplicate CL headers with differing values and return an error before forwarding - inference.rs: add ParseResult::Invalid variant; detect duplicate CL headers and return Invalid with a descriptive reason - proxy.rs: handle ParseResult::Invalid by sending HTTP 400 and denying the connection Closes #637 Signed-off-by: latenighthackathon --- crates/openshell-sandbox/src/l7/inference.rs | 12 +++++++++++- crates/openshell-sandbox/src/l7/rest.rs | 7 +++++++ crates/openshell-sandbox/src/proxy.rs | 5 +++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/openshell-sandbox/src/l7/inference.rs b/crates/openshell-sandbox/src/l7/inference.rs index 59dafdab..6a3a3c81 100644 --- a/crates/openshell-sandbox/src/l7/inference.rs +++ b/crates/openshell-sandbox/src/l7/inference.rs @@ -96,6 +96,8 @@ pub enum ParseResult { Complete(ParsedHttpRequest, usize), /// Headers are incomplete — caller should read more data. Incomplete, + /// The request is malformed and must be rejected (e.g., duplicate Content-Length). + Invalid(String), } /// Try to parse an HTTP/1.1 request from raw bytes. @@ -125,6 +127,7 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult { let mut headers = Vec::new(); let mut content_length: usize = 0; + let mut has_content_length = false; let mut is_chunked = false; for line in lines { if line.is_empty() { @@ -134,7 +137,14 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult { let name = name.trim().to_string(); let value = value.trim().to_string(); if name.eq_ignore_ascii_case("content-length") { - content_length = value.parse().unwrap_or(0); + let new_len: usize = value.parse().unwrap_or(0); + if has_content_length && new_len != content_length { + return ParseResult::Invalid(format!( + "duplicate Content-Length headers with differing values ({content_length} vs {new_len})" + )); + } + content_length = new_len; + has_content_length = true; } if name.eq_ignore_ascii_case("transfer-encoding") && value diff --git a/crates/openshell-sandbox/src/l7/rest.rs b/crates/openshell-sandbox/src/l7/rest.rs index ebb34957..9c0aa153 100644 --- a/crates/openshell-sandbox/src/l7/rest.rs +++ b/crates/openshell-sandbox/src/l7/rest.rs @@ -250,6 +250,13 @@ fn parse_body_length(headers: &str) -> Result { && let Some(val) = lower.split_once(':').map(|(_, v)| v.trim()) && let Ok(len) = val.parse::() { + if let Some(prev) = cl_value { + if prev != len { + return Err(miette!( + "Request contains multiple Content-Length headers with differing values ({prev} vs {len})" + )); + } + } cl_value = Some(len); } } diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index 1f38a2cc..e90f63b4 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -943,6 +943,11 @@ async fn handle_inference_interception( buf.resize((buf.len() * 2).min(MAX_INFERENCE_BUF), 0); } } + ParseResult::Invalid(reason) => { + let response = format_http_response(400, &[], reason.as_bytes()); + write_all(&mut tls_client, &response).await?; + return Ok(InferenceOutcome::Denied { reason }); + } } } From c50b330a459931f43b2c1ff81e5dfd1df7f39d07 Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Sun, 29 Mar 2026 17:54:54 -0500 Subject: [PATCH 2/3] fix(l7): address review feedback on Content-Length smuggling defense MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - inference.rs: reject unparseable Content-Length values instead of silently defaulting to 0 via unwrap_or(0) - rest.rs: reject unparseable Content-Length values so a valid+invalid duplicate pair cannot bypass the differing-values check - rest.rs: fix Transfer-Encoding substring match (.contains("chunked") → split/trim exact match) to align with inference.rs and prevent false positives on values like "chunkedx" - proxy.rs: log parsing details server-side via tracing::warn and return generic "Bad Request" body instead of leaking internal parsing reasons to sandboxed code - Add tests for all new rejection paths in inference.rs and rest.rs Signed-off-by: latenighthackathon --- crates/openshell-sandbox/src/l7/inference.rs | 45 ++++++++++++++- crates/openshell-sandbox/src/l7/rest.rs | 61 ++++++++++++++++++-- crates/openshell-sandbox/src/proxy.rs | 3 +- 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/crates/openshell-sandbox/src/l7/inference.rs b/crates/openshell-sandbox/src/l7/inference.rs index 6a3a3c81..430aec1d 100644 --- a/crates/openshell-sandbox/src/l7/inference.rs +++ b/crates/openshell-sandbox/src/l7/inference.rs @@ -137,7 +137,12 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult { let name = name.trim().to_string(); let value = value.trim().to_string(); if name.eq_ignore_ascii_case("content-length") { - let new_len: usize = value.parse().unwrap_or(0); + let new_len: usize = match value.parse() { + Ok(v) => v, + Err(_) => return ParseResult::Invalid( + format!("invalid Content-Length value: {value}") + ), + }; if has_content_length && new_len != content_length { return ParseResult::Invalid(format!( "duplicate Content-Length headers with differing values ({content_length} vs {new_len})" @@ -562,4 +567,42 @@ mod tests { }; assert_eq!(parsed.body.len(), 100); } + + // ---- SEC: Content-Length validation ---- + + #[test] + fn reject_differing_duplicate_content_length() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: 0\r\nContent-Length: 50\r\n\r\n"; + assert!(matches!( + try_parse_http_request(request), + ParseResult::Invalid(reason) if reason.contains("differing values") + )); + } + + #[test] + fn accept_identical_duplicate_content_length() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: 5\r\nContent-Length: 5\r\n\r\nhello"; + let ParseResult::Complete(parsed, _) = try_parse_http_request(request) else { + panic!("expected Complete for identical duplicate CL"); + }; + assert_eq!(parsed.body, b"hello"); + } + + #[test] + fn reject_non_numeric_content_length() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\n\r\n"; + assert!(matches!( + try_parse_http_request(request), + ParseResult::Invalid(reason) if reason.contains("invalid Content-Length") + )); + } + + #[test] + fn reject_two_non_numeric_content_lengths() { + let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\nContent-Length: def\r\n\r\n"; + assert!(matches!( + try_parse_http_request(request), + ParseResult::Invalid(_) + )); + } } diff --git a/crates/openshell-sandbox/src/l7/rest.rs b/crates/openshell-sandbox/src/l7/rest.rs index 9c0aa153..74e8ea0d 100644 --- a/crates/openshell-sandbox/src/l7/rest.rs +++ b/crates/openshell-sandbox/src/l7/rest.rs @@ -242,14 +242,15 @@ fn parse_body_length(headers: &str) -> Result { let lower = line.to_ascii_lowercase(); if lower.starts_with("transfer-encoding:") { let val = lower.split_once(':').map_or("", |(_, v)| v.trim()); - if val.contains("chunked") { + if val.split(',').any(|enc| enc.trim() == "chunked") { has_te_chunked = true; } } - if lower.starts_with("content-length:") - && let Some(val) = lower.split_once(':').map(|(_, v)| v.trim()) - && let Ok(len) = val.parse::() - { + if lower.starts_with("content-length:") { + let val = lower.split_once(':').map_or("", |(_, v)| v.trim()); + let len: u64 = val.parse().map_err(|_| { + miette!("Request contains invalid Content-Length value") + })?; if let Some(prev) = cl_value { if prev != len { return Err(miette!( @@ -709,6 +710,56 @@ mod tests { ); } + /// SEC: Reject differing duplicate Content-Length headers. + #[test] + fn reject_differing_duplicate_content_length() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 0\r\nContent-Length: 50\r\n\r\n"; + assert!( + parse_body_length(headers).is_err(), + "Must reject differing duplicate Content-Length" + ); + } + + /// SEC: Accept identical duplicate Content-Length headers. + #[test] + fn accept_identical_duplicate_content_length() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: 42\r\n\r\n"; + match parse_body_length(headers).unwrap() { + BodyLength::ContentLength(42) => {} + other => panic!("Expected ContentLength(42), got {other:?}"), + } + } + + /// SEC: Reject non-numeric Content-Length values. + #[test] + fn reject_non_numeric_content_length() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\n\r\n"; + assert!( + parse_body_length(headers).is_err(), + "Must reject non-numeric Content-Length" + ); + } + + /// SEC: Reject when second Content-Length is non-numeric (bypass test). + #[test] + fn reject_valid_then_invalid_content_length() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: abc\r\n\r\n"; + assert!( + parse_body_length(headers).is_err(), + "Must reject when any Content-Length is non-numeric" + ); + } + + /// SEC: Transfer-Encoding substring match must not match partial tokens. + #[test] + fn te_substring_not_chunked() { + let headers = "POST /api HTTP/1.1\r\nHost: x\r\nTransfer-Encoding: chunkedx\r\n\r\n"; + match parse_body_length(headers).unwrap() { + BodyLength::None => {} + other => panic!("Expected None for non-matching TE, got {other:?}"), + } + } + /// SEC-009: Bare LF in headers enables header injection. #[tokio::test] async fn reject_bare_lf_in_headers() { diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index e90f63b4..6f3eaf9b 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -944,7 +944,8 @@ async fn handle_inference_interception( } } ParseResult::Invalid(reason) => { - let response = format_http_response(400, &[], reason.as_bytes()); + warn!(reason = %reason, "rejecting malformed inference request"); + let response = format_http_response(400, &[], b"Bad Request"); write_all(&mut tls_client, &response).await?; return Ok(InferenceOutcome::Denied { reason }); } From 2bf573208be240fd3fababf3feef20460861a670 Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Sun, 29 Mar 2026 18:04:12 -0500 Subject: [PATCH 3/3] style(l7): apply cargo fmt formatting Signed-off-by: latenighthackathon --- crates/openshell-sandbox/src/l7/inference.rs | 11 +++++++---- crates/openshell-sandbox/src/l7/rest.rs | 15 +++++++++------ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/openshell-sandbox/src/l7/inference.rs b/crates/openshell-sandbox/src/l7/inference.rs index 430aec1d..140213f0 100644 --- a/crates/openshell-sandbox/src/l7/inference.rs +++ b/crates/openshell-sandbox/src/l7/inference.rs @@ -139,9 +139,11 @@ pub fn try_parse_http_request(buf: &[u8]) -> ParseResult { if name.eq_ignore_ascii_case("content-length") { let new_len: usize = match value.parse() { Ok(v) => v, - Err(_) => return ParseResult::Invalid( - format!("invalid Content-Length value: {value}") - ), + Err(_) => { + return ParseResult::Invalid(format!( + "invalid Content-Length value: {value}" + )); + } }; if has_content_length && new_len != content_length { return ParseResult::Invalid(format!( @@ -590,7 +592,8 @@ mod tests { #[test] fn reject_non_numeric_content_length() { - let request = b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\n\r\n"; + let request = + b"POST /v1/chat/completions HTTP/1.1\r\nHost: x\r\nContent-Length: abc\r\n\r\n"; assert!(matches!( try_parse_http_request(request), ParseResult::Invalid(reason) if reason.contains("invalid Content-Length") diff --git a/crates/openshell-sandbox/src/l7/rest.rs b/crates/openshell-sandbox/src/l7/rest.rs index 74e8ea0d..f47f01bd 100644 --- a/crates/openshell-sandbox/src/l7/rest.rs +++ b/crates/openshell-sandbox/src/l7/rest.rs @@ -248,9 +248,9 @@ fn parse_body_length(headers: &str) -> Result { } if lower.starts_with("content-length:") { let val = lower.split_once(':').map_or("", |(_, v)| v.trim()); - let len: u64 = val.parse().map_err(|_| { - miette!("Request contains invalid Content-Length value") - })?; + let len: u64 = val + .parse() + .map_err(|_| miette!("Request contains invalid Content-Length value"))?; if let Some(prev) = cl_value { if prev != len { return Err(miette!( @@ -713,7 +713,8 @@ mod tests { /// SEC: Reject differing duplicate Content-Length headers. #[test] fn reject_differing_duplicate_content_length() { - let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 0\r\nContent-Length: 50\r\n\r\n"; + let headers = + "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 0\r\nContent-Length: 50\r\n\r\n"; assert!( parse_body_length(headers).is_err(), "Must reject differing duplicate Content-Length" @@ -723,7 +724,8 @@ mod tests { /// SEC: Accept identical duplicate Content-Length headers. #[test] fn accept_identical_duplicate_content_length() { - let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: 42\r\n\r\n"; + let headers = + "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: 42\r\n\r\n"; match parse_body_length(headers).unwrap() { BodyLength::ContentLength(42) => {} other => panic!("Expected ContentLength(42), got {other:?}"), @@ -743,7 +745,8 @@ mod tests { /// SEC: Reject when second Content-Length is non-numeric (bypass test). #[test] fn reject_valid_then_invalid_content_length() { - let headers = "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: abc\r\n\r\n"; + let headers = + "POST /api HTTP/1.1\r\nHost: x\r\nContent-Length: 42\r\nContent-Length: abc\r\n\r\n"; assert!( parse_body_length(headers).is_err(), "Must reject when any Content-Length is non-numeric"