Fix shell escaping in getting current env (#53335)

Conrad Irwin created

Credit to Dario Weißer for bringing this to our attention.

Self-Review Checklist:

- [ ] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [ ] Tests cover the new/changed behavior
- [ ] Performance impact has been considered and is acceptable

Closes #ISSUE

Release Notes:

- Fixed a bug where a cleverly crafted directory name could lead to
remote code execution

Change summary

crates/remote/src/transport/ssh.rs | 37 ++++++++++----
crates/util/src/shell_env.rs       | 79 +++++++++++++++++--------------
2 files changed, 68 insertions(+), 48 deletions(-)

Detailed changes

crates/remote/src/transport/ssh.rs 🔗

@@ -1639,20 +1639,35 @@ fn build_command_posix(
     if let Some(working_dir) = working_dir {
         let working_dir = RemotePathBuf::new(working_dir, ssh_path_style).to_string();
 
-        // shlex will wrap the command in single quotes (''), disabling ~ expansion,
-        // replace with something that works
-        const TILDE_PREFIX: &'static str = "~/";
+        // For paths starting with ~/, we need $HOME to expand, but the remainder
+        // must be properly quoted to prevent command injection.
+        // Pattern: cd "$HOME"/'quoted/remainder' - $HOME expands, rest is single-quoted
+        const TILDE_PREFIX: &str = "~/";
         if working_dir.starts_with(TILDE_PREFIX) {
-            let working_dir = working_dir.trim_start_matches("~").trim_start_matches("/");
-            write!(
-                exec,
-                "cd \"$HOME/{working_dir}\" {} ",
-                ssh_shell_kind.sequential_and_commands_separator()
-            )?;
+            let remainder = working_dir.trim_start_matches(TILDE_PREFIX);
+            if remainder.is_empty() {
+                write!(
+                    exec,
+                    "cd \"$HOME\" {} ",
+                    ssh_shell_kind.sequential_and_commands_separator()
+                )?;
+            } else {
+                let quoted_remainder = ssh_shell_kind
+                    .try_quote(remainder)
+                    .context("shell quoting")?;
+                write!(
+                    exec,
+                    "cd \"$HOME\"/{quoted_remainder} {} ",
+                    ssh_shell_kind.sequential_and_commands_separator()
+                )?;
+            }
         } else {
+            let quoted_dir = ssh_shell_kind
+                .try_quote(&working_dir)
+                .context("shell quoting")?;
             write!(
                 exec,
-                "cd \"{working_dir}\" {} ",
+                "cd {quoted_dir} {} ",
                 ssh_shell_kind.sequential_and_commands_separator()
             )?;
         }
@@ -1881,7 +1896,7 @@ mod tests {
                 "-q",
                 "-t",
                 "user@host",
-                "cd \"$HOME/work\" && exec env 'INPUT_VA=val' remote_program arg1 arg2"
+                "cd \"$HOME\"/work && exec env 'INPUT_VA=val' remote_program arg1 arg2"
             ]
         );
         assert_eq!(command.env, env);

crates/util/src/shell_env.rs 🔗

@@ -49,7 +49,7 @@ async fn capture_unix(
     use crate::command::new_std_command;
 
     let shell_kind = ShellKind::new(shell_path, false);
-    let zed_path = super::get_shell_safe_zed_path(shell_kind)?;
+    let quoted_zed_path = super::get_shell_safe_zed_path(shell_kind)?;
 
     let mut command_string = String::new();
     let mut command = new_std_command(shell_path);
@@ -94,12 +94,23 @@ async fn capture_unix(
         _ => command.args(["-i", "-c"]),
     };
 
+    // Prefix with "./" if the path starts with "-" to prevent cd from interpreting it as a flag
+    let dir_str = directory.to_string_lossy();
+    let dir_str = if dir_str.starts_with('-') {
+        format!("./{dir_str}").into()
+    } else {
+        dir_str
+    };
+    let quoted_dir = shell_kind
+        .try_quote(&dir_str)
+        .context("unexpected null in directory name")?;
+
     // cd into the directory, triggering directory specific side-effects (asdf, direnv, etc)
-    command_string.push_str(&format!("cd '{}';", directory.display()));
+    command_string.push_str(&format!("cd {};", quoted_dir));
     if let Some(prefix) = shell_kind.command_prefix() {
         command_string.push(prefix);
     }
-    command_string.push_str(&format!("{} --printenv {}", zed_path, redir));
+    command_string.push_str(&format!("{} --printenv {}", quoted_zed_path, redir));
 
     if let ShellKind::Nushell = shell_kind {
         command_string.push_str("; exit");
@@ -166,56 +177,50 @@ async fn capture_windows(
         std::env::current_exe().context("Failed to determine current zed executable path.")?;
 
     let shell_kind = ShellKind::new(shell_path, true);
+    // Prefix with "./" if the path starts with "-" to prevent cd from interpreting it as a flag
     let directory_string = directory.display().to_string();
+    let directory_string = if directory_string.starts_with('-') {
+        format!("./{directory_string}")
+    } else {
+        directory_string
+    };
     let zed_path_string = zed_path.display().to_string();
     let quote_for_shell = |value: &str| {
         shell_kind
             .try_quote(value)
             .map(|quoted| quoted.into_owned())
-            .unwrap_or_else(|| value.to_owned())
+            .context("unexpected null in directory name")
     };
     let mut cmd = crate::command::new_command(shell_path);
     cmd.args(args);
+    let quoted_directory = quote_for_shell(&directory_string)?;
+    let quoted_zed_path = quote_for_shell(&zed_path_string)?;
     let cmd = match shell_kind {
         ShellKind::Csh
         | ShellKind::Tcsh
         | ShellKind::Rc
         | ShellKind::Fish
         | ShellKind::Xonsh
-        | ShellKind::Posix => {
-            let quoted_directory = quote_for_shell(&directory_string);
-            let quoted_zed_path = quote_for_shell(&zed_path_string);
-            cmd.args([
-                "-l",
-                "-i",
-                "-c",
-                &format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
-            ])
-        }
-        ShellKind::PowerShell | ShellKind::Pwsh => {
-            let quoted_directory = ShellKind::quote_pwsh(&directory_string);
-            let quoted_zed_path = ShellKind::quote_pwsh(&zed_path_string);
-            cmd.args([
-                "-NonInteractive",
-                "-NoProfile",
-                "-Command",
-                &format!(
-                    "Set-Location {}; & {} --printenv",
-                    quoted_directory, quoted_zed_path
-                ),
-            ])
-        }
-        ShellKind::Elvish => {
-            let quoted_directory = quote_for_shell(&directory_string);
-            let quoted_zed_path = quote_for_shell(&zed_path_string);
-            cmd.args([
-                "-c",
-                &format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
-            ])
-        }
+        | ShellKind::Posix => cmd.args([
+            "-l",
+            "-i",
+            "-c",
+            &format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
+        ]),
+        ShellKind::PowerShell | ShellKind::Pwsh => cmd.args([
+            "-NonInteractive",
+            "-NoProfile",
+            "-Command",
+            &format!(
+                "Set-Location {}; & {} --printenv",
+                quoted_directory, quoted_zed_path
+            ),
+        ]),
+        ShellKind::Elvish => cmd.args([
+            "-c",
+            &format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
+        ]),
         ShellKind::Nushell => {
-            let quoted_directory = quote_for_shell(&directory_string);
-            let quoted_zed_path = quote_for_shell(&zed_path_string);
             let zed_command = shell_kind
                 .prepend_command_prefix(&quoted_zed_path)
                 .into_owned();