Skip to content

Commit

Permalink
imp: run: don't drop real user/group ids in parent
Browse files Browse the repository at this point in the history
Problem: The IMP run implementation sets the real user and group id
of the process to the effective user and group id in the privileged
parent, but this makes it so that the invoking user can no longer
deliver signals to the IMP parent process.

Move the setuid()/setgid() calls to the child process just before
execve(2) is called. The parent IMP thereby maintains the real uid/gid
of the invoking user and can handle forwarding of signals from that
user to the invoked run command.

Since the parent IMP process no longer has a real userid of 0/root,
update the call that obtains the userid for setting USER and HOME
to use the effective uid.
  • Loading branch information
grondo committed Nov 2, 2024
1 parent f09b463 commit e4df56d
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/imp/run.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ imp_run (struct imp_state *imp,
imp_die (1, "run %s: relative path not allowed", name);

/* Get passwd entry for current user to set HOME and USER */
if (!(pwd = getpwuid (getuid ())))
if (!(pwd = getpwuid (geteuid ())))
imp_die (1, "run: failed to find target user");

/* Set HOME and USER */
Expand Down Expand Up @@ -199,6 +199,10 @@ imp_run (struct imp_state *imp,
/* unblock all signals */
imp_sigunblock_all ();

if (setuid (geteuid()) < 0
|| setgid (getegid()) < 0)
imp_die (1, "setuid: %s", strerror (errno));

args[0] = path;
args[1] = NULL;
#if CODE_COVERAGE_ENABLED
Expand Down Expand Up @@ -271,10 +275,6 @@ int imp_run_privileged (struct imp_state *imp,
if (!kv_env)
imp_die (1, "run: error processing command environment");

if (setuid (geteuid()) < 0
|| setgid (getegid()) < 0)
imp_die (1, "setuid: %s", strerror (errno));

imp_run (imp, name, cf_run, kv_env);

return 0;
Expand Down

0 comments on commit e4df56d

Please sign in to comment.