On Saturday 06 November 2010 22:16:42 you wrote: > On Thu, Oct 21, 2010 at 3:03 PM, Tito <[email protected]> wrote: > >> > Can at least > >> > if (geteuid()) > >> > > >> > bb_error_msg_and_die(bb_msg_perm_denied_are_you_root); > >> > be moved out of this switch statement from hell, > >> > to make it a bit easier to grok? > >> > >> No, it cannot be moved outside because it must be triggered when > >> doing deluser or delgroup (argc = 2) > >> and when doing deluser from group (argc = 3) > >> if uid != 0 > > There is no execution path which skips geteuid() check. > Therefore, you may just move check up, before switch.
Hi, wanted bb_show_usage to be triggered before bb_error_msg_and_die(bb_msg_perm_denied_are_you_root) but it is not that important. > I did ask for more comments, and now you are overdoing it: sorry about that. snip > > > Please review attached edited version. The logic should be the same as yours. > I only flagged possible bugs in logic, and changed comments. > > Applied to git, thanks. > /BUG: check should be done by GID, not by matching name! //1. find GROUP's GID //2. check that /etc/passwd doesn't have lines of the form // user:pwd:uid:GID:... //3. bail out if at least one such line exists I think the check should not be done by gid as at creation time you could add the user to an existing group (eventually with no other occurences in /etc/passwd): >adduser prova --ingroup users >Adding user `prova' ... >Adding new user `prova' (1004) with group `users' ... >Creating home directory `/home/prova' ... that you don't want to be removed at user deletion time. OTOH man adduser says: " By default, each user in Debian GNU/Linux is given a corresponding group with the same name." so checking by name seems the right thing to do, but maybe I'm missing something obvious (non Debian systems behave differently?) This could be demostrated by a little experiment: >adduser prova >Adding user `prova' ... >Adding new group `prova' (1006) ... >Adding new user `prova' (1004) with group `prova' ... now rename prova to aprova in /etc/group and /etc/gshadow and >deluser prova >Removing user `prova' ... >Warning: group `aprova' has no more members. >Done. and >grep aprova /etc/group >aprova:x:1006: so the group is removed by name. The pid is checked only to see if there are other users using the same group. Infact without changing the name the group is removed: >adduser prova >Adding user `prova' ... >Adding new group `prova' (1006) ... >Adding new user `prova' (1004) with group `prova' ... >deluser prova >Removing user `prova' ... >Warning: group `prova' has no more members. >Done. >grep prova /etc/group Ciao, Tito P.S.: thanks for you time and effort in reviewing my patches _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
