2

I'm trying to find all groups to which user belongs in my UNIX system, and that for each user.Implementation has to be in C. Here is my code:

#include <stdio.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <fcntl.h>
#include <assert.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>
#include <pwd.h>
#include <grp.h>


static void error_fatal(char* msg) 
{ perror(msg); exit(EXIT_FAILURE); }

int main(int argc, char** argv) {

    struct group* grp;
    struct passwd* pwd;
    char *name;
    int i = 0;

    setpwent();

    while((pwd = getpwent()) != NULL){

        if(  ( name = (char*) malloc( (strlen(pwd->pw_name)+1)*sizeof(char))) == NULL  ) error_fatal("malloc");
        strcpy(name, pwd->pw_name);
        printf("%s:\n", name); 

        setgrent();
        while( (grp = getgrent()) != NULL ) {
            for( i=0; i < (sizeof(grp->gr_mem)/sizeof(grp->gr_mem[0])); i++ ){
                if( /*strlen(&grp->gr_mem[i][0]) == strlen(name) && */ !strcmp(grp->gr_mem[i], name) )
                     printf("%s\n", name);
}                           }

        endgrent(); 
        free(name);

}
    endpwent();

    return 0;
}

But I get segmentation fault after "root:" output. I'm pretty sure the problem is in accessing list of members in the fourth field of /etc/group file (see man 5 group for details).

So, basically my problem would be to find out how many members each group has, so my counter(i in program, the last for loop) would have nice upper boundary.

mk1024
  • 119
  • 3

2 Answers2

1

Your problem is here:

for( i=0; i < (sizeof(grp->gr_mem)/sizeof(grp->gr_mem[0])); i++ ){

struct group is defined as:

       struct group {
           char   *gr_name;        /* group name */
           char   *gr_passwd;      /* group password */
           gid_t   gr_gid;         /* group ID */
           char  **gr_mem;         /* NULL-terminated array of pointers
                                      to names of group members */
       };

You're assuming gr_mem is an array but it is not. It is a pointer pointing to the first element of an array. So sizeof(grp->gr_mem)/sizeof(grp->gr_mem[0]) gives you the size of a pointer, probably 8 on your system. So if a user has less than 8 groups, you'll end up reading past the end of the array gr_mem points to the start of.

Because the array pointed to by gr_mem is NULL terminated, finding that terminator tells you when the loop is done:

for( i=0; grp->gr_mem[i]; i++ ){
dbush
  • 205,898
  • 23
  • 218
  • 273
  • Thank you very much! Also, instead of my last printf: printf("%s\n", name); there should be printf("%s\n", grp->gr_name); – mk1024 Dec 07 '18 at 20:00
  • @mk1024 glad I could help. Feel free to [accept this answer](https://stackoverflow.com/help/accepted-answer) if you found it useful. – dbush Dec 07 '18 at 20:08
0

Running your code I detected your issue is not verifying grp->gr_mem[i] == NULL before using it in strcmp call:

if (grp->gr_mem[i] == NULL)
  continue;

Adding those lines before strcmp call worked for me.

Also, don't forget to free the memory you are using. I don't know if this is your complete code, but here you should considering using free(name) within your while loop.

delirium
  • 868
  • 6
  • 20
  • As far as I know, getgrent and getpwent returns pointer to next struct group/struct passwd read from /etc/group or /etc/passwd, so I don't see any memory leaks here. – mk1024 Dec 07 '18 at 20:28
  • You are allocating memory for `name` inside your `if` condition. That's why you should use `free(name)`at the end of your outer `while` loop. Always free your memory. :) – delirium Dec 10 '18 at 15:05
  • Sure. Also, don't forget to approve @dbush answer. He definitely put some time to help you out - and it solved your problem. – delirium Dec 11 '18 at 12:04
  • Of corse I will, as soon as I get enough reputation to be able to approve. – mk1024 Dec 11 '18 at 17:11