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.
This commit is contained in:
1 parent
dd7395a80a
commit
6c9d5c55ad
2 files changed
+141
No files matched your search
@@ -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<String> {
|
||||
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<usize> = None;
|
||||
let mut accept_sse = false;
|
||||
let mut keep_alive = false;
|
||||
let mut origin: Option<String> = 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);
|
||||
}
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
Reference in new issue
Block a user