[openssh-commits] [openssh] 02/04: upstream: Stop doing access() before execve(). It is a TOCTOU, but

[git+noreply at mindrot.org]

This is an automated email from the git hooks/post-receive script.

djm pushed a commit to branch master
in repository openssh.

commit b75bf339eae6115c544bdcefa0d67a6dcc971ec5
Author: deraadt at openbsd.org <deraadt at openbsd.org>
AuthorDate: Sat Mar 7 18:27:52 2026 +0000

    upstream: Stop doing access() before execve(). It is a TOCTOU, but
    
    also it forces use of unveil "rx" instead of "x". This is done by using a
    pipe() through the fork+execve attempt to expose execve failure and create
    the same error return as the access() used to do. ok djm dtucker
    
    OpenBSD-Commit-ID: f9ee96e20352f35dc6f39127e0cc6b804700200a
---
 ssh-sk-client.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/ssh-sk-client.c b/ssh-sk-client.c
index df3dd0fc7..c8b0695b0 100644
--- a/ssh-sk-client.c
+++ b/ssh-sk-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk-client.c,v 1.14 2026/02/14 00:18:34 jsg Exp $ */
+/* $OpenBSD: ssh-sk-client.c,v 1.15 2026/03/07 18:27:52 deraadt Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -29,6 +29,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <fcntl.h>
 
 #include "log.h"
 #include "ssherr.h"
@@ -45,9 +46,10 @@ static int
 start_helper(int *fdp, pid_t *pidp, void (**osigchldp)(int))
 {
 	void (*osigchld)(int);
-	int oerrno, pair[2];
+	int oerrno, pair[2], execpipe[2];
+	ssize_t n;
 	pid_t pid;
-	char *helper, *verbosity = NULL;
+	char execbuf[100], *helper, *verbosity = NULL;
 
 	*fdp = -1;
 	*pidp = 0;
@@ -56,19 +58,20 @@ start_helper(int *fdp, pid_t *pidp, void (**osigchldp)(int))
 	helper = getenv("SSH_SK_HELPER");
 	if (helper == NULL || strlen(helper) == 0)
 		helper = _PATH_SSH_SK_HELPER;
-	if (access(helper, X_OK) != 0) {
-		oerrno = errno;
-		error_f("helper \"%s\" unusable: %s", helper, strerror(errno));
-		errno = oerrno;
-		return SSH_ERR_SYSTEM_ERROR;
-	}
 #ifdef DEBUG_SK
 	verbosity = "-vvv";
 #endif
 
+	/* Create a O_CLOEXEC pipe to capture the execve() failure */
+	if (pipe(execpipe) == -1) {
+		error("pipe:  %s", strerror(errno));
+		return SSH_ERR_SYSTEM_ERROR;
+	}
 	/* Start helper */
 	if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) == -1) {
 		error("socketpair: %s", strerror(errno));
+		close(execpipe[0]);
+		close(execpipe[1]);
 		return SSH_ERR_SYSTEM_ERROR;
 	}
 	osigchld = ssh_signal(SIGCHLD, SIG_DFL);
@@ -77,14 +80,20 @@ start_helper(int *fdp, pid_t *pidp, void (**osigchldp)(int))
 		error("fork: %s", strerror(errno));
 		close(pair[0]);
 		close(pair[1]);
+		close(execpipe[0]);
+		close(execpipe[1]);
 		ssh_signal(SIGCHLD, osigchld);
 		errno = oerrno;
 		return SSH_ERR_SYSTEM_ERROR;
 	}
 	if (pid == 0) {
+		close(execpipe[0]);
+		fcntl(execpipe[1], F_SETFD, FD_CLOEXEC);
 		if ((dup2(pair[1], STDIN_FILENO) == -1) ||
 		    (dup2(pair[1], STDOUT_FILENO) == -1)) {
-			error_f("dup2: %s", strerror(errno));
+			snprintf(execbuf, sizeof execbuf,
+			    "dup2: %s", strerror(errno));
+			write(execpipe[1], execbuf, strlen(execbuf)+1);
 			_exit(1);
 		}
 		close(pair[0]);
@@ -93,11 +102,22 @@ start_helper(int *fdp, pid_t *pidp, void (**osigchldp)(int))
 		debug_f("starting %s %s", helper,
 		    verbosity == NULL ? "" : verbosity);
 		execlp(helper, helper, verbosity, (char *)NULL);
-		error_f("execlp: %s", strerror(errno));
+		snprintf(execbuf, sizeof execbuf,
+		    "execlp: %s", strerror(errno));
+		write(execpipe[1], execbuf, strlen(execbuf)+1);
 		_exit(1);
 	}
 	close(pair[1]);
 
+	close(execpipe[1]);
+	n = read(execpipe[0], execbuf, sizeof execbuf);
+	close(execpipe[0]);
+	if (n > 0) {
+		execbuf[n] = '\0';
+		error("%s", execbuf);
+		return SSH_ERR_SYSTEM_ERROR;
+	}
+
 	/* success */
 	debug3_f("started pid=%ld", (long)pid);
 	*fdp = pair[0];

-- 
To stop receiving notification emails like this one, please contact
djm at mindrot.org.