From 6c9d5c55adee74885583810c0aecb0743e3b23aa Mon Sep 17 00:00:00 2001 From: charlesmamane26-sketch Date: Wed, 10 Jun 2026 21:10:18 +0200 Subject: [PATCH] Security: cap MCP HTTP request bodies and add optional Origin allowlist (#280) Reject request bodies above 16 MiB with 413 before allocating, stopping an unauthenticated Content-Length OOM (vec![0u8; len]). Add an optional, default-off OBSCURA_MCP_ALLOWED_ORIGINS allowlist; unset keeps the permissive default so issue #175 hosted dashboards are unaffected. --- crates/obscura-mcp/src/http.rs | 90 ++++++++++++++++++++++ crates/obscura-mcp/tests/cors_preflight.rs | 51 ++++++++++++ 2 files changed, 141 insertions(+) diff --git a/crates/obscura-mcp/src/http.rs b/crates/obscura-mcp/src/http.rs index ff6121b..5f1eef2 100644 --- a/crates/obscura-mcp/src/http.rs +++ b/crates/obscura-mcp/src/http.rs @@ -5,6 +5,42 @@ use tokio::net::TcpListener; use crate::{dispatch, BrowserState}; +/// Hard cap on a single MCP request body. The client-supplied `Content-Length` +/// is used to pre-size the read buffer; without a ceiling a request advertising +/// e.g. `Content-Length: 4294967296` makes the server allocate and zero-fill +/// that many bytes before reading any body — an unauthenticated OOM/DoS. 16 MiB +/// is far above any real JSON-RPC tool call. +const MAX_BODY_BYTES: usize = 16 * 1024 * 1024; + +/// Origin allowlist for browser callers, read from `OBSCURA_MCP_ALLOWED_ORIGINS` +/// (comma-separated). Unset/empty → permissive (unchanged `*`) so hosted +/// dashboards keep working (issue #175). +fn allowed_origins_env() -> Option { + std::env::var("OBSCURA_MCP_ALLOWED_ORIGINS") + .ok() + .filter(|s| !s.trim().is_empty()) +} + +/// Whether a request's `Origin` is permitted. A request with no `Origin` +/// (native, non-browser MCP clients) is always allowed — the same-origin +/// policy only constrains browser callers. When an allowlist is configured, a +/// browser `Origin` must match one of its entries (case-insensitive); this +/// stops a malicious local web page from driving the loopback MCP port. +fn origin_allowed(origin: Option<&str>, allowlist: Option<&str>) -> bool { + match allowlist { + None => true, + Some(list) => match origin { + None => true, + Some(o) => { + let o = o.trim(); + list.split(',') + .map(str::trim) + .any(|a| !a.is_empty() && a.eq_ignore_ascii_case(o)) + } + }, + } +} + /// MCP Streamable HTTP transport (POST /mcp → JSON response). /// /// Connections are handled sequentially on the current thread — the browser @@ -55,6 +91,7 @@ async fn handle_connection( let mut content_length: Option = None; let mut accept_sse = false; let mut keep_alive = false; + let mut origin: Option = None; loop { let mut line = String::new(); @@ -67,6 +104,11 @@ async fn handle_connection( if let Some(v) = lower.strip_prefix("content-length: ") { content_length = v.trim().parse().ok(); } + if lower.starts_with("origin:") { + if let Some(idx) = trimmed.find(':') { + origin = Some(trimmed[idx + 1..].trim().to_string()); + } + } if lower.contains("text/event-stream") { accept_sse = true; } @@ -81,6 +123,16 @@ async fn handle_connection( break; } + // Origin gate: when OBSCURA_MCP_ALLOWED_ORIGINS is configured, a browser + // request from a non-listed origin is refused before it can drive the + // browser session (mitigates a malicious local web page issuing + // cross-origin POSTs to the loopback MCP port). The permissive default + // and no-Origin native clients are unaffected. + if !origin_allowed(origin.as_deref(), allowed_origins_env().as_deref()) { + respond(&mut writer, 403, b"{\"error\":\"origin not allowed\"}").await?; + break; + } + match method { "OPTIONS" => { // mcp-protocol-version is part of the MCP spec, Authorization / @@ -124,6 +176,14 @@ async fn handle_connection( } }; + // Reject oversized bodies BEFORE allocating: `vec![0u8; len]` + // commits `len` bytes up front, so an attacker-chosen + // Content-Length would otherwise OOM the process (DoS). + if len > MAX_BODY_BYTES { + respond(&mut writer, 413, b"{\"error\":\"payload too large\"}").await?; + break; + } + let mut body = vec![0u8; len]; reader.read_exact(&mut body).await?; @@ -193,8 +253,10 @@ async fn respond_json(writer: &mut (impl AsyncWriteExt + Unpin), body: &[u8]) -> async fn respond(writer: &mut (impl AsyncWriteExt + Unpin), status: u16, body: &[u8]) -> Result<()> { let status_text = match status { 400 => "Bad Request", + 403 => "Forbidden", 404 => "Not Found", 405 => "Method Not Allowed", + 413 => "Payload Too Large", _ => "OK", }; let hdr = format!( @@ -209,3 +271,31 @@ async fn respond(writer: &mut (impl AsyncWriteExt + Unpin), status: u16, body: & writer.flush().await?; Ok(()) } + +#[cfg(test)] +mod mcp_hardening_tests { + use super::{origin_allowed, MAX_BODY_BYTES}; + + #[test] + fn no_allowlist_is_permissive() { + assert!(origin_allowed(Some("https://evil.example"), None)); + assert!(origin_allowed(None, None)); + } + + #[test] + fn allowlist_matches_case_insensitively_and_rejects_others() { + let list = Some("http://localhost:3000, https://app.example.com"); + assert!(origin_allowed(Some("http://localhost:3000"), list)); + assert!(origin_allowed(Some("https://APP.example.com"), list)); + assert!(!origin_allowed(Some("https://evil.example"), list)); + // A native client (no Origin header) is always allowed. + assert!(origin_allowed(None, list)); + } + + #[test] + fn body_cap_is_sane() { + // Far above a real JSON-RPC tool call, far below an OOM-inducing value. + assert!(MAX_BODY_BYTES >= 1 << 20); + assert!(MAX_BODY_BYTES <= 64 << 20); + } +} diff --git a/crates/obscura-mcp/tests/cors_preflight.rs b/crates/obscura-mcp/tests/cors_preflight.rs index b874439..80e95ef 100644 --- a/crates/obscura-mcp/tests/cors_preflight.rs +++ b/crates/obscura-mcp/tests/cors_preflight.rs @@ -84,3 +84,54 @@ async fn options_preflight_lists_required_browser_headers() { }) .await; } + +/// Regression test for the unbounded `Content-Length` allocation: a POST that +/// advertises a huge body must be rejected with 413 *before* the server tries +/// to allocate `vec![0u8; len]`, rather than committing gigabytes of RAM and +/// OOM-ing the process (unauthenticated DoS). +#[tokio::test(flavor = "current_thread")] +async fn oversized_content_length_is_rejected() { + let port = pick_free_port(); + let local = LocalSet::new(); + + let server = local.spawn_local(async move { + let _ = obscura_mcp::http::run("127.0.0.1".to_string(), port, None, None, false).await; + }); + + local + .run_until(async { + for _ in 0..40 { + if TcpStream::connect(("127.0.0.1", port)).await.is_ok() { + break; + } + sleep(Duration::from_millis(50)).await; + } + + let mut stream = TcpStream::connect(("127.0.0.1", port)) + .await + .expect("MCP server did not come up"); + // 8 GiB advertised, zero body sent. Pre-fix the server allocates 8 GiB. + let req = b"POST /mcp HTTP/1.1\r\n\ + Host: 127.0.0.1\r\n\ + Content-Type: application/json\r\n\ + Content-Length: 8589934592\r\n\ + \r\n"; + stream.write_all(req).await.unwrap(); + stream.flush().await.unwrap(); + + let mut buf = [0u8; 1024]; + let n = timeout(Duration::from_secs(2), stream.read(&mut buf)) + .await + .expect("read timed out") + .expect("read failed"); + let response = String::from_utf8_lossy(&buf[..n]).to_string(); + + server.abort(); + + assert!( + response.starts_with("HTTP/1.1 413"), + "expected 413 Payload Too Large, got:\n{response}" + ); + }) + .await; +}