Skip to content

Commit a71c38c

Browse files
ConradIrwinZed Zippy
authored andcommitted
Fix shell escaping in getting current env (#53335)
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
1 parent c8a9156 commit a71c38c

File tree

2 files changed

+68
-48
lines changed

2 files changed

+68
-48
lines changed

crates/remote/src/transport/ssh.rs

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1639,20 +1639,35 @@ fn build_command_posix(
16391639
if let Some(working_dir) = working_dir {
16401640
let working_dir = RemotePathBuf::new(working_dir, ssh_path_style).to_string();
16411641

1642-
// shlex will wrap the command in single quotes (''), disabling ~ expansion,
1643-
// replace with something that works
1644-
const TILDE_PREFIX: &'static str = "~/";
1642+
// For paths starting with ~/, we need $HOME to expand, but the remainder
1643+
// must be properly quoted to prevent command injection.
1644+
// Pattern: cd "$HOME"/'quoted/remainder' - $HOME expands, rest is single-quoted
1645+
const TILDE_PREFIX: &str = "~/";
16451646
if working_dir.starts_with(TILDE_PREFIX) {
1646-
let working_dir = working_dir.trim_start_matches("~").trim_start_matches("/");
1647-
write!(
1648-
exec,
1649-
"cd \"$HOME/{working_dir}\" {} ",
1650-
ssh_shell_kind.sequential_and_commands_separator()
1651-
)?;
1647+
let remainder = working_dir.trim_start_matches(TILDE_PREFIX);
1648+
if remainder.is_empty() {
1649+
write!(
1650+
exec,
1651+
"cd \"$HOME\" {} ",
1652+
ssh_shell_kind.sequential_and_commands_separator()
1653+
)?;
1654+
} else {
1655+
let quoted_remainder = ssh_shell_kind
1656+
.try_quote(remainder)
1657+
.context("shell quoting")?;
1658+
write!(
1659+
exec,
1660+
"cd \"$HOME\"/{quoted_remainder} {} ",
1661+
ssh_shell_kind.sequential_and_commands_separator()
1662+
)?;
1663+
}
16521664
} else {
1665+
let quoted_dir = ssh_shell_kind
1666+
.try_quote(&working_dir)
1667+
.context("shell quoting")?;
16531668
write!(
16541669
exec,
1655-
"cd \"{working_dir}\" {} ",
1670+
"cd {quoted_dir} {} ",
16561671
ssh_shell_kind.sequential_and_commands_separator()
16571672
)?;
16581673
}
@@ -1881,7 +1896,7 @@ mod tests {
18811896
"-q",
18821897
"-t",
18831898
"user@host",
1884-
"cd \"$HOME/work\" && exec env 'INPUT_VA=val' remote_program arg1 arg2"
1899+
"cd \"$HOME\"/work && exec env 'INPUT_VA=val' remote_program arg1 arg2"
18851900
]
18861901
);
18871902
assert_eq!(command.env, env);

crates/util/src/shell_env.rs

Lines changed: 42 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ async fn capture_unix(
3737
use crate::command::new_std_command;
3838

3939
let shell_kind = ShellKind::new(shell_path, false);
40-
let zed_path = super::get_shell_safe_zed_path(shell_kind)?;
40+
let quoted_zed_path = super::get_shell_safe_zed_path(shell_kind)?;
4141

4242
let mut command_string = String::new();
4343
let mut command = new_std_command(shell_path);
@@ -82,12 +82,23 @@ async fn capture_unix(
8282
_ => command.args(["-i", "-c"]),
8383
};
8484

85+
// Prefix with "./" if the path starts with "-" to prevent cd from interpreting it as a flag
86+
let dir_str = directory.to_string_lossy();
87+
let dir_str = if dir_str.starts_with('-') {
88+
format!("./{dir_str}").into()
89+
} else {
90+
dir_str
91+
};
92+
let quoted_dir = shell_kind
93+
.try_quote(&dir_str)
94+
.context("unexpected null in directory name")?;
95+
8596
// cd into the directory, triggering directory specific side-effects (asdf, direnv, etc)
86-
command_string.push_str(&format!("cd '{}';", directory.display()));
97+
command_string.push_str(&format!("cd {};", quoted_dir));
8798
if let Some(prefix) = shell_kind.command_prefix() {
8899
command_string.push(prefix);
89100
}
90-
command_string.push_str(&format!("{} --printenv {}", zed_path, redir));
101+
command_string.push_str(&format!("{} --printenv {}", quoted_zed_path, redir));
91102

92103
if let ShellKind::Nushell = shell_kind {
93104
command_string.push_str("; exit");
@@ -155,56 +166,50 @@ async fn capture_windows(
155166
std::env::current_exe().context("Failed to determine current zed executable path.")?;
156167

157168
let shell_kind = ShellKind::new(shell_path, true);
169+
// Prefix with "./" if the path starts with "-" to prevent cd from interpreting it as a flag
158170
let directory_string = directory.display().to_string();
171+
let directory_string = if directory_string.starts_with('-') {
172+
format!("./{directory_string}")
173+
} else {
174+
directory_string
175+
};
159176
let zed_path_string = zed_path.display().to_string();
160177
let quote_for_shell = |value: &str| {
161178
shell_kind
162179
.try_quote(value)
163180
.map(|quoted| quoted.into_owned())
164-
.unwrap_or_else(|| value.to_owned())
181+
.context("unexpected null in directory name")
165182
};
166183
let mut cmd = crate::command::new_command(shell_path);
167184
cmd.args(args);
185+
let quoted_directory = quote_for_shell(&directory_string)?;
186+
let quoted_zed_path = quote_for_shell(&zed_path_string)?;
168187
let cmd = match shell_kind {
169188
ShellKind::Csh
170189
| ShellKind::Tcsh
171190
| ShellKind::Rc
172191
| ShellKind::Fish
173192
| ShellKind::Xonsh
174-
| ShellKind::Posix => {
175-
let quoted_directory = quote_for_shell(&directory_string);
176-
let quoted_zed_path = quote_for_shell(&zed_path_string);
177-
cmd.args([
178-
"-l",
179-
"-i",
180-
"-c",
181-
&format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
182-
])
183-
}
184-
ShellKind::PowerShell | ShellKind::Pwsh => {
185-
let quoted_directory = ShellKind::quote_pwsh(&directory_string);
186-
let quoted_zed_path = ShellKind::quote_pwsh(&zed_path_string);
187-
cmd.args([
188-
"-NonInteractive",
189-
"-NoProfile",
190-
"-Command",
191-
&format!(
192-
"Set-Location {}; & {} --printenv",
193-
quoted_directory, quoted_zed_path
194-
),
195-
])
196-
}
197-
ShellKind::Elvish => {
198-
let quoted_directory = quote_for_shell(&directory_string);
199-
let quoted_zed_path = quote_for_shell(&zed_path_string);
200-
cmd.args([
201-
"-c",
202-
&format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
203-
])
204-
}
193+
| ShellKind::Posix => cmd.args([
194+
"-l",
195+
"-i",
196+
"-c",
197+
&format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
198+
]),
199+
ShellKind::PowerShell | ShellKind::Pwsh => cmd.args([
200+
"-NonInteractive",
201+
"-NoProfile",
202+
"-Command",
203+
&format!(
204+
"Set-Location {}; & {} --printenv",
205+
quoted_directory, quoted_zed_path
206+
),
207+
]),
208+
ShellKind::Elvish => cmd.args([
209+
"-c",
210+
&format!("cd {}; {} --printenv", quoted_directory, quoted_zed_path),
211+
]),
205212
ShellKind::Nushell => {
206-
let quoted_directory = quote_for_shell(&directory_string);
207-
let quoted_zed_path = quote_for_shell(&zed_path_string);
208213
let zed_command = shell_kind
209214
.prepend_command_prefix(&quoted_zed_path)
210215
.into_owned();

0 commit comments

Comments
 (0)