From e4144230f410204aa2f43a07a57182c751239597 Mon Sep 17 00:00:00 2001 From: Mario Minardi Date: Mon, 13 Jul 2026 11:57:02 -0600 Subject: [PATCH] util/osuser: reject leading dashes in usernames Reject leading dashes in usernames and add double dash to getent call on linux to prevent values sent as usernames being interpreted as command options. Fixes https://github.com/tailscale/corp/issues/44813 Signed-off-by: Mario Minardi --- util/osuser/user.go | 83 ++++++++++++++++++++++++++++--- util/osuser/user_test.go | 103 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 7 deletions(-) create mode 100644 util/osuser/user_test.go diff --git a/util/osuser/user.go b/util/osuser/user.go index 2de3da762..3fdbfbbd3 100644 --- a/util/osuser/user.go +++ b/util/osuser/user.go @@ -8,11 +8,14 @@ package osuser import ( "context" "errors" + "fmt" "log" "os/exec" "os/user" + "reflect" "runtime" "strings" + "sync" "time" "unicode/utf8" @@ -50,6 +53,12 @@ func LookupByUsername(username string) (*user.User, error) { // lookupStd is either user.Lookup or user.LookupId. type lookupStd func(string) (*user.User, error) +var execGetent = func(ctx context.Context, args ...string) ([]byte, error) { + return exec.CommandContext(ctx, "getent", args...).Output() +} + +var getentDoubleDashSupported = sync.OnceValue(probeGetentDoubleDashSupport) + func lookup(usernameOrUID string, std lookupStd, wantShell bool) (*user.User, string, error) { // Skip getent entirely on Non-Unix platforms that won't ever have it. // (Using HasPrefix for "wasip1", anticipating that WASI support will @@ -118,6 +127,12 @@ func checkGetentInput(usernameOrUID string) bool { if len(usernameOrUID) > maxUid || len(usernameOrUID) == 0 { return false } + + // Leading dashes aren't valid for usernames. + if strings.HasPrefix(usernameOrUID, "-") { + return false + } + for _, r := range usernameOrUID { if r < ' ' || r == 0x7f || r == utf8.RuneError { // TODO(bradfitz): more? return false @@ -139,23 +154,77 @@ func userLookupGetent(usernameOrUID string, std lookupStd) (*user.User, string, } ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - out, err := exec.CommandContext(ctx, "getent", "passwd", usernameOrUID).Output() + + args := []string{"passwd"} + // Append "--" only if the local getent accepts it, to prevent a username or + // UID from being interpreted as an option without breaking getent variants + // that do not support end-of-options parsing here. + if getentDoubleDashSupported() { + args = append(args, "--") + } + args = append(args, usernameOrUID) + + out, err := execGetent(ctx, args...) if err != nil { log.Printf("error calling getent for user %q: %v", usernameOrUID, err) u, err := std(usernameOrUID) return u, "", err } + u, shell, err := parseGetentUser(out) + if err != nil { + log.Printf("getent for user %q returned invalid output %q: %v", usernameOrUID, out, err) + u, err := std(usernameOrUID) + return u, "", err + } + return u, shell, nil +} + +func probeGetentDoubleDashSupport() bool { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + out, err := execGetent(ctx, "passwd", "root") + if err != nil { + return false + } + + outDoubleDash, err := execGetent(ctx, "passwd", "--", "root") + if err != nil { + return false + } + + u, shell, err := parseGetentUser(out) + if err != nil { + return false + } + + uDoubleDash, shellDoubleDash, err := parseGetentUser(outDoubleDash) + if err != nil { + return false + } + + // Short-circuit if the user is obviously incorrect + if uDoubleDash.Name != "root" || uDoubleDash.Uid != "0" || uDoubleDash.Gid != "0" { + return false + } + + if !reflect.DeepEqual(u, uDoubleDash) || shell != shellDoubleDash { + return false + } + + return true +} + +func parseGetentUser(out []byte) (*user.User, string, error) { // output is "alice:x:1001:1001:Alice Smith,,,:/home/alice:/bin/bash" f := strings.SplitN(strings.TrimSpace(string(out)), ":", 10) for len(f) < 7 { f = append(f, "") } - var mandatoryFields = []int{0, 2, 3, 5} - for _, v := range mandatoryFields { - if f[v] == "" { - log.Printf("getent for user %q returned invalid output: %q", usernameOrUID, out) - u, err := std(usernameOrUID) - return u, "", err + var mandatoryFields = map[int]string{0: "Username", 2: "Uid", 3: "Gid", 5: "HomeDir"} + for k, v := range mandatoryFields { + if f[k] == "" { + return nil, "", fmt.Errorf("missing mandatory field %q", v) } } return &user.User{ diff --git a/util/osuser/user_test.go b/util/osuser/user_test.go new file mode 100644 index 000000000..cbea237af --- /dev/null +++ b/util/osuser/user_test.go @@ -0,0 +1,103 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package osuser + +import ( + "context" + "errors" + "os/user" + "reflect" + "sync" + "testing" +) + +func TestProbeGetentDoubleDashSupport(t *testing.T) { + origExecGetent := execGetent + t.Cleanup(func() { + execGetent = origExecGetent + }) + + tests := []struct { + name string + out []byte + err error + want bool + }{ + { + name: "supported", + out: []byte("root:x:0:0:root:/root:/bin/sh\n"), + want: true, + }, + { + name: "error", + err: errors.New("unsupported"), + want: false, + }, + { + name: "wrong-user", + out: []byte("daemon:x:1:1:daemon:/usr/sbin:/usr/sbin/nologin\n"), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + execGetent = func(_ context.Context, args ...string) ([]byte, error) { + return tt.out, tt.err + } + if got := probeGetentDoubleDashSupport(); got != tt.want { + t.Fatalf("probeGetentDashDashSupport() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestUserLookupGetentUsesProbeResult(t *testing.T) { + origExecGetent := execGetent + origProbe := getentDoubleDashSupported + t.Cleanup(func() { + execGetent = origExecGetent + getentDoubleDashSupported = origProbe + }) + + tests := []struct { + name string + supported bool + wantArgs []string + }{ + { + name: "supported", + supported: true, + wantArgs: []string{"passwd", "--", "alice"}, + }, + { + name: "unsupported", + supported: false, + wantArgs: []string{"passwd", "alice"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + getentDoubleDashSupported = sync.OnceValue(func() bool { return tt.supported }) + execGetent = func(_ context.Context, args ...string) ([]byte, error) { + if !reflect.DeepEqual(args, tt.wantArgs) { + t.Fatalf("args = %q, want %q", args, tt.wantArgs) + } + return []byte("alice:x:1001:1001:Alice:/home/alice:/bin/sh\n"), nil + } + std := func(string) (*user.User, error) { + t.Fatal("std lookup should not be called") + return nil, nil + } + u, shell, err := userLookupGetent("alice", std) + if err != nil { + t.Fatalf("userLookupGetent error: %v", err) + } + if u.Username != "alice" || shell != "/bin/sh" { + t.Fatalf("got user=%+v shell=%q", u, shell) + } + }) + } +}