Преглед на файлове

fix: address soundness issues from audit

- Add docker image validation in doctor repair to prevent Dockerfile injection
- Add socket interface name validation in health check to prevent command injection
- Add node validation in stop_cluster to fail when specified node is not found
Alexander Korolev преди 4 месеца
родител
ревизия
98abe64703
променени са 3 файла, в които са добавени 42 реда и са изтрити 7 реда
  1. 9 3
      src/docker/health.rs
  2. 16 4
      src/docker/mod.rs
  3. 17 0
      src/doctor.rs

+ 9 - 3
src/docker/health.rs

@@ -42,11 +42,17 @@ pub async fn health_check(config: &Config, cluster: &Cluster, dry_run: bool) ->
     };
 
     // Build the network check command snippet for this cluster's NCCL interfaces.
-    let net_script = nccl.map(|n| {
+    let net_script = nccl.as_ref().and_then(|n| {
         let socket = &n.socket_ifname;
-        format!(
+        // Validate socket interface name to prevent command injection
+        // Must be a valid network interface name: alphanumeric, dots, underscores, hyphens
+        let valid = socket.chars().all(|c| c.is_alphanumeric() || ".-_".contains(c));
+        if !valid {
+            return None;
+        }
+        Some(format!(
             "echo '---NET---'; echo -n 'socket:' && (ip -o link show dev {socket} 2>/dev/null | grep -qi 'state UP' && echo UP || echo DOWN); echo '---END---'"
-        )
+        ))
     });
 
     // SSH into each node in parallel.

+ 16 - 4
src/docker/mod.rs

@@ -139,10 +139,22 @@ pub async fn stop_cluster(
     let nodes: Vec<&Node> = if nodes.is_empty() {
         cluster.nodes.iter().collect()
     } else {
-        nodes
-            .iter()
-            .filter_map(|nn| cluster.nodes.iter().find(|n| n.name == *nn))
-            .collect()
+        let mut found_nodes: Vec<&Node> = Vec::new();
+        let mut not_found: Vec<String> = Vec::new();
+        for nn in nodes {
+            if let Some(node) = cluster.nodes.iter().find(|n| n.name == *nn) {
+                found_nodes.push(node);
+            } else {
+                not_found.push(nn.clone());
+            }
+        }
+        if !not_found.is_empty() {
+            let names = not_found.join(", ");
+            anyhow::bail!(
+                fluent_i18n::t!("exec-node-not-found", { "name" => &names, "cluster" => &cluster.name })
+            );
+        }
+        found_nodes
     };
     for node in &nodes {
         run_ssh(

+ 17 - 0
src/doctor.rs

@@ -282,8 +282,25 @@ async fn run_cluster_repair(
     Ok(repaired_tag)
 }
 
+/// Validate docker image name to prevent Dockerfile injection.
+/// Returns true if the image name is safe to use in a FROM statement.
+fn is_valid_docker_image(image: &str) -> bool {
+    // Valid docker image pattern: [registry/]namespace/repo[:tag|@digest]
+    // Allow alphanumeric, dots, dashes, underscores, slashes, colons, and @
+    // No newlines, no shell metacharacters
+    image
+        .chars()
+        .all(|c| c.is_alphanumeric() || ".-_/:@".contains(c))
+        && !image.contains('\\')
+        && !image.contains('\n')
+        && !image.contains('\r')
+}
+
 /// Generate a Dockerfile to repair a container.
 fn generate_repair_dockerfile(base_image: &str, recipe: &Recipe) -> String {
+    if !is_valid_docker_image(base_image) {
+        panic!("Invalid docker image name: {}", base_image);
+    }
     let mut lines = vec![format!("FROM {}", base_image)];
 
     if recipe.distributed_backend == Some(crate::config::DistributedBackend::Ray) {