From 0b7be1175dced8a1c7b8667aaed36034082f94c9 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 14 Feb 2020 16:17:32 +0100 Subject: [PATCH] parse ssh commands with shlex instead of use our bugged home made method. Fixes #72 --- README.md | 1 + go.mod | 1 + go.sum | 2 ++ sftpd/internal_test.go | 8 ++++++-- sftpd/ssh_cmd.go | 27 +++++++++++++-------------- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 67233b50..74d1eb30 100644 --- a/README.md +++ b/README.md @@ -811,6 +811,7 @@ The **connection failed logs** can be used for integration in tools such as [Fai - [PipeAt](https://github.com/eikenb/pipeat) - [ZeroConf](https://github.com/grandcat/zeroconf) - [SB Admin 2](https://github.com/BlackrockDigital/startbootstrap-sb-admin-2) +- [shlex](https://github.com/google/shlex) Some code was initially taken from [Pterodactyl sftp server](https://github.com/pterodactyl/sftp-server) diff --git a/go.mod b/go.mod index 729c251f..1c7d75f8 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/go-sql-driver/mysql v1.5.0 github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect github.com/golang/protobuf v1.3.3 // indirect + github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/grandcat/zeroconf v1.0.0 github.com/lib/pq v1.3.0 github.com/mattn/go-sqlite3 v2.0.3+incompatible diff --git a/go.sum b/go.sum index bea2d164..e802f38e 100644 --- a/go.sum +++ b/go.sum @@ -103,6 +103,8 @@ github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OI github.com/google/pprof v0.0.0-20190515194954-54271f7e092f/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= github.com/google/pprof v0.0.0-20191218002539-d4f498aebedc/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+Tv3SM= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index 4263fcf6..0087b1a3 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -606,7 +606,7 @@ func TestSSHCommandPath(t *testing.T) { } func TestSSHParseCommandPayload(t *testing.T) { - cmd := "command -a -f some\\ spaces\\ \\ .txt" + cmd := "command -a -f /ab\\ à/some\\ spaces\\ \\ \\(\\).txt" name, args, _ := parseCommandPayload(cmd) if name != "command" { t.Errorf("unexpected command: %v", name) @@ -614,9 +614,13 @@ func TestSSHParseCommandPayload(t *testing.T) { if len(args) != 3 { t.Errorf("unexpected number of arguments %v/3", len(args)) } - if !utils.IsStringInSlice("some spaces .txt", args) { + if args[2] != "/ab à/some spaces ().txt" { t.Errorf("command parsing error, expected arguments not found: %v", args) } + _, _, err := parseCommandPayload("") + if err == nil { + t.Error("parsing invalid command must fail") + } } func TestSSHCommandErrors(t *testing.T) { diff --git a/sftpd/ssh_cmd.go b/sftpd/ssh_cmd.go index 036ed905..f4b2e664 100644 --- a/sftpd/ssh_cmd.go +++ b/sftpd/ssh_cmd.go @@ -22,6 +22,7 @@ import ( "github.com/drakkan/sftpgo/metrics" "github.com/drakkan/sftpgo/utils" "github.com/drakkan/sftpgo/vfs" + "github.com/google/shlex" "golang.org/x/crypto/ssh" ) @@ -45,11 +46,11 @@ type systemCommand struct { func processSSHCommand(payload []byte, connection *Connection, channel ssh.Channel, enabledSSHCommands []string) bool { var msg sshSubsystemExecMsg if err := ssh.Unmarshal(payload, &msg); err == nil { - name, args, err := parseCommandPayload(strings.TrimSpace(msg.Command)) - connection.Log(logger.LevelDebug, logSenderSSH, "new ssh command: %#v args: %v user: %v, error: %v", - name, args, connection.User.Username, err) + name, args, err := parseCommandPayload(msg.Command) + connection.Log(logger.LevelDebug, logSenderSSH, "new ssh command: %#v args: %v num args: %v user: %v, error: %v", + name, args, len(args), connection.User.Username, err) if err == nil && utils.IsStringInSlice(name, enabledSSHCommands) { - connection.command = fmt.Sprintf("%v %v", name, strings.Join(args, " ")) + connection.command = msg.Command if name == "scp" && len(args) >= 2 { connection.protocol = protocolSCP connection.channel = channel @@ -421,17 +422,15 @@ func computeHashForFile(hasher hash.Hash, path string) (string, error) { } func parseCommandPayload(command string) (string, []string, error) { - parts := strings.Split(strings.ReplaceAll(command, "\\ ", "\\"), " ") + parts, err := shlex.Split(command) + if err == nil && len(parts) == 0 { + err = fmt.Errorf("invalid command: %#v", command) + } + if err != nil { + return "", []string{}, err + } if len(parts) < 2 { return parts[0], []string{}, nil } - args := []string{} - for _, arg := range parts[1:] { - parsed := strings.TrimSpace(strings.ReplaceAll(arg, "\\", " ")) - if len(parsed) == 0 { - continue - } - args = append(args, parsed) - } - return parts[0], args, nil + return parts[0], parts[1:], nil }