[PATCH] su code clean up and new config option V.2

Tito farmatito at tiscali.it
Mon Jul 10 00:08:55 PDT 2006


On Monday 10 July 2006 05:06, Rob Landley wrote:
> On Sunday 09 July 2006 1:28 pm, Tito wrote:
> > On Sunday 9 July 2006 15:51, Tito wrote:
> >  Hi,
> >
> >  this is an improved version of the previous patch:
> > >1) removes an a lot of #ifdef's from su.c
> > >2) adds a new config option to enable/disable syslogging in su
> > >	(it now loggs successes and failures!!!)
> >
> > 3)  size is about the same as before with syslogging enabled.
> > 4) fixes a potential issue with struct passwd *pw making old_user =
> > opt_username  (new_user)
> 
> Applied, with further cleanups.
> 
> What's the issue with #4?

If getlogin fails we use getpwuid  to initialise old_user

        old_user = getlogin ( );
        if ( !old_user ) 
      { 
                  pw = getpwuid ( cur_uid ); 	 
                 old_user = ( pw ? pw->pw_name : "" ); 	 
      }

later we reuse pw

   pw = getpwnam ( opt_username );


so in the end we have 

old_user = pw->pw_name;
opt_username == pw->pw_name;

as the man page says (GETPWNAM(3) ):


       The return value may point to static area, and may  be  overwritten  by
       subsequent calls to getpwent(), getpwnam(), or getpwuid().

so potentially we could have:

old_user == opt_username

this would mess up the logs.
That is why I strdup the first pw->pw_name.

BTW, I was not able to trigger this on my system....
so it is just to be sure.

Ciao,
Tito
> Rob


More information about the busybox mailing list