diff --git a/client/ssh/server/command_execution_unix.go b/client/ssh/server/command_execution_unix.go index 279b89341..0c3537bd3 100644 --- a/client/ssh/server/command_execution_unix.go +++ b/client/ssh/server/command_execution_unix.go @@ -159,7 +159,7 @@ func (s *Server) prepareCommandEnv(_ *log.Entry, localUser *user.User, session s // executeCommandWithPty executes a command with PTY allocation func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, execCmd *exec.Cmd, privilegeResult PrivilegeCheckResult, ptyReq ssh.Pty, winCh <-chan ssh.Window) bool { termType := ptyReq.Term - if termType == "" { + if termType == "" || !acceptEnv("TERM="+termType) { termType = "xterm-256color" } execCmd.Env = append(execCmd.Env, fmt.Sprintf("TERM=%s", termType)) diff --git a/client/ssh/server/shell.go b/client/ssh/server/shell.go index 7b356b2a0..5b8f0dfed 100644 --- a/client/ssh/server/shell.go +++ b/client/ssh/server/shell.go @@ -124,12 +124,11 @@ func prepareUserEnv(user *user.User, shell string) []string { } // acceptEnv checks if environment variable from SSH client should be accepted -// This is a whitelist of variables that SSH clients can send to the server +// This is a whitelist of variables that SSH clients can send to the server. +// Accepted variables end up in the environment of login, su and the executor, +// which start with the daemon's privileges, so the value is checked as well as the name. func acceptEnv(envVar string) bool { - varName := envVar - if idx := strings.Index(envVar, "="); idx != -1 { - varName = envVar[:idx] - } + varName, value, _ := strings.Cut(envVar, "=") exactMatches := []string{ "LANG", @@ -150,19 +149,40 @@ func acceptEnv(envVar string) bool { for _, exact := range exactMatches { if varName == exact { - return true + return acceptEnvValue(varName, value) } } for _, prefix := range prefixMatches { if strings.HasPrefix(varName, prefix) { - return true + return acceptEnvValue(varName, value) } } return false } +// acceptEnvValue rejects values that make libc open a file of the client's choosing +// in a process that has not dropped privileges yet. Locale names are path elements of +// the gettext message catalogs (which supply format strings to login and su), terminal +// names of the terminfo entry and TZ names the time zone file. libc only guards against +// this in setuid programs, not in programs started by root, so the same rules sudo +// applies to these variables are enforced here. +func acceptEnvValue(varName, value string) bool { + switch { + case varName == "TZ": + zone := strings.TrimPrefix(value, ":") + return !strings.HasPrefix(zone, "/") && !strings.Contains(zone, "..") + case varName == "LANG", varName == "LANGUAGE", varName == "TERM", varName == "COLORTERM", + strings.HasPrefix(varName, "LC_"): + // ".." is a path component on its own: it needs no neighboring "/" to traverse + // once concatenated into a locale, terminfo or message-catalog path. + return !strings.ContainsAny(value, "/%") && !strings.Contains(value, "..") + default: + return true + } +} + // prepareSSHEnv prepares SSH protocol-specific environment variables // These variables provide information about the SSH connection itself func prepareSSHEnv(session ssh.Session) []string { diff --git a/client/ssh/server/shell_test.go b/client/ssh/server/shell_test.go new file mode 100644 index 000000000..9a0cb2ab9 --- /dev/null +++ b/client/ssh/server/shell_test.go @@ -0,0 +1,56 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAcceptEnv(t *testing.T) { + tests := []struct { + envVar string + want bool + }{ + {"LANG=en_US.UTF-8", true}, + {"LANGUAGE=de:en", true}, + {"LC_ALL=C.UTF-8", true}, + {"LC_MESSAGES=de_DE.UTF-8@euro", true}, + {"TERM=xterm-256color", true}, + {"COLORTERM=truecolor", true}, + {"TZ=Europe/Berlin", true}, + {"TZ=:Europe/Berlin", true}, + {"TZ=CET-1CEST,M3.5.0,M10.5.0/3", true}, + {"EDITOR=/usr/bin/vim", true}, + {"LANG", true}, + + {"LD_PRELOAD=/tmp/x.so", false}, + {"PATH=/tmp", false}, + {"=LANG", false}, + + // values naming files must not reach login/su, which run as root + {"LANGUAGE=../../../../tmp/catalogs", false}, + {"LANGUAGE=de:../../../../tmp/catalogs", false}, + {"LANG=/tmp/locale", false}, + {"LC_MESSAGES=../x", false}, + {"LC_ALL=%n", false}, + {"TERM=../../tmp/terminfo", false}, + {"COLORTERM=/tmp/x", false}, + {"TZ=/etc/shadow", false}, + {"TZ=:/etc/shadow", false}, + {"TZ=../../../etc/shadow", false}, + {"TZ=Europe/../../../../etc/shadow", false}, + + // a bare ".." is a path component on its own, with no "/" needed to traverse + {"LANG=..", false}, + {"LANGUAGE=..", false}, + {"TERM=..", false}, + {"COLORTERM=..", false}, + {"LC_ALL=..", false}, + } + + for _, tt := range tests { + t.Run(tt.envVar, func(t *testing.T) { + assert.Equal(t, tt.want, acceptEnv(tt.envVar)) + }) + } +} diff --git a/client/ssh/server/userswitching_unix.go b/client/ssh/server/userswitching_unix.go index ae60ec64c..66e0b9624 100644 --- a/client/ssh/server/userswitching_unix.go +++ b/client/ssh/server/userswitching_unix.go @@ -243,7 +243,7 @@ func (s *Server) createDirectPtyCommand(session ssh.Session, localUser *user.Use // preparePtyEnv prepares environment variables for Pty execution func (s *Server) preparePtyEnv(localUser *user.User, ptyReq ssh.Pty, session ssh.Session) []string { termType := ptyReq.Term - if termType == "" { + if termType == "" || !acceptEnv("TERM="+termType) { termType = "xterm-256color" }