From 5e1da4827a739b40e7ae6919a2b532ced556f6d3 Mon Sep 17 00:00:00 2001 From: AI-Reviewer-QS Date: Wed, 18 Feb 2026 00:33:42 +0800 Subject: [PATCH] fix: check Content-Length before downloading HTTP response body (#74) * fix: check Content-Length before downloading HTTP response body The HTTP tool previously downloaded the entire response body into memory before checking the size limit, allowing a malicious server to cause OOM. Now the Content-Length header is checked first to reject obviously oversized responses, and the body is streamed with a hard size cap so reading stops as soon as the limit is exceeded. * fix: check chunk size before allocation and fix Content-Length parsing Address review feedback: - Check body.len() + chunk.len() before extend_from_slice to prevent OOM from a single oversized chunk - Use let-chain for Content-Length parsing instead of unwrap_or to gracefully handle invalid headers * docs: document MAX_RESPONSE_SIZE rationale and add tracing on rejection Address review feedback: explain why 5 MB was chosen for the response size limit and log a warning when Content-Length causes early rejection. --------- Co-authored-by: Yi LIU --- src/tools/builtin/http.rs | 55 ++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/src/tools/builtin/http.rs b/src/tools/builtin/http.rs index 79e5caa6..1f37480c 100644 --- a/src/tools/builtin/http.rs +++ b/src/tools/builtin/http.rs @@ -5,13 +5,18 @@ use std::net::{IpAddr, ToSocketAddrs}; use std::time::Duration; use async_trait::async_trait; +use futures::StreamExt; use reqwest::Client; use crate::context::JobContext; use crate::safety::LeakDetector; use crate::tools::tool::{Tool, ToolError, ToolOutput, require_str}; -/// Maximum response body size (5 MB). Prevents OOM from unbounded responses. +/// Maximum response body size (5 MB). +/// +/// 5 MB is large enough for typical JSON API responses and moderate HTML pages, +/// but small enough to prevent OOM from malicious or runaway servers. The WASM +/// HTTP wrapper uses the same limit for consistency. const MAX_RESPONSE_SIZE: usize = 5 * 1024 * 1024; /// Tool for making HTTP requests. @@ -230,19 +235,43 @@ impl Tool for HttpTool { .filter_map(|(k, v)| v.to_str().ok().map(|v| (k.to_string(), v.to_string()))) .collect(); - // Get response body with size cap to prevent OOM - let body_bytes = response.bytes().await.map_err(|e| { - ToolError::ExternalService(format!("failed to read response body: {}", e)) - })?; - - if body_bytes.len() > MAX_RESPONSE_SIZE { + // Pre-check Content-Length header to reject obviously oversized responses + // before downloading anything, preventing OOM from malicious servers. + if let Some(content_length) = response.headers().get(reqwest::header::CONTENT_LENGTH) + && let Ok(s) = content_length.to_str() + && let Ok(len) = s.parse::() + && len > MAX_RESPONSE_SIZE + { + tracing::warn!( + url = %parsed_url, + content_length = len, + max = MAX_RESPONSE_SIZE, + "Rejected HTTP response: Content-Length exceeds limit" + ); return Err(ToolError::ExecutionFailed(format!( - "Response body too large ({} bytes, max {})", - body_bytes.len(), - MAX_RESPONSE_SIZE + "Response Content-Length ({} bytes) exceeds maximum allowed size ({} bytes)", + len, MAX_RESPONSE_SIZE ))); } + // Stream the response body with a hard size cap. Even if Content-Length was + // absent or lied about the size, we stop reading once we exceed the limit. + let mut body = Vec::new(); + let mut stream = response.bytes_stream(); + while let Some(chunk) = StreamExt::next(&mut stream).await { + let chunk = chunk.map_err(|e| { + ToolError::ExternalService(format!("failed to read response body: {}", e)) + })?; + if body.len() + chunk.len() > MAX_RESPONSE_SIZE { + return Err(ToolError::ExecutionFailed(format!( + "Response body exceeds maximum allowed size ({} bytes)", + MAX_RESPONSE_SIZE + ))); + } + body.extend_from_slice(&chunk); + } + let body_bytes = bytes::Bytes::from(body); + let body_text = String::from_utf8_lossy(&body_bytes).into_owned(); // Try to parse as JSON, fall back to string @@ -328,4 +357,10 @@ mod tests { // Public assert!(!is_disallowed_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)))); } + + #[test] + fn test_max_response_size_is_reasonable() { + // MAX_RESPONSE_SIZE should be 5 MB to prevent OOM while allowing typical API responses. + assert_eq!(MAX_RESPONSE_SIZE, 5 * 1024 * 1024); + } }