diff --git a/go.mod b/go.mod index 837a84c9282d2..a0bbef4ecfd84 100644 --- a/go.mod +++ b/go.mod @@ -93,6 +93,7 @@ require ( github.com/google/go-cmp v0.6.0 github.com/google/go-querystring v1.1.0 github.com/google/go-tpm-tools v0.4.0 + github.com/google/safetext v0.0.0-20240104143208-7a7d9b3d812f github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.6.0 github.com/googleapis/gax-go/v2 v2.12.0 @@ -483,6 +484,7 @@ require ( k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect k8s.io/metrics v0.29.0 // indirect k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect + mvdan.cc/sh/v3 v3.7.0 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/kustomize/api v0.13.5-0.20230601165947-6ce0bf390ce3 // indirect sigs.k8s.io/kustomize/kustomize/v5 v5.0.4-0.20230601165947-6ce0bf390ce3 // indirect diff --git a/go.sum b/go.sum index 52f3366b26bff..e37c7b4cf99dd 100644 --- a/go.sum +++ b/go.sum @@ -474,8 +474,8 @@ github.com/fortytw2/leaktest v1.3.0 h1:u8491cBMTQ8ft8aeV+adlcytMZylmA5nnwwkRZjI8 github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g= github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4= github.com/franela/goreq v0.0.0-20171204163338-bcd34c9993f8/go.mod h1:ZhphrRTfi2rbfLwlschooIH4+wKKDR4Pdxhh+TRoA20= -github.com/frankban/quicktest v1.14.4 h1:g2rn0vABPOOXmZUj+vbmUp0lPoXEMuhTpIluN0XL9UY= -github.com/frankban/quicktest v1.14.4/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= +github.com/frankban/quicktest v1.14.5 h1:dfYrrRyLtiqT9GyKXgdh+k4inNeTvmGbuSgZ3lx3GhA= +github.com/frankban/quicktest v1.14.5/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.5.4/go.mod h1:OVB6XrOHzAwXMpEM7uPOzcehqUV2UqJxmVXmkdnm1bU= @@ -761,6 +761,8 @@ github.com/google/renameio/v2 v2.0.0 h1:UifI23ZTGY8Tt29JbYFiuyIU3eX+RNFtUwefq9qA github.com/google/renameio/v2 v2.0.0/go.mod h1:BtmJXm5YlszgC+TD4HOEEUFgkJP3nLxehU6hfe7jRt4= github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o= github.com/google/s2a-go v0.1.7/go.mod h1:50CgR4k1jNlWBu4UfS4AcfhVe1r6pdZPygJ3R8F0Qdw= +github.com/google/safetext v0.0.0-20240104143208-7a7d9b3d812f h1:o2yGZLlsOj5H5uvtQNEdi6DeA0GbUP3lm0gWW5RvY0s= +github.com/google/safetext v0.0.0-20240104143208-7a7d9b3d812f/go.mod h1:H3K1Iu/utuCfa10JO+GsmKUYSWi7ug57Rk6GaDRHaaQ= 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/tink/go v1.7.0 h1:6Eox8zONGebBFcCBqkVmt60LaWZa6xg1cl/DwAh/J1w= @@ -2277,6 +2279,8 @@ k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSn k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= launchpad.net/gocheck v0.0.0-20140225173054-000000000087 h1:Izowp2XBH6Ya6rv+hqbceQyw/gSGoXfH/UPoTGduL54= launchpad.net/gocheck v0.0.0-20140225173054-000000000087/go.mod h1:hj7XX3B/0A+80Vse0e+BUHsHMTEhd0O4cpUHr/e/BUM= +mvdan.cc/sh/v3 v3.7.0 h1:lSTjdP/1xsddtaKfGg7Myu7DnlHItd3/M2tomOcNNBg= +mvdan.cc/sh/v3 v3.7.0/go.mod h1:K2gwkaesF/D7av7Kxl0HbF5kGOd2ArupNTX3X44+8l8= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= diff --git a/lib/utils/cli.go b/lib/utils/cli.go index c4b873fa406e5..e768367ec6cf1 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -25,7 +25,6 @@ import ( "io" stdlog "log" "os" - "regexp" "runtime" "strconv" "strings" @@ -337,24 +336,6 @@ func SplitIdentifiers(s string) []string { }) } -var unixShellQuoteCharacters = regexp.MustCompile( - "[^" + // Match any character that is NOT one of the following: - "\\w" + // Word characters (letter, number, underscore) - "@%+=:,./-" + // Safe symbols that don't typically have a special meaning in shells - "]") - -// UnixShellQuote returns the string in quotes if quoting is necessary to prevent possible execution or injection for -// UNIX-like systems. This is intended to be used when building shell scripts for Linux or macOS. -func UnixShellQuote(s string) string { - if unixShellQuoteCharacters.MatchString(s) { - s = strings.ReplaceAll(s, "\n", "\\n") - s = strings.ReplaceAll(s, "\r", "\\r") - return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'" - } - - return s -} - // EscapeControl escapes all ANSI escape sequences from string and returns a // string that is safe to print on the CLI. This is to ensure that malicious // servers can not hide output. For more details, see: diff --git a/lib/utils/cli_test.go b/lib/utils/cli_test.go index 60c978d3bfc6b..7bc2fa212723f 100644 --- a/lib/utils/cli_test.go +++ b/lib/utils/cli_test.go @@ -62,143 +62,6 @@ func TestUserMessageFromError(t *testing.T) { } } -func TestUnixShellQuote(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - in string - out string - }{ - { - name: "emptyString", - in: "", - out: "", - }, - { - name: "noQuote", - in: "foo", - out: "foo", - }, - { - name: "bang", - in: "foo!", - out: "'foo!'", - }, - { - name: "variable", - in: "foo$BAR", - out: "'foo$BAR'", - }, - { - name: "semicolon", - in: "foo;bar", - out: "'foo;bar'", - }, - { - name: "singleQuoteStart", - in: "'foo", - out: "''\"'\"'foo'", - }, - { - name: "singleQuoteMid", - in: "foo'bar", - out: "'foo'\"'\"'bar'", - }, - { - name: "singleQuoteEnd", - in: "foo'", - out: "'foo'\"'\"''", - }, - { - name: "singleQuotesSurrounding", - in: "'foo'", - out: "''\"'\"'foo'\"'\"''", - }, - { - name: "space", - in: "foo bar", - out: "'foo bar'", - }, - { - name: "path", - in: "/usr/local/bin", - out: "/usr/local/bin", - }, - { - name: "commandSubstitution", - in: "$(ls -la)", - out: "'$(ls -la)'", - }, - { - name: "backticks", - in: "`echo foo`", - out: "'`echo foo`'", - }, - { - name: "doubleQuotes", - in: "foo\"bar", - out: "'foo\"bar'", - }, - { - name: "brackets", - in: "[1,2,3]", - out: "'[1,2,3]'", - }, - { - name: "parentheses", - in: "(1+2)", - out: "'(1+2)'", - }, - { - name: "braceExpansion", - in: "{a,b}", - out: "'{a,b}'", - }, - { - name: "escapeCharacters", - in: "foo\\bar", - out: "'foo\\bar'", - }, - { - name: "wildcards", - in: "*", - out: "'*'", - }, - { - name: "pipe", - in: "foo | bar", - out: "'foo | bar'", - }, - { - name: "andOperator", - in: "foo && bar", - out: "'foo && bar'", - }, - { - name: "newline", - in: "foo\nbar", - out: "'foo\\nbar'", - }, - { - name: "carriageReturn", - in: "foo\rbar", - out: "'foo\\rbar'", - }, - { - name: "tab", - in: "foo\tbar", - out: "'foo\tbar'", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.out, UnixShellQuote(tt.in)) - }) - } -} - // TestEscapeControl tests escape control func TestEscapeControl(t *testing.T) { t.Parallel() diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 8fe0a1ef357c5..62a74a068f11b 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -37,6 +37,7 @@ import ( "sync/atomic" "time" + "github.com/google/safetext/shsprintf" "github.com/google/uuid" "github.com/gorilla/websocket" "github.com/gravitational/oxy/ratelimit" @@ -1864,11 +1865,11 @@ func (h *Handler) installer(w http.ResponseWriter, r *http.Request, p httprouter tmpl := installers.Template{ PublicProxyAddr: h.PublicProxyAddr(), - MajorVersion: utils.UnixShellQuote(version), + MajorVersion: shsprintf.EscapeDefaultContext(version), TeleportPackage: teleportPackage, - RepoChannel: utils.UnixShellQuote(repoChannel), + RepoChannel: shsprintf.EscapeDefaultContext(repoChannel), AutomaticUpgrades: strconv.FormatBool(installUpdater), - AzureClientID: utils.UnixShellQuote(azureClientID), + AzureClientID: shsprintf.EscapeDefaultContext(azureClientID), } err = instTmpl.Execute(w, tmpl) return nil, trace.Wrap(err) diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index 9f9428b789306..adea2d817c3e7 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -23,6 +23,7 @@ import ( "slices" "strings" + "github.com/google/safetext/shsprintf" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -40,7 +41,6 @@ import ( "github.com/gravitational/teleport/lib/integrations/awsoidc/deployserviceconfig" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/services" - libutils "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/web/scripts/oneoff" "github.com/gravitational/teleport/lib/web/ui" ) @@ -262,11 +262,11 @@ func (h *Handler) awsOIDCConfigureDeployServiceIAM(w http.ResponseWriter, r *htt // teleport integration configure deployservice-iam argsList := []string{ "integration", "configure", "deployservice-iam", - fmt.Sprintf("--cluster=%s", libutils.UnixShellQuote(clusterName)), - fmt.Sprintf("--name=%s", libutils.UnixShellQuote(integrationName)), - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), - fmt.Sprintf("--task-role=%s", libutils.UnixShellQuote(taskRole)), + fmt.Sprintf("--cluster=%s", shsprintf.EscapeDefaultContext(clusterName)), + fmt.Sprintf("--name=%s", shsprintf.EscapeDefaultContext(integrationName)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), + fmt.Sprintf("--task-role=%s", shsprintf.EscapeDefaultContext(taskRole)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -301,8 +301,8 @@ func (h *Handler) awsOIDCConfigureEICEIAM(w http.ResponseWriter, r *http.Request // teleport integration configure eice-iam argsList := []string{ "integration", "configure", "eice-iam", - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -750,10 +750,10 @@ func (h *Handler) awsOIDCConfigureIdP(w http.ResponseWriter, r *http.Request, p // teleport integration configure awsoidc-idp argsList := []string{ "integration", "configure", "awsoidc-idp", - fmt.Sprintf("--cluster=%s", libutils.UnixShellQuote(clusterName)), - fmt.Sprintf("--name=%s", libutils.UnixShellQuote(integrationName)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), - fmt.Sprintf("--s3-bucket-uri=%s", libutils.UnixShellQuote(s3URI.String())), + fmt.Sprintf("--cluster=%s", shsprintf.EscapeDefaultContext(clusterName)), + fmt.Sprintf("--name=%s", shsprintf.EscapeDefaultContext(integrationName)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), + fmt.Sprintf("--s3-bucket-uri=%s", shsprintf.EscapeDefaultContext(s3URI.String())), fmt.Sprintf("--s3-jwks-base64=%s", base64.StdEncoding.EncodeToString(jwksJSON)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ @@ -788,8 +788,8 @@ func (h *Handler) awsOIDCConfigureListDatabasesIAM(w http.ResponseWriter, r *htt // teleport integration configure listdatabases-iam argsList := []string{ "integration", "configure", "listdatabases-iam", - fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)), - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--aws-region=%s", shsprintf.EscapeDefaultContext(awsRegion)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), @@ -829,7 +829,7 @@ func (h *Handler) awsAccessGraphOIDCSync(w http.ResponseWriter, r *http.Request, // "teleport integration configure access-graph aws-iam" argsList := []string{ "integration", "configure", "access-graph", "aws-iam", - fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)), + fmt.Sprintf("--role=%s", shsprintf.EscapeDefaultContext(role)), } script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{ TeleportArgs: strings.Join(argsList, " "), diff --git a/lib/web/integrations_awsoidc_test.go b/lib/web/integrations_awsoidc_test.go index 890db07fa9ebc..eafeb8a123fb1 100644 --- a/lib/web/integrations_awsoidc_test.go +++ b/lib/web/integrations_awsoidc_test.go @@ -86,7 +86,7 @@ func TestBuildDeployServiceConfigureIAMScript(t *testing.T) { `--cluster=localhost ` + `--name=myintegration ` + `--aws-region=us-east-1 ` + - `--role=Test+1=2,3.4@5-6_7 ` + + `--role=Test\+1=2,3.4\@5-6_7 ` + `--task-role=taskRole`, }, { @@ -199,7 +199,7 @@ func TestBuildEICEConfigureIAMScript(t *testing.T) { errCheck: require.NoError, expectedTeleportArgs: "integration configure eice-iam " + "--aws-region=us-east-1 " + - "--role=Test+1=2,3.4@5-6_7", + "--role=Test\\+1=2,3.4\\@5-6_7", }, { name: "missing aws-region", @@ -302,7 +302,7 @@ func TestBuildAWSOIDCIdPConfigureScript(t *testing.T) { expectedTeleportArgs: "integration configure awsoidc-idp " + "--cluster=localhost " + "--name=myintegration " + - "--role=Test+1=2,3.4@5-6_7 " + + "--role=Test\\+1=2,3.4\\@5-6_7 " + `--s3-bucket-uri=s3://my-bucket/prefix ` + "--s3-jwks-base64=" + jwksBase64, }, @@ -418,7 +418,7 @@ func TestBuildListDatabasesConfigureIAMScript(t *testing.T) { errCheck: require.NoError, expectedTeleportArgs: "integration configure listdatabases-iam " + "--aws-region=us-east-1 " + - "--role=Test+1=2,3.4@5-6_7", + "--role=Test\\+1=2,3.4\\@5-6_7", }, { name: "missing aws-region", diff --git a/lib/web/join_tokens.go b/lib/web/join_tokens.go index 6eae2795253e2..26d195a2c2c66 100644 --- a/lib/web/join_tokens.go +++ b/lib/web/join_tokens.go @@ -31,6 +31,7 @@ import ( "strings" "time" + "github.com/google/safetext/shsprintf" "github.com/google/uuid" "github.com/gravitational/trace" "github.com/julienschmidt/httprouter" @@ -500,16 +501,16 @@ func getJoinScript(ctx context.Context, settings scriptSettings, m nodeAPIGetter "packageName": packageName, "repoChannel": repoChannel, "installUpdater": strconv.FormatBool(settings.installUpdater), - "version": utils.UnixShellQuote(version), + "version": shsprintf.EscapeDefaultContext(version), "appInstallMode": strconv.FormatBool(settings.appInstallMode), - "appName": utils.UnixShellQuote(settings.appName), - "appURI": utils.UnixShellQuote(settings.appURI), - "joinMethod": utils.UnixShellQuote(settings.joinMethod), + "appName": shsprintf.EscapeDefaultContext(settings.appName), + "appURI": shsprintf.EscapeDefaultContext(settings.appURI), + "joinMethod": shsprintf.EscapeDefaultContext(settings.joinMethod), "labels": strings.Join(labelsList, ","), "databaseInstallMode": strconv.FormatBool(settings.databaseInstallMode), "db_service_resource_labels": dbServiceResourceLabels, "discoveryInstallMode": settings.discoveryInstallMode, - "discoveryGroup": utils.UnixShellQuote(settings.discoveryGroup), + "discoveryGroup": shsprintf.EscapeDefaultContext(settings.discoveryGroup), }) if err != nil { return "", trace.Wrap(err)