From ac6117a9d8a4bebe083caec9757e9c6d16f28428 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 7 Apr 2026 20:56:06 -0600 Subject: [PATCH] Fix shell escaping in getting current env (#53335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/remote/src/transport/ssh.rs | 37 +++++++++----- crates/util/src/shell_env.rs | 79 ++++++++++++++++-------------- 2 files changed, 68 insertions(+), 48 deletions(-) diff --git a/crates/remote/src/transport/ssh.rs b/crates/remote/src/transport/ssh.rs index 1884ea43b6492efba91623eb1ab4c5a1ed4d3de1..a101a3bda1cd6d970cc9335afd948398d80aba50 100644 --- a/crates/remote/src/transport/ssh.rs +++ b/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); diff --git a/crates/util/src/shell_env.rs b/crates/util/src/shell_env.rs index 72c563abe52336c2b5ccc511746834a9a0384aeb..d12c352d541f2737e40c1ea1e18d5816db37ec8b 100644 --- a/crates/util/src/shell_env.rs +++ b/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("ed_zed_path) .into_owned();