openssh unix dev - Jan 2005 - Question about a recent change to uidswap.c in the portability snapshot

A change was recently introduced into uidswap.c to cover the case where
the user is root.  The change is "&& pw->pw_uid != 0
&&".
 
        /* Try restoration of GID if changed (test clearing of saved
gid) */
        if (old_gid != pw->pw_gid && pw->pw_uid != 0 &&
            (setgid(old_gid) != -1 || setegid(old_gid) != -1))
                fatal("%s: was able to restore old [e]gid", __func__);

My question is, should this change also be included in the setuid() call
a few lines later?
 
...
 
        /* Try restoration of UID if changed (test clearing of saved
uid) */
        if (old_uid != pw->pw_uid &&   [should change be here also?]
            (setuid(old_uid) != -1 || seteuid(old_uid) != -1))
                fatal("%s: was able to restore old [e]uid", __func__);


David Holmes 
F5 Networks 
d.holmes at f5.com

David Holmes wrote:
> A change was recently introduced into uidswap.c to cover the case where
> the user is root.  The change is "&& pw->pw_uid != 0
&&".
>  
>         /* Try restoration of GID if changed (test clearing of saved
> gid) */
>         if (old_gid != pw->pw_gid && pw->pw_uid != 0
&&
>             (setgid(old_gid) != -1 || setegid(old_gid) != -1))
>                 fatal("%s: was able to restore old [e]gid",
__func__);
> 
> My question is, should this change also be included in the setuid() call
> a few lines later?
>  
>         /* Try restoration of UID if changed (test clearing of saved
> uid) */
>         if (old_uid != pw->pw_uid &&   [should change be here
also?]
>             (setuid(old_uid) != -1 || seteuid(old_uid) != -1))
>                 fatal("%s: was able to restore old [e]uid",
__func__);
I don't think it's necessary.

The gid test was put in to handle the case where the root user is 
running as their non-default group (ie they've run "newgrp").  In
that
case, because root can set its uid to whatever it wants, the group 
restore test will fail.  For the second test, old_uid and pw_uid will be 
the same so the second test won't fail either.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
     Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.