From dee99b6616772ee39acb4e1cc404d2cbbd7e31e1 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 2 Apr 2026 09:20:30 +0000 Subject: [PATCH 1/2] fix(builtins): use serde_json to prevent JSON injection in HTTP build_json_body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1000 — build_json_body was constructing JSON via string formatting without escaping special characters, allowing injection of arbitrary JSON fields. Now uses serde_json::Value::String for proper escaping. --- crates/bashkit/src/builtins/http.rs | 50 +++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/crates/bashkit/src/builtins/http.rs b/crates/bashkit/src/builtins/http.rs index 7e3c796f..4a5d100a 100644 --- a/crates/bashkit/src/builtins/http.rs +++ b/crates/bashkit/src/builtins/http.rs @@ -203,24 +203,27 @@ 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. 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 +590,39 @@ 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); + } } From f007f53b6a474a240f666f1f6bb160ec89f8ba82 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 2 Apr 2026 09:25:43 +0000 Subject: [PATCH 2/2] chore(specs): add TM-NET-018 threat entry for JSON body injection --- crates/bashkit/src/builtins/http.rs | 6 +++++- specs/006-threat-model.md | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/bashkit/src/builtins/http.rs b/crates/bashkit/src/builtins/http.rs index 4a5d100a..cf7b597b 100644 --- a/crates/bashkit/src/builtins/http.rs +++ b/crates/bashkit/src/builtins/http.rs @@ -204,6 +204,7 @@ fn build_url_with_query(base_url: &str, items: &[ItemType]) -> String { } /// 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 map = serde_json::Map::new(); for item in items { @@ -620,7 +621,10 @@ mod tests { #[test] fn test_json_body_raw_field_unchanged() { - let items = vec![ItemType::JsonRawField("count".to_string(), "42".to_string())]; + 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