Merge pull request #480 from uselagoon/fix-shell-escape

Correctly escape complex shell commands
uselagoon · Oct 18, 2024 · 1c1557f · 1c1557f
2 parents 503c057 + 71ea3a6
commit 1c1557f
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 2 deletions.
diff --git a/go.mod b/go.mod
@@ -3,6 +3,7 @@ module github.com/uselagoon/ssh-portal
 go 1.22.2
 
 require (
+	al.essio.dev/pkg/shellescape v1.5.1
 	github.com/DATA-DOG/go-sqlmock v1.5.2
 	github.com/MicahParks/keyfunc/v2 v2.1.0
 	github.com/alecthomas/assert/v2 v2.11.0

diff --git a/go.sum b/go.sum
@@ -1,3 +1,5 @@
+al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
+al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
 filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
 filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
 github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU=
@@ -67,6 +69,8 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
 github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af h1:kmjWCqn2qkEml422C2Rrd27c3VGxi6a/6HNq8QmHRKM=
 github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
+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/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379 h1:9pvPp/2VCtCB2xdSUCaKe1VKCzVHMR+GGgIAVLfQxIs=
 github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA=

diff --git a/internal/sshserver/sessionhandler.go b/internal/sshserver/sessionhandler.go
@@ -5,9 +5,9 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
-	"strings"
 	"time"
 
+	"al.essio.dev/pkg/shellescape"
 	"github.com/gliderlabs/ssh"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -86,7 +86,7 @@ func getSSHIntent(sftp bool, cmd []string) []string {
 	// if there is a command, wrap it in a shell the way openssh does
 	// https://github.com/openssh/openssh-portable/blob/
 	// 	73dcca12115aa12ed0d123b914d473c384e52651/session.c#L1705-L1713
-	return []string{"sh", "-c", strings.Join(cmd, " ")}
+	return []string{"sh", "-c", shellescape.QuoteCommand(cmd)}
 }
 
 // sessionHandler returns a ssh.Handler which connects the ssh session to the

diff --git a/internal/sshserver/sessionhandler_test.go b/internal/sshserver/sessionhandler_test.go
@@ -39,6 +39,42 @@ func TestExec(t *testing.T) {
 			logAccessEnabled: false,
 			pty:              false,
 		},
+		"subshell": {
+			user:             "project-test",
+			deployment:       "cli",
+			rawCommand:       []string{"/bin/sh", "-c", "( echo foo; echo bar; echo baz ) | tail -n2"},
+			command:          []string{"sh", "-c", "/bin/sh -c '( echo foo; echo bar; echo baz ) | tail -n2'"},
+			sftp:             false,
+			logAccessEnabled: false,
+			pty:              false,
+		},
+		"process substitution 1": {
+			user:             "project-test",
+			deployment:       "cli",
+			rawCommand:       []string{"/bin/sh", "-c", "sleep 3 & echo $(pgrep sleep)"},
+			command:          []string{"sh", "-c", "/bin/sh -c 'sleep 3 & echo $(pgrep sleep)'"},
+			sftp:             false,
+			logAccessEnabled: false,
+			pty:              false,
+		},
+		"process substitution 2": {
+			user:             "project-test",
+			deployment:       "cli",
+			rawCommand:       []string{"/bin/sh", "-c", "sleep 3 & echo $( pgrep sleep )"},
+			command:          []string{"sh", "-c", "/bin/sh -c 'sleep 3 & echo $( pgrep sleep )'"},
+			sftp:             false,
+			logAccessEnabled: false,
+			pty:              false,
+		},
+		"shell variables": {
+			user:             "project-test",
+			deployment:       "cli",
+			rawCommand:       []string{"/bin/sh", "-c", "echo $$ $USER"},
+			command:          []string{"sh", "-c", "/bin/sh -c 'echo $$ $USER'"},
+			sftp:             false,
+			logAccessEnabled: false,
+			pty:              false,
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(tt *testing.T) {