mirror of
https://github.com/openbsd/src.git
synced 2026-04-25 14:45:52 +00:00
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
This commit is contained in:
@@ -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
|
||||
*
|
||||
@@ -27,6 +27,7 @@
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <unistd.h>
|
||||
#include <fcntl.h>
|
||||
|
||||
#include "log.h"
|
||||
#include "ssherr.h"
|
||||
@@ -43,9 +44,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;
|
||||
@@ -54,19 +56,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);
|
||||
@@ -75,14 +78,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]);
|
||||
@@ -91,11 +100,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];
|
||||
|
||||
Reference in New Issue
Block a user