Skip to content

Commit 1f3dbcd

Browse files
committed
log_server_accept: audit_details cannot be a local variable.
This reverts part of f278cb8. Because log_server_open() stores audit_details in the closure it needs to have global scope. Also add audit_details_free() to free the only dynamically allocated part of audit_details. This makes things a bit more consistent with similar code in iolog.c. Found by Joshua Rogers using the ZeroPath tool.
1 parent c488119 commit 1f3dbcd

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

plugins/sudoers/audit.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,19 @@ audit_to_eventlog(const struct sudoers_context *ctx, struct eventlog *evlog,
241241
}
242242

243243
#ifdef SUDOERS_LOG_CLIENT
244+
static struct log_details audit_details;
245+
246+
static void
247+
free_audit_details(void)
248+
{
249+
debug_decl(free_audit_details, SUDOERS_DEBUG_PLUGIN);
250+
251+
/* Only the log_servers string list is dynamically allocated. */
252+
str_list_free(audit_details.log_servers);
253+
254+
debug_return;
255+
}
256+
244257
static bool
245258
log_server_accept(const struct sudoers_context *ctx, struct eventlog *evlog)
246259
{
@@ -272,13 +285,16 @@ log_server_accept(const struct sudoers_context *ctx, struct eventlog *evlog)
272285
ret = true;
273286
}
274287
} else {
275-
struct log_details audit_details;
276-
288+
/* No existing client closure, I/O logging not enabled. */
277289
if (sudo_gettime_awake(&start_time) == -1) {
278290
sudo_warn("%s", U_("unable to get time of day"));
279291
goto done;
280292
}
281293

294+
/*
295+
* audit_details is stored in client_closure->log_details
296+
* and must remain in scope until the command exits.
297+
*/
282298
if (!init_log_details(&audit_details, evlog))
283299
goto done;
284300

@@ -287,9 +303,6 @@ log_server_accept(const struct sudoers_context *ctx, struct eventlog *evlog)
287303
SEND_ACCEPT, NULL);
288304
if (client_closure != NULL)
289305
ret = true;
290-
291-
/* Only the log_servers string list is dynamically allocated. */
292-
str_list_free(audit_details.log_servers);
293306
}
294307

295308
done:
@@ -320,6 +333,7 @@ log_server_exit(int status_type, int status)
320333
log_server_close(client_closure, exit_status, error);
321334
client_closure = NULL;
322335
}
336+
free_audit_details();
323337

324338
debug_return;
325339
}

0 commit comments

Comments
 (0)