security: prevent SSH option injection via URL host (#325)
The `sshurl.Parse()` function and `openSSHSource()` function in cliamp were vulnerable to SSH option injection through the URL host field. Go's `url.Parse` accepts `-oProxyCommand=...` in the host field, which cliamp's `SSHArgs()` function appends bare to the ssh argv, allowing arbitrary command execution. This commit adds defense-in validation at two layers? 1. `internal/sshurl/sshurl.go:50` - Rejects hostnames starting with `-` (the `-o` ssh option prefix) or containing `=` (key-value separator) during URL parsing. 2. `player/decode.go:101` - Defense-in-depth check in `openSSHSource()` that validates the parsed host after `sshurl.Parse()` returns, rejecting the same disallowed patterns before constructing the ssh command. On OpenSSH version above 9.6, an additional `ssh_valid_hostname()` check blocks the destination hostname `cat -- /x` from being accepted. However, the code-layer validation is still necessary because: - The `-oProxyCommand=...` option injection itself is not blocked by OpenSSH's hostname check (the option value itself is accepted?) - On OpenSSH below 9.6 (the vast deployed base: Ubuntu 22.04/24.04, Debian 12, RHEL/CentOS, macOS), the code-layer validation is the only protection Both checks use `strings.HasPrefix(host, "-") || strings.Contains(host, "=")` to catch the injection vector while still preserving actaul legitimate `ssh://host/path` links.
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
"net"
|
||||
"net/url"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Parsed holds the components of an ssh:// URL.
|
||||
@@ -44,6 +45,12 @@ func Parse(raw string) (Parsed, error) {
|
||||
host = u.User.Username() + "@" + host
|
||||
}
|
||||
|
||||
// Reject hostnames that start with - (ssh -o option prefix) or contain =
|
||||
// (= would separate a key/value pair, neither a valid hostname pattern).
|
||||
if strings.HasPrefix(host, "-") || strings.Contains(host, "=") {
|
||||
return Parsed{}, fmt.Errorf("invalid ssh URL %q: host %q contains disallowed characters", raw, host)
|
||||
}
|
||||
|
||||
port := u.Port()
|
||||
// net/url splits host:port correctly; verify with SplitHostPort for edge cases.
|
||||
if port == "" && u.Host != host {
|
||||
|
||||
@@ -96,6 +96,12 @@ func openSSHSource(path string) (sourceResult, error) {
|
||||
return sourceResult{}, err
|
||||
}
|
||||
|
||||
// Defense-in-depth: reject hosts that start with - or contain =, which would
|
||||
// indicate ssh option injection (e.g. -oProxyCommand=...) injected via the URL host.
|
||||
if strings.HasPrefix(parsed.Host, "-") || strings.Contains(parsed.Host, "=") {
|
||||
return sourceResult{}, fmt.Errorf("invalid ssh URL %q: host %q contains disallowed characters", path, parsed.Host)
|
||||
}
|
||||
|
||||
catCmd := "cat -- " + shellQuoteSSH(parsed.Path)
|
||||
args := parsed.SSHArgs()
|
||||
args = append(args, catCmd)
|
||||
|
||||
Reference in New Issue
Block a user