Skip to content

Commit

Permalink
[v15] Quote user supplied inputs provided to scripts to avoid RCE (#3…
Browse files Browse the repository at this point in the history 
…9837)

* Quote user supplied inputs provided to scripts to avoid RCE

This change introduces the func `utils.UnixShellQuote` which will quote any inputs which could potentially allow execution or script escape.
This is utilized to ensure that scripts produced from a potential Phishing link could not contain code execution which may expose a user.

* awsAccessGraphOIDCSync: Ensure role parameter is quoted correctly

* join_tokens: Move shell quote to `getJoinScript` rather than where parameters are extracted

This will increase safety moving forward, but it requires a more conservative quoting strategy.
  • Loading branch information
@jentfoo
jentfoo committed Mar 26, 2024
1 parent c7c4ede commit 89a4da5
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 24 deletions.
19 changes: 19 additions & 0 deletions lib/utils/cli.go
View file
Expand Up @@ -29,6 +29,7 @@ import (
stdlog "log"
"log/slog"
"os"
"regexp"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -419,6 +420,24 @@ 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:
Expand Down
137 changes: 137 additions & 0 deletions lib/utils/cli_test.go
View file
Expand Up @@ -68,6 +68,143 @@ 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()
Expand Down
6 changes: 3 additions & 3 deletions lib/web/apiserver.go
View file
Expand Up @@ -1907,11 +1907,11 @@ func (h *Handler) installer(w http.ResponseWriter, r *http.Request, p httprouter

tmpl := installers.Template{
PublicProxyAddr: h.PublicProxyAddr(),
MajorVersion: version,
MajorVersion: utils.UnixShellQuote(version),
TeleportPackage: teleportPackage,
RepoChannel: repoChannel,
RepoChannel: utils.UnixShellQuote(repoChannel),
AutomaticUpgrades: strconv.FormatBool(installUpdater),
AzureClientID: azureClientID,
AzureClientID: utils.UnixShellQuote(azureClientID),
}
err = instTmpl.Execute(w, tmpl)
return nil, trace.Wrap(err)
Expand Down
33 changes: 17 additions & 16 deletions lib/web/integrations_awsoidc.go
View file
Expand Up @@ -45,6 +45,7 @@ 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"
)
Expand Down Expand Up @@ -266,11 +267,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", clusterName),
fmt.Sprintf("--name=%s", integrationName),
fmt.Sprintf("--aws-region=%s", awsRegion),
fmt.Sprintf("--role=%s", role),
fmt.Sprintf("--task-role=%s", taskRole),
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)),
}
script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{
TeleportArgs: strings.Join(argsList, " "),
Expand Down Expand Up @@ -305,8 +306,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", awsRegion),
fmt.Sprintf("--role=%s", role),
fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)),
fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)),
}
script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{
TeleportArgs: strings.Join(argsList, " "),
Expand Down Expand Up @@ -340,8 +341,8 @@ func (h *Handler) awsOIDCConfigureEKSIAM(w http.ResponseWriter, r *http.Request,
// "teleport integration configure eks-iam"
argsList := []string{
"integration", "configure", "eks-iam",
fmt.Sprintf("--aws-region=%s", awsRegion),
fmt.Sprintf("--role=%s", role),
fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)),
fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)),
}
script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{
TeleportArgs: strings.Join(argsList, " "),
Expand Down Expand Up @@ -879,10 +880,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", clusterName),
fmt.Sprintf("--name=%s", integrationName),
fmt.Sprintf("--role=%s", role),
fmt.Sprintf("--s3-bucket-uri=%s", s3URI.String()),
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("--s3-jwks-base64=%s", base64.StdEncoding.EncodeToString(jwksJSON)),
}
script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{
Expand Down Expand Up @@ -917,8 +918,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", awsRegion),
fmt.Sprintf("--role=%s", role),
fmt.Sprintf("--aws-region=%s", libutils.UnixShellQuote(awsRegion)),
fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)),
}
script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{
TeleportArgs: strings.Join(argsList, " "),
Expand Down Expand Up @@ -958,7 +959,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", role),
fmt.Sprintf("--role=%s", libutils.UnixShellQuote(role)),
}
script, err := oneoff.BuildScript(oneoff.OneOffScriptParams{
TeleportArgs: strings.Join(argsList, " "),
Expand Down
10 changes: 5 additions & 5 deletions lib/web/join_tokens.go
View file
Expand Up @@ -510,16 +510,16 @@ func getJoinScript(ctx context.Context, settings scriptSettings, m nodeAPIGetter
"packageName": packageName,
"repoChannel": repoChannel,
"installUpdater": strconv.FormatBool(settings.installUpdater),
"version": version,
"version": utils.UnixShellQuote(version),
"appInstallMode": strconv.FormatBool(settings.appInstallMode),
"appName": settings.appName,
"appURI": settings.appURI,
"joinMethod": settings.joinMethod,
"appName": utils.UnixShellQuote(settings.appName),
"appURI": utils.UnixShellQuote(settings.appURI),
"joinMethod": utils.UnixShellQuote(settings.joinMethod),
"labels": strings.Join(labelsList, ","),
"databaseInstallMode": strconv.FormatBool(settings.databaseInstallMode),
"db_service_resource_labels": dbServiceResourceLabels,
"discoveryInstallMode": settings.discoveryInstallMode,
"discoveryGroup": settings.discoveryGroup,
"discoveryGroup": utils.UnixShellQuote(settings.discoveryGroup),
})
if err != nil {
return "", trace.Wrap(err)
Expand Down

0 comments on commit 89a4da5

Please sign in to comment.