[client] Refuse path-bearing locale, TERM and TZ values from SSH clients (#8388)

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))

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 {

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))
+		})
+	}
+}

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"
 	}