Reject option-like SSH destinations and terminate ssh options

0aac82b→main·

Soph·1mo ago·3 files·+79 added/-2 removed

sshInvocationArgs built ssh -o BatchMode=yes [-p port] <destination> <command>
with no -- separator and no validation of the destination, which comes straight from the URL. A URL like ssh://-oProxyCommand=evil/repo made ssh parse the destination as an option — ProxyCommand means arbitrary local command execution (git's CVE-2017-1000117 class).

Reject any host, username, or port that begins with - (the portable primary guard, matching git's own fix), and insert -- before the destination as defense in depth so options can never run past it. Update the ssh test shim to skip the new -- so the invocation assertions are unchanged, and add coverage for both the rejection and the -- placement.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

191ffd473fdbView transcript

Changes

3

• internal

• gitproto

• Mssh.go+27/-2

• Mssh_test.go+49

• syncer

• Mssh_integration_test.go+3

143 unmodified lines

144
145
146
147
148
149
150
151
149
152
153
154
155
156
157
158
2 unmodified lines

161
162
163
164
165
166
167
159
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191

143 unmodified lines

}
    args := []string{"-o", "BatchMode=yes"}
    if port := ep.Port(); port != "" {
        if err := rejectOptionLike("SSH port", port); err != nil {
            return nil, err
        }
        args = append(args, "-p", port)
    }
    args = append(args, destination, remoteCommand)
    // The "--" terminates ssh option parsing so the destination can never be
    // consumed as a flag; rejectOptionLike below is the portable primary guard
    // (older clients ignore unknown operands but all support "--").
    args = append(args, "--", destination, remoteCommand)
    return args, nil
}

2 unmodified lines

return "", errors.New("missing SSH host")
    }
    host := ep.Hostname()
    if err := rejectOptionLike("SSH host", host); err != nil {
        return "", err
    }
    if ep.User != nil && ep.User.Username() != "" {
        return ep.User.Username() + "@" + host, nil
        user := ep.User.Username()
        if err := rejectOptionLike("SSH username", user); err != nil {
            return "", err
        }
        return user + "@" + host, nil
    }
    return host, nil
}

// rejectOptionLike refuses a destination component that begins with "-". Such
// a value would be parsed by ssh as an option rather than an operand — e.g. a
// host of "-oProxyCommand=..." turns into arbitrary local command execution
// (the class of git's CVE-2017-1000117) — and is never a legitimate host,
// username, or port.
func rejectOptionLike(what, value string) error {
    if strings.HasPrefix(value, "-") {
        return fmt.Errorf("refusing %s %q: must not begin with '-'", what, value)
    }
    return nil
}

func sshRemoteCommand(ep *url.URL, service string, gitProtocol string) (string, error) {
    if ep == nil || ep.Path == "" {
        return "", errors.New("missing SSH repository path")
    }

Minternal/gitproto/ssh.go+27/-2

3 unmodified lines

4 5 6 7 8 9 10 4 unmodified lines

15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 205 unmodified lines

268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285

3 unmodified lines

"context" "errors" "io" "net/url" "os" "path/filepath" "strconv" 4 unmodified lines

"github.com/go-git/go-git/v6/plumbing/transport";

// A host or username beginning with "-" must be refused, not handed to ssh as // an operand: ssh would parse "-oProxyCommand=..." as an option and execute an // arbitrary local command (git's CVE-2017-1000117 class). func TestSSHInvocationArgsRejectsOptionLikeDestination(t *testing.T) { cases := []struct { name string ep *url.URL }{ {"host", &url.URL{Scheme: "ssh", Host: "-oProxyCommand=touch /tmp/pwned", Path: "/repo.git"}}, {"username", &url.URL{Scheme: "ssh", User: url.User("-oProxyCommand=x"), Host: "example.com", Path: "/repo.git"}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { if _, err := sshInvocationArgs(tc.ep, "git-upload-pack", ""); err == nil { t.Fatalf("expected option-like %s to be rejected, got nil error", tc.name) } }) } }

// "--" must precede the destination so ssh stops parsing options at it. func TestSSHInvocationArgsTerminatesOptionsBeforeDestination(t *testing.T) { ep := &url.URL{Scheme: "ssh", User: url.User("alice"), Host: "example.com:2222", Path: "/repo.git"} args, err := sshInvocationArgs(ep, "git-upload-pack", "") if err != nil { t.Fatalf("sshInvocationArgs: %v", err) } destIdx := -1 for i, a := range args { if a == "alice@example.com" { destIdx = i break } } if destIdx <= 0 { t.Fatalf("destination not found in args: %v", args) } if args[destIdx-1] != "--" { t.Fatalf("expected "--" immediately before destination, got %v", args) } }

func TestNewSSHConnRequiresBinary(t *testing.T) { orig := SSHLookPath t.Cleanup(func() { SSHLookPath = orig }) 205 unmodified lines

"printf '%s' "$count" >" + shellQuote(countFile), "dest=""", "remote=""", "port=""", "if [ "$1" = "-o" ]; then", " shift 2", "fi", "if [ "$1" = "-p" ]; then", " port="$2"", " shift 2", "fi", "if [ "$1" = "--" ]; then", " shift", "fi", "if [ -n "$port" ]; then", " dest="-p $port $1"", "else", " dest="$1"",