set*id() return values not checked in pop3d and comsatd

[Jeffrey]

Hi,

Several set*id() family function return values are not checked in pop3d and
comsatd code:

pop3d/user.c:

     21 int
     22 pop3d_begin_session ()
     23 {
    [...]
     39
     40   if (auth_data->change_uid)
     41     setuid (auth_data->uid);

pop3d/popauth.c:

    469 int
    470 action_create (void)
    471 {
    [...]
    479
    480   /* Make sure we have proper privileges if popauth is setuid */
    481   setuid (getuid ());

comsat/comsat.c:

    526 int
    527 change_user (const char *user)
    528 {
    529   struct passwd *pw;
    530
    531   pw = getpwnam (user);
    532   if (!pw)
    533     {
    534       mu_diag_output (MU_DIAG_CRIT, _("no such user: %s"), user);
    535       return 1;
    536     }
    537
    538   setgid (pw->pw_gid);
    539   setuid (pw->pw_uid);
    540   chdir (pw->pw_dir);
    541   username = user;
    542   return 0;
    543 }

There are cases where set*id() functions can fail.

man 2 setuid():

RETURN VALUE
       On success, zero is returned.  On error, -1 is returned, and errno
is set to indicate the error.

       Note: there are cases where setuid() can fail even when the caller
is UID 0; it is a grave security error to omit checking for a failure
return from setuid().


The above code could be abused in different ways to trigger such failures,
potentially remotely in the case of pop3d and comsatd. That could lead to
either privilege
escalations or user impersonation depending on the occurrence.

I believe the attached patch mitigates the issue, let me know if that suits
you.

Regards,

-- 
Jeffrey BENCTEUX

0001-pop3-comsat-fix-missing-checks-for-set-id-return-val.patch
Description: Binary data