2
0
mirror of https://github.com/sudo-project/sudo.git synced 2025-08-22 09:57:41 +00:00

Initialize exec closure before calling sudo_fatal_callback_register()

The pty_cleanup() function, which may be called via fatal()/fatalx(),
expects that ec->details is set.  If there is a fatal error after
the cleanup hook is registered but before the exec closure it filled
in, pty_cleanup() would dereference a NULL pointer.
Reported by Bjorn Baron.
This commit is contained in:
Todd C. Miller 2025-01-21 09:32:30 -07:00
parent 6fc816d90b
commit 0be9f0f947

View File

@ -1,7 +1,7 @@
/*
* SPDX-License-Identifier: ISC
*
* Copyright (c) 2009-2024 Todd C. Miller <Todd.Miller@sudo.ws>
* Copyright (c) 2009-2025 Todd C. Miller <Todd.Miller@sudo.ws>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@ -63,44 +63,6 @@ static void sync_ttysize(struct exec_closure *ec);
static void schedule_signal(struct exec_closure *ec, int signo);
static void send_command_status(struct exec_closure *ec, int type, int val);
/*
* Allocate a pty if /dev/tty is a tty.
* Fills in io_fds[SFD_USERTTY], io_fds[SFD_LEADER] and io_fds[SFD_FOLLOWER].
* Returns the dynamically allocated pty name on success, NULL on failure.
*/
static char *
pty_setup(struct command_details *details)
{
char *ptyname = NULL;
debug_decl(pty_setup, SUDO_DEBUG_EXEC);
io_fds[SFD_USERTTY] = open(_PATH_TTY, O_RDWR);
if (io_fds[SFD_USERTTY] == -1) {
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: no %s, not allocating a pty",
__func__, _PATH_TTY);
debug_return_ptr(NULL);
}
ptyname = get_pty(&io_fds[SFD_LEADER], &io_fds[SFD_FOLLOWER],
details->cred.euid);
if (ptyname == NULL)
sudo_fatal("%s", U_("unable to allocate pty"));
/* Add entry to utmp/utmpx? */
if (ISSET(details->flags, CD_SET_UTMP))
utmp_login(details->tty, ptyname, io_fds[SFD_FOLLOWER], details->utmp_user);
/* Update tty name in command details (used by monitor, SELinux, AIX). */
details->tty = ptyname;
sudo_debug_printf(SUDO_DEBUG_INFO,
"%s: %s fd %d, pty leader fd %d, pty follower fd %d",
__func__, _PATH_TTY, io_fds[SFD_USERTTY], io_fds[SFD_LEADER],
io_fds[SFD_FOLLOWER]);
debug_return_str(ptyname);
}
/*
* Restore user's terminal settings and update utmp, as needed.
*/
@ -136,6 +98,49 @@ pty_cleanup_hook(void)
pty_cleanup(&pty_ec, 0);
}
/*
* Allocate a pty if /dev/tty is a tty.
* Fills in io_fds[SFD_USERTTY], io_fds[SFD_LEADER] and io_fds[SFD_FOLLOWER].
* Returns the dynamically allocated pty name on success, NULL on failure.
*/
static bool
pty_setup(struct exec_closure *ec)
{
struct command_details *details = ec->details;
debug_decl(pty_setup, SUDO_DEBUG_EXEC);
io_fds[SFD_USERTTY] = open(_PATH_TTY, O_RDWR);
if (io_fds[SFD_USERTTY] == -1) {
sudo_debug_printf(SUDO_DEBUG_INFO, "%s: no %s, not allocating a pty",
__func__, _PATH_TTY);
debug_return_bool(false);
}
ec->ptyname = get_pty(&io_fds[SFD_LEADER], &io_fds[SFD_FOLLOWER],
details->cred.euid);
if (ec->ptyname == NULL)
sudo_fatal("%s", U_("unable to allocate pty"));
/* Add entry to utmp/utmpx? */
if (ISSET(details->flags, CD_SET_UTMP)) {
utmp_login(details->tty, ec->ptyname, io_fds[SFD_FOLLOWER],
details->utmp_user);
}
/* Update tty name in command details (used by monitor, SELinux, AIX). */
details->tty = ec->ptyname;
/* Register cleanup function. */
sudo_fatal_callback_register(pty_cleanup_hook);
sudo_debug_printf(SUDO_DEBUG_INFO,
"%s: %s fd %d, pty leader fd %d, pty follower fd %d",
__func__, _PATH_TTY, io_fds[SFD_USERTTY], io_fds[SFD_LEADER],
io_fds[SFD_FOLLOWER]);
debug_return_bool(true);
}
/*
* Check whether sudo is running in the foreground.
* Updates the foreground flag in the closure.
@ -956,16 +961,14 @@ fwdchannel_cb(int sock, int what, void *v)
}
/*
* Fill in the exec closure and setup initial exec events.
* Allocates events for the signal pipe and backchannel.
* Forwarded signals on the backchannel are enabled on demand.
* Fill in the non-event part of the exec closure.
*/
static void
fill_exec_closure(struct exec_closure *ec, struct command_status *cstat,
init_exec_closure(struct exec_closure *ec, struct command_status *cstat,
struct command_details *details, const struct user_details *user_details,
struct sudo_event_base *evbase, pid_t sudo_pid, pid_t ppgrp, int backchannel)
pid_t sudo_pid, pid_t ppgrp)
{
debug_decl(fill_exec_closure, SUDO_DEBUG_EXEC);
debug_decl(init_exec_closure, SUDO_DEBUG_EXEC);
/* Fill in the non-event part of the closure. */
ec->sudo_pid = sudo_pid;
@ -976,9 +979,18 @@ fill_exec_closure(struct exec_closure *ec, struct command_status *cstat,
ec->rows = user_details->ts_rows;
ec->cols = user_details->ts_cols;
/* Reset cstat for running the command. */
cstat->type = CMD_INVALID;
cstat->val = 0;
debug_return;
}
/*
* Allocate and set events for the signal pipe and backchannel.
* Forwarded signals on the backchannel are enabled on demand.
*/
static void
init_exec_events(struct exec_closure *ec, struct sudo_event_base *evbase,
int backchannel)
{
debug_decl(init_exec_events, SUDO_DEBUG_EXEC);
/* Setup event base and events. */
ec->evbase = evbase;
@ -1099,23 +1111,22 @@ exec_pty(struct command_details *details,
int sv[2], intercept_sv[2] = { -1, -1 };
struct exec_closure *ec = &pty_ec;
struct plugin_container *plugin;
const pid_t sudo_pid = getpid();
const pid_t ppgrp = getpgrp();
int evloop_retries = -1;
bool cmnd_foreground;
sigset_t set, oset;
struct sigaction sa;
pid_t ppgrp, sudo_pid;
debug_decl(exec_pty, SUDO_DEBUG_EXEC);
/*
* Allocate a pty if sudo is running in a terminal.
* Allocate a pty if sudo is running in a terminal. The exec
* closure must be set for pty_setup() and pty_cleanup_hook().
*/
ec->ptyname = pty_setup(details);
if (ec->ptyname == NULL)
init_exec_closure(ec, cstat, details, user_details, sudo_pid, ppgrp);
if (!pty_setup(ec))
debug_return_bool(false);
/* Register cleanup function */
sudo_fatal_callback_register(pty_cleanup_hook);
/*
* We communicate with the monitor over a bi-directional pair of sockets.
* Parent sends signal info to monitor and monitor sends back wait status.
@ -1161,8 +1172,6 @@ exec_pty(struct command_details *details,
* to and from pty.
*/
init_ttyblock();
ppgrp = getpgrp(); /* parent's pgrp, so child can signal us */
sudo_pid = getpid();
/* Determine whether any of std{in,out,err} should be logged. */
TAILQ_FOREACH(plugin, &io_plugins, entries) {
@ -1400,12 +1409,8 @@ exec_pty(struct command_details *details,
if (ISSET(details->flags, CD_SET_TIMEOUT))
alarm(details->timeout);
/*
* Fill in exec closure, allocate event base, signal events and
* the backchannel event.
*/
fill_exec_closure(ec, cstat, details, user_details, evbase,
sudo_pid, ppgrp, sv[0]);
/* Allocate and set signal events and the backchannel event. */
init_exec_events(ec, evbase, sv[0]);
/* Create event and closure for intercept mode. */
if (ISSET(details->flags, CD_INTERCEPT|CD_LOG_SUBCMDS)) {
@ -1414,6 +1419,10 @@ exec_pty(struct command_details *details,
terminate_command(ec->cmnd_pid, true);
}
/* Reset cstat for running the command. */
cstat->type = CMD_INVALID;
cstat->val = 0;
/* Restore signal mask now that signal handlers are setup. */
sigprocmask(SIG_SETMASK, &oset, NULL);