From 9c9bcfc0a0f4128bb1d741fb963ed8474508bf3e Mon Sep 17 00:00:00 2001 From: Damien Degois Date: Mon, 18 May 2026 12:34:16 +0200 Subject: [PATCH 1/3] security: validate session directory ownership and mode Verify the session directory ($HOME/.cache/ or the /tmp/.- fallback) is a real directory owned by the current uid with mode 0700 after mkdir(). The mkdir() return value was previously ignored, so on the /tmp fallback path a hostile local user could pre-create /tmp/.-/ with permissive modes (or as a symlink) and intercept sockets and logs written by the victim. Also open the session log with O_NOFOLLOW so a pre-placed symlink at .log cannot redirect master writes into an arbitrary file. --- atch.c | 32 ++++++++++++++++++++++++++++++++ master.c | 6 +++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/atch.c b/atch.c index 9a516de..be59d24 100644 --- a/atch.c +++ b/atch.c @@ -254,6 +254,36 @@ static int parse_options(int *argc, char ***argv) return 0; } +/* Verify dir is a real directory owned by us with mode 0700. Refuse if not: +** an attacker who pre-creates /tmp/.atch-/ (or any session dir) with +** weaker modes or as a symlink could trick us into writing sockets and logs +** under their control. lstat ensures we don't follow a symlink. */ +static int ensure_safe_dir(const char *path) +{ + struct stat st; + + if (lstat(path, &st) < 0) { + printf("%s: %s: %s\n", progname, path, strerror(errno)); + return 1; + } + if (!S_ISDIR(st.st_mode)) { + printf("%s: %s: not a directory\n", progname, path); + return 1; + } + if (st.st_uid != getuid()) { + printf("%s: %s: refusing to use directory not owned by uid %d\n", + progname, path, (int)getuid()); + return 1; + } + if ((st.st_mode & 077) != 0) { + printf("%s: %s: refusing to use directory with mode %04o " + "(must be 0700)\n", + progname, path, (unsigned)(st.st_mode & 07777)); + return 1; + } + return 0; +} + /* Expand a bare session name to its full socket path in-place. */ static void expand_sockname(void) { @@ -273,6 +303,8 @@ static void expand_sockname(void) *slash = '/'; } mkdir(dir, 0700); + if (ensure_safe_dir(dir)) + exit(1); fulllen = strlen(dir) + 1 + strlen(sockname); full = malloc(fulllen + 1); snprintf(full, fulllen + 1, "%s/%s", dir, sockname); diff --git a/master.c b/master.c index 16f29e3..2b0fc0f 100644 --- a/master.c +++ b/master.c @@ -83,7 +83,11 @@ static int open_log(const char *path) { int fd; - fd = open(path, O_RDWR | O_CREAT, 0600); + /* O_NOFOLLOW: refuse to open if the log path is a symlink. Combined + ** with the session-dir uid/mode check, this stops an attacker who + ** controls the session directory from redirecting the log into an + ** arbitrary file via a pre-placed symlink. */ + fd = open(path, O_RDWR | O_CREAT | O_NOFOLLOW, 0600); if (fd < 0) return -1; From 0f7dcfc9fdfe92cc9e1dd8c567cda03d120d32ce Mon Sep 17 00:00:00 2001 From: Damien Degois Date: Mon, 18 May 2026 12:35:06 +0200 Subject: [PATCH 2/3] security: stat before unlinking stale session socket cmd_open() and the legacy -A mode unlink sockname unconditionally when connect() returns ECONNREFUSED. With the session directory now validated as user-owned 0700 this is no longer reachable cross-user, but defense in depth is cheap: lstat the path and only unlink when it really is a unix socket owned by the current uid, closing the TOCTOU between the failed connect and the unlink. --- atch.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/atch.c b/atch.c index be59d24..642552a 100644 --- a/atch.c +++ b/atch.c @@ -254,6 +254,23 @@ static int parse_options(int *argc, char ***argv) return 0; } +/* lstat the socket path and unlink only if it really is a unix socket owned +** by the current user. Without this check, a race between connect() failing +** with ECONNREFUSED and the unlink lets an attacker who controls the session +** directory swap in a symlink and have us delete an arbitrary file. */ +static void safe_unlink_socket(const char *path) +{ + struct stat st; + + if (lstat(path, &st) < 0) + return; + if (!S_ISSOCK(st.st_mode)) + return; + if (st.st_uid != getuid()) + return; + unlink(path); +} + /* Verify dir is a real directory owned by us with mode 0700. Refuse if not: ** an attacker who pre-creates /tmp/.atch-/ (or any session dir) with ** weaker modes or as a symlink could trick us into writing sockets and logs @@ -738,7 +755,7 @@ static int cmd_open(char *session, int argc, char **argv) replay_session_log(saved_errno); if (saved_errno == ECONNREFUSED) - unlink(sockname); + safe_unlink_socket(sockname); if (master_main(argv, 1, 0) != 0) return 1; if (!quiet) @@ -933,7 +950,7 @@ int main(int argc, char **argv) replay_session_log(saved_errno); if (saved_errno == ECONNREFUSED) - unlink(sockname); + safe_unlink_socket(sockname); if (master_main(argv, 1, 0) != 0) return 1; } From 7bb439db9e4171a55857252b259272fe0c2785c8 Mon Sep 17 00:00:00 2001 From: Damien Degois Date: Mon, 18 May 2026 12:35:50 +0200 Subject: [PATCH 3/3] security: copy session name from env in cmd_clear, O_NOFOLLOW the log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd_clear stored a const pointer returned by getenv() into the global char *sockname via a cast that drops const — undefined behavior if any future code path mutates sockname, and fragile in general. Duplicate the chosen ancestry segment into owned storage instead. Open the log with O_NOFOLLOW so a pre-placed symlink at .log cannot redirect the truncate onto an arbitrary user-writable file. --- atch.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/atch.c b/atch.c index 642552a..9ce622f 100644 --- a/atch.c +++ b/atch.c @@ -569,6 +569,7 @@ static int cmd_clear(int argc, char **argv) } else { const char *chain = getenv(SESSION_ENVVAR); const char *last; + char *copy; if (!chain || !*chain) { printf("%s: No session was specified.\n", progname); @@ -577,7 +578,13 @@ static int cmd_clear(int argc, char **argv) return 1; } last = strrchr(chain, ':'); - sockname = (char *)(last ? last + 1 : chain); + /* getenv() returns a pointer into the environment block; sockname + ** is char* and other code paths assume it's owned writable storage. + ** Duplicate so we never cast away const or risk mutating env. */ + copy = strdup(last ? last + 1 : chain); + if (!copy) + return 1; + sockname = copy; } if (argc > 0) { printf("%s: Invalid number of arguments.\n", progname); @@ -585,7 +592,10 @@ static int cmd_clear(int argc, char **argv) return 1; } snprintf(log_path, sizeof(log_path), "%s.log", sockname); - fd = open(log_path, O_WRONLY | O_TRUNC); + /* O_NOFOLLOW: refuse to truncate via a symlink. Without it, anyone + ** who can plant a symlink at .log can redirect this truncate + ** onto an arbitrary user-writable file. */ + fd = open(log_path, O_WRONLY | O_TRUNC | O_NOFOLLOW); if (fd >= 0) { close(fd); if (!quiet)