ssh/tailssh: reject dangerous LD_/DYLD_ env vars in acceptEnv filtering (#19914)

Block dynamic linker environment variables (LD_PRELOAD, LD_LIBRARY_PATH,
DYLD_INSERT_LIBRARIES, and friends) from being forwarded regardless of
acceptEnv policy, preventing privilege escalation via wildcard patterns
like "*".

We are not aware of any legitimate use of these variables so they are
safe to exclude from being passed.

Thanks to Tim Sageser (dtrsecurity) for this report.

Updates tailscale/corp#42033

Signed-off-by: Patrick O'Doherty <patrick@tailscale.com>

ssh/tailssh/accept_env.go

@@ -9,6 +9,17 @@ import (
 	"strings"
 )
 
+// isDangerousEnvVar reports whether the given environment variable name
+// is unconditionally prohibited from being forwarded, regardless of
+// acceptEnv policy. This prevents privilege escalation via dynamic
+// linker environment variables (e.g. LD_PRELOAD, LD_LIBRARY_PATH,
+// DYLD_INSERT_LIBRARIES) even when a wildcard acceptEnv pattern like
+// "*" is configured.
+func isDangerousEnvVar(name string) bool {
+	upper := strings.ToUpper(name)
+	return strings.HasPrefix(upper, "LD_") || strings.HasPrefix(upper, "DYLD_")
+}
+
 // filterEnv filters a passed in environ string slice (a slice with strings
 // representing environment variables in the form "key=value") based on
 // the supplied slice of acceptEnv values.
@@ -18,6 +29,10 @@ import (
 //
 // acceptEnv values may contain * and ? wildcard characters which match against
 // zero or one or more characters and a single character respectively.
+//
+// Certain dangerous environment variables (such as those controlling the
+// dynamic linker) are always rejected regardless of the acceptEnv policy.
+// See isDangerousEnvVar.
 func filterEnv(acceptEnv []string, environ []string) ([]string, error) {
 	var acceptedPairs []string
 
@@ -32,6 +47,12 @@ func filterEnv(acceptEnv []string, environ []string) ([]string, error) {
 			return nil, fmt.Errorf(`invalid environment variable: %q. Variables must be in "KEY=VALUE" format`, envPair)
 		}
 
+		// Always reject dangerous environment variables that could
+		// enable privilege escalation, regardless of acceptEnv policy.
+		if isDangerousEnvVar(variableName) {
+			continue
+		}
+
 		// Short circuit if we have a direct match between the environment
 		// variable and an AcceptEnv value.
 		if slices.Contains(acceptEnv, variableName) {

ssh/tailssh/accept_env_test.go

@@ -10,6 +10,39 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
+func TestIsDangerousEnvVar(t *testing.T) {
+	tests := []struct {
+		name      string
+		dangerous bool
+	}{
+		{"LD_PRELOAD", true},
+		{"LD_LIBRARY_PATH", true},
+		{"LD_AUDIT", true},
+		{"LD_DEBUG", true},
+		{"LD_PROFILE", true},
+		{"ld_preload", true},
+		{"DYLD_INSERT_LIBRARIES", true},
+		{"DYLD_LIBRARY_PATH", true},
+		{"DYLD_FRAMEWORK_PATH", true},
+		{"dyld_insert_libraries", true},
+		{"TERM", false},
+		{"LANG", false},
+		{"LC_ALL", false},
+		{"PATH", false},
+		{"HOME", false},
+		{"LDFLAGS", false},
+		{"MY_LD_PRELOAD", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isDangerousEnvVar(tt.name); got != tt.dangerous {
+				t.Errorf("isDangerousEnvVar(%q) = %v, want %v", tt.name, got, tt.dangerous)
+			}
+		})
+	}
+}
+
 func TestMatchAcceptEnvPattern(t *testing.T) {
 	testCases := []struct {
 		pattern string
@@ -135,6 +168,42 @@ func TestFilterEnv(t *testing.T) {
 			expectedFiltered: nil,
 			wantErrMessage:   `invalid environment variable: "FOOBAR". Variables must be in "KEY=VALUE" format`,
 		},
+		{
+			name:             "ld-preload-rejected-with-wildcard",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"LD_PRELOAD=/tmp/evil.so", "TERM=xterm"},
+			expectedFiltered: []string{"TERM=xterm"},
+		},
+		{
+			name:             "ld-vars-rejected-with-wildcard",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"LD_PRELOAD=/tmp/evil.so", "LD_LIBRARY_PATH=/tmp", "LD_AUDIT=/tmp/audit.so", "SAFE_VAR=ok"},
+			expectedFiltered: []string{"SAFE_VAR=ok"},
+		},
+		{
+			name:             "ld-vars-rejected-with-explicit-match",
+			acceptEnv:        []string{"LD_PRELOAD", "LD_LIBRARY_PATH"},
+			environ:          []string{"LD_PRELOAD=/tmp/evil.so", "LD_LIBRARY_PATH=/tmp"},
+			expectedFiltered: nil,
+		},
+		{
+			name:             "ld-vars-rejected-with-prefix-pattern",
+			acceptEnv:        []string{"LD_*"},
+			environ:          []string{"LD_PRELOAD=/tmp/evil.so", "LD_LIBRARY_PATH=/tmp"},
+			expectedFiltered: nil,
+		},
+		{
+			name:             "ld-vars-case-insensitive",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"ld_preload=/tmp/evil.so", "Ld_Library_Path=/tmp", "SAFE=ok"},
+			expectedFiltered: []string{"SAFE=ok"},
+		},
+		{
+			name:             "dyld-vars-rejected",
+			acceptEnv:        []string{"*"},
+			environ:          []string{"DYLD_INSERT_LIBRARIES=/tmp/evil.dylib", "DYLD_LIBRARY_PATH=/tmp", "TERM=xterm"},
+			expectedFiltered: []string{"TERM=xterm"},
+		},
 	}
 
 	for _, tc := range testCases {