Skip to content

Commit 7d24051

Browse files
committed
auditd: Avoid blocking on open syscall
`open` might block on FIFO files or some character/block devices such as /dev/tty. Checking `stat` based on path introduces a TOCTOU issue.
1 parent 263414b commit 7d24051

File tree

3 files changed

+29
-15
lines changed

3 files changed

+29
-15
lines changed

audisp/audispd-pconfig.c

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ void clear_pconfig(plugin_conf_t *config)
134134
config->restart_cnt = 0;
135135
}
136136

137-
int load_pconfig(plugin_conf_t *config, char *file)
137+
int load_pconfig(plugin_conf_t *config, int dirfd, char *file)
138138
{
139139
int fd, rc, mode, lineno = 1;
140140
struct stat st;
@@ -143,9 +143,12 @@ int load_pconfig(plugin_conf_t *config, char *file)
143143

144144
clear_pconfig(config);
145145

146-
/* open the file */
147-
mode = O_RDONLY;
148-
rc = open(file, mode);
146+
/* only get a file descriptor, don't fully open.
147+
* This avoids blocking on block/char devices and FIFO files.
148+
* We do not pass O_NOFOLLOW, which allows for symlinked configs.
149+
*/
150+
mode = O_RDONLY | O_NONBLOCK;
151+
rc = openat(dirfd, file, mode);
149152
if (rc < 0) {
150153
if (errno != ENOENT) {
151154
audit_msg(LOG_ERR, "Error opening %s (%s)", file,
@@ -159,7 +162,7 @@ int load_pconfig(plugin_conf_t *config, char *file)
159162
fd = rc;
160163

161164
/* check the file's permissions: owned by root, not world writable,
162-
* not symlink.
165+
* pointing to regular file.
163166
*/
164167
if (fstat(fd, &st) < 0) {
165168
audit_msg(LOG_ERR, "Error fstat'ing config file (%s)",
@@ -168,24 +171,32 @@ int load_pconfig(plugin_conf_t *config, char *file)
168171
return 1;
169172
}
170173
if (st.st_uid != 0) {
171-
audit_msg(LOG_ERR, "Error - %s isn't owned by root",
172-
file);
174+
audit_msg(LOG_ERR, "Error - %s isn't owned by root, instead it is owned by %i",
175+
file, st.st_uid);
173176
close(fd);
174177
return 1;
175178
}
176179
if ((st.st_mode & S_IWOTH) == S_IWOTH) {
177-
audit_msg(LOG_ERR, "Error - %s is world writable",
178-
file);
180+
audit_msg(LOG_ERR, "Error - %s is world writable: %i",
181+
file, st.st_mode);
179182
close(fd);
180183
return 1;
181184
}
185+
// this checks if the actual file is a regular file. The initial open might have followed a symlink, which is acceptable.
182186
if (!S_ISREG(st.st_mode)) {
183187
audit_msg(LOG_ERR, "Error - %s is not a regular file",
184188
file);
185189
close(fd);
186190
return 1;
187191
}
188192

193+
if (fcntl(fd, F_SETFL, mode & (~O_NONBLOCK)) < 0) {
194+
audit_msg(LOG_ERR, "Error - Failed to remove nonblock flag for %s",
195+
file);
196+
close(fd);
197+
return 1;
198+
}
199+
189200
/* it's ok, read line by line */
190201
f = fdopen(fd, "rm");
191202
if (f == NULL) {

audisp/audispd-pconfig.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ typedef struct plugin_conf
5050
} plugin_conf_t;
5151

5252
void clear_pconfig(plugin_conf_t *config);
53-
int load_pconfig(plugin_conf_t *config, char *file);
53+
int load_pconfig(plugin_conf_t *config, int dirfd, char *file);
5454
void free_pconfig(plugin_conf_t *config);
5555

5656
#endif

audisp/audispd.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,24 @@ static int count_dots(const char *s)
104104
static void load_plugin_conf(conf_llist *plugin)
105105
{
106106
DIR *d;
107+
int dfd;
107108

108109
/* init plugin list */
109110
plist_create(plugin);
110111

111112
/* read configs */
112113
d = opendir(daemon_config.plugin_dir);
113114
if (d) {
115+
int dfd = dirfd(d);
116+
if (dfd < 0) {
117+
closedir(d);
118+
return;
119+
}
120+
114121
struct dirent *e;
115122

116123
while ((e = readdir(d))) {
117124
plugin_conf_t config;
118-
char fname[PATH_MAX];
119125
const char *ext, *reason = NULL;
120126

121127
if (e->d_type != DT_REG)
@@ -132,11 +138,8 @@ static void load_plugin_conf(conf_llist *plugin)
132138
continue;
133139
}
134140

135-
snprintf(fname, sizeof(fname), "%s/%s",
136-
daemon_config.plugin_dir, e->d_name);
137-
138141
clear_pconfig(&config);
139-
if (load_pconfig(&config, fname) == 0) {
142+
if (load_pconfig(&config, dfd, e->d_name) == 0) {
140143
/* Push onto config list only if active */
141144
if (config.active == A_YES)
142145
plist_append(plugin, &config);

0 commit comments

Comments
 (0)