diff --git a/crates/bashkit/src/builtins/http.rs b/crates/bashkit/src/builtins/http.rs index 7e3c796f..cf7b597b 100644 --- a/crates/bashkit/src/builtins/http.rs +++ b/crates/bashkit/src/builtins/http.rs @@ -203,24 +203,28 @@ fn build_url_with_query(base_url: &str, items: &[ItemType]) -> String { format!("{}{}{}", base_url, sep, params.join("&")) } -/// Build JSON body from items. +/// Build JSON body from items using serde_json for proper escaping. +// THREAT[TM-NET-018]: serde_json prevents JSON injection via special characters in values fn build_json_body(items: &[ItemType]) -> String { - let mut fields = Vec::new(); + let mut map = serde_json::Map::new(); for item in items { match item { ItemType::JsonField(k, v) => { - fields.push(format!(" \"{}\": \"{}\"", k, v)); + map.insert(k.clone(), serde_json::Value::String(v.clone())); } ItemType::JsonRawField(k, v) => { - fields.push(format!(" \"{}\": {}", k, v)); + // Raw values are pre-formatted JSON (numbers, booleans, etc.) + let raw: serde_json::Value = + serde_json::from_str(v).unwrap_or(serde_json::Value::String(v.clone())); + map.insert(k.clone(), raw); } _ => {} } } - if fields.is_empty() { + if map.is_empty() { return String::new(); } - format!("{{\n{}\n}}", fields.join(",\n")) + serde_json::to_string_pretty(&serde_json::Value::Object(map)).unwrap_or_default() } /// Build form body from items. @@ -587,4 +591,42 @@ mod tests { )) ); } + + #[test] + fn test_json_body_escapes_quotes() { + let items = vec![ItemType::JsonField( + "name".to_string(), + r#"test","admin":true,"x":"y"#.to_string(), + )]; + let body = build_json_body(&items); + // Must produce valid JSON — injected field must NOT appear as separate key + let parsed: serde_json::Value = serde_json::from_str(&body).unwrap(); + assert_eq!( + parsed["name"].as_str().unwrap(), + r#"test","admin":true,"x":"y"# + ); + assert!(parsed.get("admin").is_none()); + } + + #[test] + fn test_json_body_escapes_backslash_and_newline() { + let items = vec![ItemType::JsonField( + "msg".to_string(), + "line1\nline2\\end".to_string(), + )]; + let body = build_json_body(&items); + let parsed: serde_json::Value = serde_json::from_str(&body).unwrap(); + assert_eq!(parsed["msg"].as_str().unwrap(), "line1\nline2\\end"); + } + + #[test] + fn test_json_body_raw_field_unchanged() { + let items = vec![ItemType::JsonRawField( + "count".to_string(), + "42".to_string(), + )]; + let body = build_json_body(&items); + let parsed: serde_json::Value = serde_json::from_str(&body).unwrap(); + assert_eq!(parsed["count"].as_i64().unwrap(), 42); + } } diff --git a/specs/006-threat-model.md b/specs/006-threat-model.md index ea8a1fee..bfb2150b 100644 --- a/specs/006-threat-model.md +++ b/specs/006-threat-model.md @@ -605,6 +605,7 @@ allowlist.allow("https://api.example.com"); | TM-NET-012 | Chunked encoding bomb | Infinite chunked response | Response size limit (streaming) | **MITIGATED** | | TM-NET-013 | Gzip bomb / Zip bomb | 10KB gzip → 10GB decompressed | Auto-decompression disabled | **MITIGATED** | | TM-NET-014 | DNS rebind via redirect | Redirect to rebinded IP | Manual redirect requires allowlist check | **MITIGATED** | +| TM-NET-018 | JSON body injection | `http POST url name='x","admin":true'` via unescaped string formatting | Use `serde_json` for JSON construction | **MITIGATED** | **Current Risk**: LOW - Multiple mitigations in place