Skip to content

Commit 91b4211

Browse files
committed
run_part: use fexecve(3) to avoid toctou issues
Pin the script to execute before gathering its information to avoid any potential time-of-check-time-of-use issues.
1 parent 133b76b commit 91b4211

File tree

2 files changed

+46
-31
lines changed

2 files changed

+46
-31
lines changed

configure.ac

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ AC_CHECK_FUNCS(arc4random_buf futimes \
5252
setgroups updwtmpx innetgr \
5353
getspnam_r \
5454
rpmatch \
55-
memset_explicit explicit_bzero stpecpy stpeprintf)
55+
memset_explicit explicit_bzero scandirat stpecpy stpeprintf)
5656
AC_SYS_LARGEFILE
5757

5858
dnl Checks for typedefs, structures, and compiler characteristics.

lib/run_part.c

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <dirent.h>
44
#include <errno.h>
5+
#include <fcntl.h>
56
#include <stdio.h>
67
#include <stdlib.h>
78
#include <string.h>
@@ -15,12 +16,12 @@
1516
#include "shadowlog_internal.h"
1617

1718

18-
static int run_part(char *script_path, const char *name, const char *action)
19+
static int run_part(int script_fd, const char *script_name, const char *name, const char *action)
1920
{
2021
pid_t pid;
2122
int wait_status;
2223
pid_t pid_status;
23-
char *args[] = { script_path, NULL };
24+
char *args[] = { (char *) script_name, NULL };
2425

2526
pid=fork();
2627
if (pid==-1) {
@@ -30,8 +31,8 @@ static int run_part(char *script_path, const char *name, const char *action)
3031
if (pid==0) {
3132
setenv("ACTION",action,1);
3233
setenv("SUBJECT",name,1);
33-
execv(script_path,args);
34-
fprintf(shadow_logfd, "execv: %s\n", strerror(errno));
34+
fexecve(script_fd, args, environ);
35+
fprintf(shadow_logfd, "fexecve(%s): %s\n", script_name, strerror(errno));
3536
exit(1);
3637
}
3738

@@ -40,7 +41,7 @@ static int run_part(char *script_path, const char *name, const char *action)
4041
return (wait_status);
4142
}
4243

43-
fprintf(shadow_logfd, "waitpid: %s\n", strerror(errno));
44+
fprintf(shadow_logfd, "failed to wait for pid %d (%s): %s\n", pid, script_name, strerror(errno));
4445
return (1);
4546
}
4647

@@ -51,68 +52,82 @@ int run_parts(const char *directory, const char *name, const char *action)
5152
int n;
5253
int execute_result = 0;
5354

55+
int dfd = open(directory, O_PATH | O_DIRECTORY | O_CLOEXEC);
56+
if (dfd == -1) {
57+
fprintf(shadow_logfd, "failed open directory %s: %s\n", directory, strerror(errno));
58+
return (1);
59+
}
60+
61+
#ifdef HAVE_SCANDIRAT
62+
scanlist = scandirat(dfd, ".", &namelist, 0, alphasort);
63+
#else
5464
scanlist = scandir(directory, &namelist, 0, alphasort);
65+
#endif
5566
if (scanlist<=0) {
67+
(void) close(dfd);
5668
return (0);
5769
}
5870

5971
for (n=0; n<scanlist; n++) {
60-
char *s;
61-
struct stat sb;
62-
63-
if (asprintf(&s, "%s/%s", directory, namelist[n]->d_name) == -1) {
64-
fprintf(shadow_logfd, "asprintf: %s\n", strerror(errno));
65-
for (; n<scanlist; n++) {
66-
free(namelist[n]);
72+
struct stat sb;
73+
74+
int fd = openat (dfd, namelist[n]->d_name, O_PATH | O_CLOEXEC);
75+
if (fd == -1) {
76+
fprintf(shadow_logfd, "failed to open %s/%s: %s\n",
77+
directory, namelist[n]->d_name, strerror(errno));
78+
for (int k = n; k < scanlist; k++) {
79+
free(namelist[k]);
6780
}
6881
free(namelist);
82+
(void) close(dfd);
6983
return (1);
7084
}
7185

72-
execute_result = 0;
73-
if (stat(s, &sb) == -1) {
74-
fprintf(shadow_logfd, "stat: %s\n", strerror(errno));
75-
free(s);
76-
for (; n<scanlist; n++) {
77-
free(namelist[n]);
86+
if (fstat (fd, &sb) == -1) {
87+
fprintf(shadow_logfd, "failed to stat %s/%s: %s\n",
88+
directory, namelist[n]->d_name, strerror(errno));
89+
(void) close(fd);
90+
(void) close(dfd);
91+
for (int k = n; k < scanlist; k++) {
92+
free(namelist[k]);
7893
}
7994
free(namelist);
8095
return (1);
8196
}
8297

8398
if (!S_ISREG(sb.st_mode)) {
84-
free(s);
99+
(void) close(fd);
85100
free(namelist[n]);
86101
continue;
87102
}
88103

89104
if ((sb.st_uid != 0 && sb.st_uid != getuid()) ||
90105
(sb.st_gid != 0 && sb.st_gid != getgid()) ||
91106
(sb.st_mode & 0002)) {
92-
fprintf(shadow_logfd, "skipping %s due to insecure ownership/permission\n",
93-
s);
94-
free(s);
107+
fprintf(shadow_logfd, "skipping %s/%s due to insecure ownership/permission\n",
108+
directory, namelist[n]->d_name);
109+
(void) close(fd);
95110
free(namelist[n]);
96111
continue;
97112
}
98113

99-
execute_result = run_part(s, name, action);
114+
execute_result = run_part(fd, namelist[n]->d_name, name, action);
115+
(void) close(fd);
100116
if (execute_result!=0) {
101117
fprintf(shadow_logfd,
102-
"%s: did not exit cleanly.\n",
103-
s);
104-
free(s);
105-
for (; n<scanlist; n++) {
106-
free(namelist[n]);
118+
"%s/%s: did not exit cleanly.\n",
119+
directory, namelist[n]->d_name);
120+
for (int k = n; k < scanlist; k++) {
121+
free(namelist[k]);
107122
}
108123
break;
109124
}
110125

111-
free(s);
112126
free(namelist[n]);
113127
}
114128
free(namelist);
115129

130+
(void) close(dfd);
131+
116132
return (execute_result);
117133
}
118-

0 commit comments

Comments
 (0)