From 574e68a9e6d069aaed39e84481ac934674bea265 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 22 Sep 2026 21:09:53 +0200 Subject: [PATCH] Harden the xauth traversal walk against symlinks, keep the runtime dir readable, and enable SeIncreaseQuotaPrivilege for agent spawning --- client/vnc/server/server_windows.go | 6 +++++- client/vnc/server/xauth_x11.go | 28 ++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/client/vnc/server/server_windows.go b/client/vnc/server/server_windows.go index c3f85ff25..2c26a3ee4 100644 --- a/client/vnc/server/server_windows.go +++ b/client/vnc/server/server_windows.go @@ -349,7 +349,11 @@ func (s *Server) platformShutdown() { // Session 0 operations (agent spawning, SendSAS). func (s *Server) platformInit() { var prior []windows.Tokenprivileges - for _, priv := range []string{"SeTcbPrivilege", "SeAssignPrimaryTokenPrivilege"} { + // SeIncreaseQuotaPrivilege belongs with the other two: CreateProcessAsUser + // assigns the new process a quota against the target user, and without it + // the call fails with ERROR_PRIVILEGE_NOT_HELD, which the spawn path treats + // as fatal and disables service mode for the whole run. + for _, priv := range []string{"SeTcbPrivilege", "SeAssignPrimaryTokenPrivilege", "SeIncreaseQuotaPrivilege"} { prev, err := enablePrivilege(priv) if err != nil { log.Debugf("enable %s: %v", priv, err) diff --git a/client/vnc/server/xauth_x11.go b/client/vnc/server/xauth_x11.go index aca3dec10..61d1d4466 100644 --- a/client/vnc/server/xauth_x11.go +++ b/client/vnc/server/xauth_x11.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "github.com/jezek/xgb" @@ -39,6 +40,16 @@ func writeXAuthFile(path, hostname, display string, cookie []byte, uid, gid uint return fmt.Errorf("cookie must be 16 bytes") } dir := filepath.Dir(path) + // The runtime dir is shared: unprivileged CLI and UI clients list it to + // find the daemon's socket, so it has to stay readable. Create it at 0755 + // before MkdirAll can mint it at 0711 as a side effect of creating the + // xauth subdirectory below, which would break that discovery on a host + // where nothing else had created it yet. + if root := filepath.Clean(configs.RuntimeDir); root != "" { + if err := os.MkdirAll(root, 0755); err != nil { + return fmt.Errorf("mkdir runtime dir: %w", err) + } + } if err := os.MkdirAll(dir, 0711); err != nil { return fmt.Errorf("mkdir xauth parent: %w", err) } @@ -137,7 +148,20 @@ func ensureTraversable(dir string) error { // addTraversalBits ORs group and other execute onto dir's mode, leaving every // other bit as it was. A directory that is already traversable is not touched. func addTraversalBits(dir string) error { - info, err := os.Stat(dir) + // os.Stat and os.Chmod both resolve symlinks, and this walks a directory + // other local accounts can write to. A component pre-created there as a + // symlink would have the daemon, running as root, OR the execute bits onto + // whatever it points at, loosening a directory the attacker could not + // otherwise traverse. Going through an O_NOFOLLOW descriptor refuses the + // symlink and pins the mode change to the directory actually opened, the + // same way the temp file above is handled. + f, err := os.OpenFile(dir, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_DIRECTORY, 0) + if err != nil { + return fmt.Errorf("open %s: %w", dir, err) + } + defer f.Close() + + info, err := f.Stat() if err != nil { return fmt.Errorf("stat %s: %w", dir, err) } @@ -146,7 +170,7 @@ func addTraversalBits(dir string) error { if mode&traversal == traversal { return nil } - if err := os.Chmod(dir, mode|traversal); err != nil { + if err := f.Chmod(mode | traversal); err != nil { return fmt.Errorf("chmod %s: %w", dir, err) } return nil