fix(remote): the SSH bootstrap makes its scratch dir, never adopts one

The same hole as the local one, on the far side of the connection and in
generated shell rather than Rust — and on a shared server, which is where
several accounts actually coexist.

  __tty7_d=${TMPDIR:-/tmp}/tty7-zdotdir-$$
  command mkdir -p "$__tty7_d" 2>/dev/null
  command cat > "$__tty7_d/.zshrc" <<'EOF' ...
  ... && export ZDOTDIR="$__tty7_d"

`mkdir -p` succeeds on a directory that already exists, and $$ is the
remote shell's pid, which every other account on that box can read. So
the startup files went into whatever held the name, and its owner could
rewrite them in the moment before zsh read them.

Now `mkdir -m 700` without `-p`, which fails when the path exists, with
the writes moved inside the `if`. A taken name means nothing is written,
the `[ -s ... ]` guards that were already there see nothing, and the login
falls through to the plain `exec <shell> -l` those guards exist for.

Run rather than reasoned about. Against a directory planted at the name
with mode 777 and an `evil` .zshrc: the fixed script leaves ZDOTDIR empty
and the plant untouched, while the `mkdir -p` version exported the
planted path for zsh to read. Both scripts pass `sh -n`, and the ordinary
path still creates drwx------ and exports ZDOTDIR.
This commit is contained in:
l0ng-ai
2026-08-23 17:54:04 +08:00
parent 4764a35fe5
commit a93d5b3bf7
2 changed files with 83 additions and 4 deletions
+8
View File
@@ -153,6 +153,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- The bootstrap tty7 runs on a remote machine now creates its own scratch
directory instead of adopting one. The startup files a remote shell sources
were written into `${TMPDIR:-/tmp}/tty7-zdotdir-$$`, created with `mkdir -p`,
which succeeds on a directory that is already there. On a shared server
another account can read that pid and take the name first, and then rewrite
those files between tty7 writing them and the shell reading them. The
directory is now created exclusively at mode 700; if the name is taken
nothing is written and the login falls through to a plain login shell.
- A shell's scratch directory is now created by tty7 alone, closed to other
accounts. The startup files a pane's shell sources — the redirectors
`ZDOTDIR` points zsh at, and bash's and nu's equivalents — were written into
@@ -1448,6 +1448,29 @@ pub mod remote {
format!("'{}'", s.replace('\\', r"\\").replace('\'', r"\'"))
}
/// Open `$__tty7_d` as a directory this login made, or leave the block
/// unentered.
///
/// The far end sources what goes in here, and the name is
/// `<prefix>-$$` — the remote shell's pid, which on a shared server every
/// other account can read out of `/proc` and get to first. `mkdir -p`
/// succeeds on a directory that is already there, so the startup files
/// went into whatever held the name, and its owner could rewrite them
/// before the shell read them.
///
/// `mkdir` without `-p` fails when the path exists, and `-m 700` puts the
/// mode on the creating syscall rather than a chmod after it. The caller
/// closes the `if`; everything it writes is inside, so a name already
/// taken means the files are never written, the `[ -s ... ]` guards that
/// already exist see nothing, and the login falls through to the plain
/// `exec <shell> -l` those guards were written for.
fn open_scratch_dir(prefix: &str) -> String {
format!(
"__tty7_d=${{TMPDIR:-/tmp}}/{prefix}-$$\n\
if command mkdir -m 700 \"$__tty7_d\" 2>/dev/null; then\n"
)
}
fn write_file(out: &mut String, name: &str, body: &str) {
out.push_str(&format!(
"command cat > \"$__tty7_d/{name}\" <<'{HEREDOC}'\n{}\n{HEREDOC}\n",
@@ -1457,8 +1480,7 @@ pub mod remote {
fn zsh_bootstrap(shell_path: &str) -> String {
let mut out = String::new();
out.push_str("__tty7_d=${TMPDIR:-/tmp}/tty7-zdotdir-$$\n");
out.push_str("command mkdir -p \"$__tty7_d\" 2>/dev/null\n");
out.push_str(&open_scratch_dir("tty7-zdotdir"));
let mut guard = String::new();
for (name, contents) in zsh_redirectors() {
@@ -1473,6 +1495,7 @@ pub mod remote {
out.push_str(&format!(
"{guard}export ZDOTDIR=\"$__tty7_d\" TTY7_RM_DIR=\"$__tty7_d\"\n"
));
out.push_str("fi\n");
out.push_str(&format!("exec {} -l\n", shell_quote(shell_path)));
out
}
@@ -1480,8 +1503,7 @@ pub mod remote {
fn bash_bootstrap(shell_path: &str) -> String {
let quoted = shell_quote(shell_path);
let mut out = String::new();
out.push_str("__tty7_d=${TMPDIR:-/tmp}/tty7-bashrc-$$\n");
out.push_str("command mkdir -p \"$__tty7_d\" 2>/dev/null\n");
out.push_str(&open_scratch_dir("tty7-bashrc"));
write_file(
&mut out,
"bashrc",
@@ -1491,6 +1513,7 @@ pub mod remote {
out.push_str("export TTY7_RM_DIR=\"$__tty7_d\"\n");
out.push_str(&format!("exec {quoted} --rcfile \"$__tty7_d/bashrc\" -i\n"));
out.push_str("fi\n");
out.push_str("fi\n");
out.push_str(&format!("exec {quoted} -l\n"));
out
}
@@ -1539,6 +1562,54 @@ fi
use super::super::{BASH_INTEGRATION, ZSH_INTEGRATION};
use super::*;
/// The remote bootstrap makes its scratch directory or writes nothing.
///
/// The far end sources what goes in there, and the name is the remote
/// shell's `$$` — which on a shared server every other account can
/// read and get to first. `mkdir -p` took whatever held the name, so
/// the startup files were written into a directory somebody else owned
/// and could rewrite before the shell read it.
///
/// Run against a planted directory, the fixed script leaves `ZDOTDIR`
/// empty and the plant untouched; the `mkdir -p` version exported the
/// planted path. Verified by hand that way — what is pinned here is
/// the shape that made the difference.
#[test]
fn a_remote_bootstrap_creates_its_scratch_dir_rather_than_adopting_one() {
for (shell, path) in [
(RemoteShell::Zsh, "/bin/zsh"),
(RemoteShell::Bash, "/bin/bash"),
] {
let script = bootstrap_command(shell, path);
assert!(
script.contains("if command mkdir -m 700 \"$__tty7_d\" 2>/dev/null; then"),
"{shell:?} does not create its own scratch directory:\n{script}"
);
assert!(
!script.contains("mkdir -p"),
"{shell:?} still adopts a directory that is already there:\n{script}"
);
// Everything written has to sit inside that `if`, or a taken
// name means writing to `$__tty7_d` while it is somebody
// else's — or, with the variable cleared, to `/`.
let opened = script.find("; then").expect("the guard is there");
let closed = script.rfind("\nfi\n").expect("the guard is closed");
let writes = script.match_indices("command cat > ");
for (at, _) in writes {
assert!(
at > opened && at < closed,
"{shell:?} writes a startup file outside the guard:\n{script}"
);
}
// And the fallback the guard drops into is still a plain login
// shell, which is what makes refusing a taken name safe.
assert!(
script.trim_end().ends_with(&format!("exec '{path}' -l")),
"{shell:?} has no plain login shell to fall back to:\n{script}"
);
}
}
#[test]
fn probe_reads_the_line_after_the_marker() {
assert_eq!(