fix(builtins): sanitize curl multipart field names to prevent header injection

crates/bashkit/src/builtins/curl.rs

@@ -8,6 +8,7 @@
 //! - URLs must match the configured allowlist
 //! - Response size is limited (default: 10MB) to prevent memory exhaustion
 //! - Timeouts prevent hanging on unresponsive servers
+//! - Multipart field names/filenames are sanitized to prevent header injection (issue #985)
 //! - Redirects are not followed automatically (to prevent allowlist bypass)
 //! - Compression decompression is size-limited to prevent zip bombs
 
@@ -205,6 +206,17 @@
             }
         };
 
+        // Validate multipart field names early to reject injection attempts
+        // even when network is not configured (defense in depth).
+        for field in &form_fields {
+            if let Some(eq_pos) = field.find('=') {
+                let name = &field[..eq_pos];
+                if let Err(e) = sanitize_multipart_name(name, "field name") {
+                    return Ok(ExecResult::err(e, 2));
+                }
+            }
+        }
+
         // Check if network is configured
         #[cfg(feature = "http_client")]
         {
@@ -268,6 +280,18 @@
     }
 }
 
+/// Sanitize a multipart field name or filename to prevent header injection.
+/// Rejects CR/LF characters (which could inject headers) and escapes double quotes.
+fn sanitize_multipart_name(value: &str, label: &str) -> std::result::Result<String, String> {
+    if value.contains('\r') || value.contains('\n') {
+        return Err(format!(
+            "curl: multipart {} contains illegal newline characters\n",
+            label
+        ));
+    }
+    Ok(value.replace('"', "\\\""))
+}
+
 /// Execute the actual curl request when http_client feature is enabled.
 #[cfg(feature = "http_client")]
 #[allow(clippy::too_many_arguments)]
@@ -364,6 +388,13 @@
             if let Some(eq_pos) = field.find('=') {
                 let name = &field[..eq_pos];
                 let value = &field[eq_pos + 1..];
+
+                // Sanitize field name to prevent header injection
+                let safe_name = match sanitize_multipart_name(name, "field name") {
+                    Ok(n) => n,
+                    Err(e) => return Ok(ExecResult::err(e, 2)),
+                };
+
                 body.extend_from_slice(format!("--{}\r\n", boundary).as_bytes());
                 if let Some(file_path) = value.strip_prefix('@') {
                     // File upload: key=@filepath[;type=mime]
@@ -383,10 +414,17 @@
                         .file_name()
                         .map(|n| n.to_string_lossy().to_string())
                         .unwrap_or_else(|| "file".to_string());
+
+                    // Sanitize filename to prevent header injection
+                    let safe_filename = match sanitize_multipart_name(&filename, "filename") {
+                        Ok(n) => n,
+                        Err(e) => return Ok(ExecResult::err(e, 2)),
+                    };
+
                     body.extend_from_slice(
                         format!(
                             "Content-Disposition: form-data; name=\"{}\"; filename=\"{}\"\r\n",
-                            name, filename
+                            safe_name, safe_filename
                         )
                         .as_bytes(),
                     );
@@ -395,8 +433,11 @@
                 } else {
                     // Text field: key=value
                     body.extend_from_slice(
-                        format!("Content-Disposition: form-data; name=\"{}\"\r\n\r\n", name)
-                            .as_bytes(),
+                        format!(
+                            "Content-Disposition: form-data; name=\"{}\"\r\n\r\n",
+                            safe_name
+                        )
+                        .as_bytes(),
                     );
                     body.extend_from_slice(value.as_bytes());
                 }
@@ -649,7 +690,7 @@
 }
 
 /// Check if two URLs have the same origin (scheme + host + port).
 fn same_origin(a: &str, b: &str) -> bool {
     let (Ok(a_url), Ok(b_url)) = (url::Url::parse(a), url::Url::parse(b)) else {
         return false;
     };
@@ -659,7 +700,7 @@
 }
 
 /// Sensitive headers that must not be forwarded cross-origin on redirect.
 const SENSITIVE_HEADERS: &[&str] = &["authorization", "cookie", "proxy-authorization"];
 
 /// Format the -w/--write-out output.
 #[cfg(feature = "http_client")]
@@ -1358,5 +1399,76 @@
             assert_eq!(filtered.len(), 1);
             assert_eq!(filtered[0].0, "Content-Type");
         }
+
+        #[test]
+        fn test_sanitize_multipart_name_normal() {
+            let result = sanitize_multipart_name("field1", "field name").unwrap();
+            assert_eq!(result, "field1");
+        }
+
+        #[test]
+        fn test_sanitize_multipart_name_escapes_quotes() {
+            let result = sanitize_multipart_name("fie\"ld", "field name").unwrap();
+            assert_eq!(result, "fie\\\"ld");
+        }
+
+        #[test]
+        fn test_sanitize_multipart_name_rejects_cr() {
+            let result = sanitize_multipart_name("field\r\nInjected: header", "field name");
+            assert!(result.is_err());
+            assert!(result.unwrap_err().contains("illegal newline"));
+        }
+
+        #[test]
+        fn test_sanitize_multipart_name_rejects_lf() {
+            let result = sanitize_multipart_name("field\nInjected: header", "field name");
+            assert!(result.is_err());
+            assert!(result.unwrap_err().contains("illegal newline"));
+        }
+
+        #[test]
+        fn test_sanitize_multipart_name_rejects_bare_cr() {
+            let result = sanitize_multipart_name("field\rname", "filename");
+            assert!(result.is_err());
+        }
+
+        #[tokio::test]
+        async fn test_curl_multipart_field_name_with_quotes() {
+            // Field name with quotes should be escaped, not cause injection
+            let result = run_curl_with_stdin_and_fs(
+                &["-F", "fie\"ld=value", "https://example.com"],
+                None,
+                &[],
+            )
+            .await;
+            // Should reach network error (field name accepted after escaping)
+            assert!(result.stderr.contains("network access not configured"));
+        }
+
+        #[tokio::test]
+        async fn test_curl_multipart_field_name_with_newline_rejected() {
+            // Field name with newline must be rejected
+            let result = run_curl_with_stdin_and_fs(
+                &["-F", "field\r\nInjected: evil=value", "https://example.com"],
+                None,
+                &[],
+            )
+            .await;
+            assert_ne!(result.exit_code, 0);
+            assert!(result.stderr.contains("illegal newline"));
+        }
+
+        #[tokio::test]
+        async fn test_curl_multipart_normal_field_works() {
+            // Normal field names should work fine
+            let result = run_curl_with_stdin_and_fs(
+                &["-F", "username=alice", "https://example.com"],
+                None,
+                &[],
+            )
+            .await;
+            // Should reach network error (multipart built successfully)
+            assert!(result.stderr.contains("network access not configured"));
+        }
     }
 }