diff --git a/CHANGELOG.md b/CHANGELOG.md index 21c78c4..40a784c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## [Unreleased] +### Fixed +- **Stack overflow on deeply nested JSON payloads** (`src/middleware/payload_filter.rs`, `src/gateway.rs`): `scan_value` (payload filter) was fully recursive and would overflow the stack on deeply nested JSON arguments. Replaced with an explicit-stack iterative traversal capped at `MAX_DEPTH = 64`. `redact_value` (gateway) was also recursive; refactored to depth-parameterised recursion that returns the value unchanged beyond `REDACT_MAX_DEPTH = 64` instead of panicking. Added two new unit tests covering the boundary. Closes #95. + ### Changed - **Rate limiter replaced with `governor` (GCRA, lock-free)** (`src/middleware/rate_limit.rs`): replaced the custom `Mutex>>` sliding-window implementation with the `governor` crate (GCRA algorithm). Enforcement is now lock-free (atomics) and O(1) per check — no more O(n) `Vec::retain` on every request, no more background cleanup task, no more TOCTOU race (closes #82 root cause). Adds optional `rate_limit_burst` field on `AgentPolicy` (defaults to `rate_limit` for full backward compatibility). Closes #98. diff --git a/src/gateway.rs b/src/gateway.rs index 0cdd172..72c44e5 100644 --- a/src/gateway.rs +++ b/src/gateway.rs @@ -609,6 +609,17 @@ fn scrub_request_args(mut msg: Value, patterns: &[regex::Regex]) -> Value { /// is replaced with the literal string `"[REDACTED]"`. Returns the filtered value /// and a flag indicating whether anything was redacted. pub fn redact_value(val: Value, patterns: &[regex::Regex]) -> (Value, bool) { + redact_value_depth(val, patterns, 0) +} + +/// Maximum nesting depth accepted during redaction. Payloads deeper than this +/// are returned unchanged (no redaction attempted) to avoid stack overflow. +const REDACT_MAX_DEPTH: usize = 64; + +fn redact_value_depth(val: Value, patterns: &[regex::Regex], depth: usize) -> (Value, bool) { + if depth > REDACT_MAX_DEPTH { + return (val, false); + } match val { Value::String(s) => { if matches_any_variant(&s, patterns) { @@ -622,7 +633,7 @@ pub fn redact_value(val: Value, patterns: &[regex::Regex]) -> (Value, bool) { let new_arr = arr .into_iter() .map(|v| { - let (v, r) = redact_value(v, patterns); + let (v, r) = redact_value_depth(v, patterns, depth + 1); any |= r; v }) @@ -634,7 +645,7 @@ pub fn redact_value(val: Value, patterns: &[regex::Regex]) -> (Value, bool) { let new_obj = obj .into_iter() .map(|(k, v)| { - let (v, r) = redact_value(v, patterns); + let (v, r) = redact_value_depth(v, patterns, depth + 1); any |= r; (k, v) }) diff --git a/src/middleware/payload_filter.rs b/src/middleware/payload_filter.rs index 7a7b403..7349526 100644 --- a/src/middleware/payload_filter.rs +++ b/src/middleware/payload_filter.rs @@ -378,6 +378,35 @@ mod tests { assert!(matches!(mw.check(&ctx).await, Decision::Block { .. })); } + // ── Depth limit ─────────────────────────────────────────────────────────── + + #[tokio::test] + async fn deeply_nested_json_blocked() { + // Build a JSON value nested MAX_DEPTH + 10 levels deep. + let mut val = json!("harmless"); + for _ in 0..(super::MAX_DEPTH + 10) { + val = json!({ "x": val }); + } + let re = Regex::new("secret").unwrap(); + let mw = make_mw(vec![re]); + let ctx = ctx_call("echo", val); + // Must block (depth guard) rather than panic / overflow. + assert!(matches!(mw.check(&ctx).await, Decision::Block { .. })); + } + + #[tokio::test] + async fn max_depth_exactly_allowed() { + // A payload at exactly MAX_DEPTH levels should still be scanned normally. + let mut val = json!("harmless"); + for _ in 0..super::MAX_DEPTH { + val = json!({ "x": val }); + } + let re = Regex::new("secret").unwrap(); + let mw = make_mw(vec![re]); + let ctx = ctx_call("echo", val); + assert!(matches!(mw.check(&ctx).await, Decision::Allow { .. })); + } + // ── SSRF & Domain Bypass ───────────────────────────────────────────────── #[tokio::test] @@ -424,21 +453,52 @@ mod tests { } } -/// Recursively scan JSON value string leaves with encoding-aware pattern matching. -/// Returns the pattern string of the first match, or None if clean. +/// Maximum nesting depth accepted before a payload is treated as a block. +/// Prevents stack overflow on pathological inputs (e.g. 10 000-level deep JSON). +const MAX_DEPTH: usize = 64; + +/// Iterative scan of all JSON string leaves with encoding-aware pattern matching. +/// +/// Uses an explicit stack instead of recursion so deeply-nested payloads cannot +/// overflow the thread stack. Returns the matching pattern string on the first +/// hit, or `None` if the payload is clean. Payloads deeper than `MAX_DEPTH` +/// are treated as a match so they are blocked without further inspection. fn scan_value(val: &Value, patterns: &[regex::Regex]) -> Option { if patterns.is_empty() { return None; } - match val { - Value::String(s) => patterns - .iter() - .find(|p| matches_any_variant(s, std::slice::from_ref(p))) - .map(|p| p.as_str().to_string()), - Value::Array(arr) => arr.iter().find_map(|v| scan_value(v, patterns)), - Value::Object(obj) => obj.values().find_map(|v| scan_value(v, patterns)), - _ => None, + + // Stack entries: (node, current_depth). + let mut stack: Vec<(&Value, usize)> = vec![(val, 0)]; + + while let Some((node, depth)) = stack.pop() { + if depth > MAX_DEPTH { + return Some("[max depth exceeded]".to_string()); + } + match node { + Value::String(s) => { + if let Some(p) = patterns + .iter() + .find(|p| matches_any_variant(s, std::slice::from_ref(p))) + { + return Some(p.as_str().to_string()); + } + } + Value::Array(arr) => { + for item in arr { + stack.push((item, depth + 1)); + } + } + Value::Object(obj) => { + for v in obj.values() { + stack.push((v, depth + 1)); + } + } + _ => {} + } } + + None } pub struct PayloadFilterMiddleware {