Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v14] Quote user supplied inputs provided to scripts to avoid RCE #39838

Merged
merged 1 commit into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/utils/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"io"
stdlog "log"
"os"
"regexp"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -336,6 +337,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
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,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
Original file line number Diff line number Diff line change
Expand Up @@ -1864,11 +1864,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
29 changes: 15 additions & 14 deletions lib/web/integrations_awsoidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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 @@ -261,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", 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 @@ -300,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", 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 @@ -749,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", 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 @@ -787,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", 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 @@ -828,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", 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
Original file line number Diff line number Diff line change
Expand Up @@ -500,16 +500,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