diff --git a/crates/bashkit/src/builtins/http.rs b/crates/bashkit/src/builtins/http.rs index cf7b597b..98ea59e4 100644 --- a/crates/bashkit/src/builtins/http.rs +++ b/crates/bashkit/src/builtins/http.rs @@ -184,23 +184,27 @@ fn parse_http_args(args: &[String]) -> std::result::Result { }) } -/// Build query string from query params. +/// Build query string from query params with proper URL encoding. +// THREAT[TM-NET-019]: URL-encode query params to prevent parameter injection fn build_url_with_query(base_url: &str, items: &[ItemType]) -> String { - let params: Vec = items + let query_items: Vec<(&str, &str)> = items .iter() .filter_map(|item| { if let ItemType::QueryParam(k, v) = item { - Some(format!("{}={}", k, v)) + Some((k.as_str(), v.as_str())) } else { None } }) .collect(); - if params.is_empty() { + if query_items.is_empty() { return base_url.to_string(); } + let encoded: String = url::form_urlencoded::Serializer::new(String::new()) + .extend_pairs(query_items) + .finish(); let sep = if base_url.contains('?') { "&" } else { "?" }; - format!("{}{}{}", base_url, sep, params.join("&")) + format!("{}{}{}", base_url, sep, encoded) } /// Build JSON body from items using serde_json for proper escaping. @@ -227,19 +231,22 @@ fn build_json_body(items: &[ItemType]) -> String { serde_json::to_string_pretty(&serde_json::Value::Object(map)).unwrap_or_default() } -/// Build form body from items. +/// Build form body from items with proper URL encoding. +// THREAT[TM-NET-020]: URL-encode form values to prevent field injection fn build_form_body(items: &[ItemType]) -> String { - let pairs: Vec = items + let form_items: Vec<(&str, &str)> = items .iter() .filter_map(|item| { if let ItemType::JsonField(k, v) = item { - Some(format!("{}={}", k, v)) + Some((k.as_str(), v.as_str())) } else { None } }) .collect(); - pairs.join("&") + url::form_urlencoded::Serializer::new(String::new()) + .extend_pairs(form_items) + .finish() } /// Format the parsed request for display. @@ -629,4 +636,48 @@ mod tests { let parsed: serde_json::Value = serde_json::from_str(&body).unwrap(); assert_eq!(parsed["count"].as_i64().unwrap(), 42); } + + #[test] + fn test_query_param_injection_encoded() { + let items = vec![ItemType::QueryParam( + "q".to_string(), + "foo&admin=true".to_string(), + )]; + let url = build_url_with_query("https://example.com", &items); + // The & in the value must be encoded, not treated as a param separator + assert!(!url.contains("admin=true")); + assert!(url.contains("q=foo%26admin%3Dtrue") || url.contains("q=foo%26admin=true")); + } + + #[test] + fn test_query_param_normal_value() { + let items = vec![ItemType::QueryParam( + "search".to_string(), + "hello world".to_string(), + )]; + let url = build_url_with_query("https://example.com", &items); + assert!(url.contains("search=hello")); + } + + #[test] + fn test_form_body_injection_encoded() { + let items = vec![ItemType::JsonField( + "user".to_string(), + "admin&role=superadmin".to_string(), + )]; + let body = build_form_body(&items); + // The & in the value must be encoded + assert!(!body.contains("role=superadmin")); + assert!( + body.contains("user=admin%26role%3Dsuperadmin") + || body.contains("user=admin%26role%3Dsuperadmin") + ); + } + + #[test] + fn test_form_body_normal_value() { + let items = vec![ItemType::JsonField("name".to_string(), "test".to_string())]; + let body = build_form_body(&items); + assert_eq!(body, "name=test"); + } } diff --git a/specs/006-threat-model.md b/specs/006-threat-model.md index bfb2150b..58f97286 100644 --- a/specs/006-threat-model.md +++ b/specs/006-threat-model.md @@ -606,6 +606,8 @@ allowlist.allow("https://api.example.com"); | 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** | +| TM-NET-019 | Query param injection | `http GET url q=='foo&admin=true'` injects extra params | URL-encode via `url::form_urlencoded` | **MITIGATED** | +| TM-NET-020 | Form body injection | `http --form POST url user='x&role=admin'` injects extra fields | URL-encode via `url::form_urlencoded` | **MITIGATED** | **Current Risk**: LOW - Multiple mitigations in place