isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Call closefrom() before we change to a non-root UID.

Browse Source 
This prevents another process from changing the NOFILE resource limit
of the child process and defeating the closefrom() call.
Reported by Joe Vennix from Apple Information Security.
This commit is contained in:
Todd C. Miller
2019-11-02 10:51:49 -06:00
parent fd9fa6bcaa
commit 7acbfc18a9
2 changed files with 32 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
src/exec.c
View File

@@ -56,13 +56,38 @@
#include "sudo_plugin.h"
#include "sudo_plugin_int.h"
static void
close_fds(struct command_details *details, int errfd)
{
int fd, maxfd;
unsigned char *debug_fds;
debug_decl(close_fds, SUDO_DEBUG_EXEC)
if (details->closefrom < 0)
debug_return;
/* Preserve debug fds and error pipe as needed. */
maxfd = sudo_debug_get_fds(&debug_fds);
for (fd = 0; fd <= maxfd; fd++) {
if (sudo_isset(debug_fds, fd))
add_preserved_fd(&details->preserved_fds, fd);
}
if (errfd != -1)
add_preserved_fd(&details->preserved_fds, errfd);
/* Close all fds except those explicitly preserved. */
closefrom_except(details->closefrom, &details->preserved_fds);
debug_return;
}
/*
* Setup the execution environment immediately prior to the call to execve().
* Group setup is performed by policy_init_session(), called earlier.
* Returns true on success and false on failure.
*/
static bool
exec_setup(struct command_details *details)
exec_setup(struct command_details *details, int errfd)
{
bool ret = false;
debug_decl(exec_setup, SUDO_DEBUG_EXEC)
@@ -145,6 +170,9 @@ exec_setup(struct command_details *details)
if (ISSET(details->flags, CD_OVERRIDE_UMASK))
(void) umask(details->umask);
/* Close fds before chroot (need /dev) or uid change (prlimit on Linux). */
close_fds(details, errfd);
if (details->chroot) {
if (chroot(details->chroot) != 0 || chdir("/") != 0) {
sudo_warn(U_("unable to change root to %s"), details->chroot);
@@ -215,24 +243,8 @@ exec_cmnd(struct command_details *details, int errfd)
debug_decl(exec_cmnd, SUDO_DEBUG_EXEC)
restore_signals();
if (exec_setup(details) == true) {
if (exec_setup(details, errfd) == true) {
/* headed for execve() */
if (details->closefrom >= 0) {
int fd, maxfd;
unsigned char *debug_fds;
/* Preserve debug fds and error pipe as needed. */
maxfd = sudo_debug_get_fds(&debug_fds);
for (fd = 0; fd <= maxfd; fd++) {
if (sudo_isset(debug_fds, fd))
add_preserved_fd(&details->preserved_fds, fd);
}
if (errfd != -1)
add_preserved_fd(&details->preserved_fds, errfd);
/* Close all fds except those explicitly preserved. */
closefrom_except(details->closefrom, &details->preserved_fds);
}
#ifdef HAVE_SELINUX
if (ISSET(details->flags, CD_RBAC_ENABLED)) {
selinux_execve(details->execfd, details->command, details->argv,

3
src/tgetpass.c
View File

@@ -318,6 +318,8 @@ sudo_askpass(const char *askpass, const char *prompt)
}
if (setuid(ROOT_UID) == -1)
sudo_warn("setuid(%d)", ROOT_UID);
/* Close fds before uid change to prevent prlimit sabotage on Linux. */
closefrom(STDERR_FILENO + 1);
if (setgid(user_details.gid)) {
sudo_warn(U_("unable to set gid to %u"), (unsigned int)user_details.gid);
_exit(255);
@@ -326,7 +328,6 @@ sudo_askpass(const char *askpass, const char *prompt)
sudo_warn(U_("unable to set uid to %u"), (unsigned int)user_details.uid);
_exit(255);
}
closefrom(STDERR_FILENO + 1);
execl(askpass, askpass, prompt, (char *)NULL);
sudo_warn(U_("unable to run %s"), askpass);
_exit(255);