Some thoughts about security in correct_password.c

[Tito]

Hi,
these are just some thoughts about a possible security enhancement in correct_passwd.c.
Maybe it could be a good idea to drop privileges in it if:
a) support for shadow passwords is not enabled
b) or just after the shadow password stuff is done.
Like this:

diff -uN libbb/correct_passwor_orig.c libbb/correct_password.c
--- libbb/correct_password_orig.c       2005-12-04 13:55:50.000000000 +0100
+++ libbb/correct_password.c    2005-12-23 14:33:33.000000000 +0100
@@ -52,6 +52,7 @@

 #ifdef CONFIG_FEATURE_SHADOWPASSWDS
        if (( strcmp ( pw-> pw_passwd, "x" ) == 0 ) || ( strcmp ( pw-> pw_passwd, "*" ) == 0 )) {
+               seteuid(0);
                struct spwd *sp = getspnam ( pw-> pw_name );

                if ( !sp )
@@ -62,7 +63,7 @@
        else
 #endif
        correct = pw-> pw_passwd;
-
+       seteuid(pw->pw_uid);
        if ( correct == 0 || correct[0] == '\0' )
                return 1;

-------------------------------------------------------------------------------------------------------------------------------

This libbb function is used only by su, login and vlock.
I've tested them all, but only with shadow passwords enabled, and they all seem to work
correctly.
The only minor change needed is in login.c to make the change_identity() call work:

diff -uN loginutils/login_orig.c loginutils/login.c
--- loginutils/login_orig.c     2005-12-04 13:58:13.000000000 +0100
+++ loginutils/login.c  2005-12-23 14:24:40.000000000 +0100
@@ -270,8 +270,9 @@
         * (for example when the root fs is read only) */
        chown ( full_tty, pw-> pw_uid, pw-> pw_gid );
        chmod ( full_tty, 0600 );
-
+       seteuid(0);
        change_identity ( pw );
+       seteuid(pw->pw_uid);
        tmp = pw-> pw_shell;
        if(!tmp || !*tmp)
                tmp = DEFAULT_SHELL;
-----------------------------------------------------------------------------------------------------------------------------

I don't know if the possible enhancement in security is wort the effort,
so this is mostly a call for advice to more experienced programmers.
BTW, the man page for seteuid states:

NOTES
       Setting the effective user (group) ID to the saved user (group)  ID  is
       possible  since  Linux  1.1.37  (1.1.38).   On  an arbitrary system one
       should check _POSIX_SAVED_IDS.

       Under  libc4,  libc5  and  glibc2.0  seteuid(euid)  is  equivalent   to
       setreuid(-1,  euid)  and  hence  may  change  the saved user ID.  Under
       glibc2.1 it is equivalent to setresuid(-1, euid,-1) and hence does  not
       change the saved user ID.  Similar remarks hold for setegid.

CONFORMING TO
       BSD 4.3

So there could be some problems with different versions of libc???

Ciao,
Tito