Re: pkg/32988: Bug in databases/nss_ldap

[Matthias Drochner <M.Drochner%fz-juelich.de@localhost>]

The following reply was made to PR pkg/32988; it has been noted by GNATS.

From: Matthias Drochner <M.Drochner%fz-juelich.de@localhost>
To: thesing%cs.uni-sb.de@localhost
Cc: gnats-bugs%netbsd.org@localhost
Subject: Re: pkg/32988: Bug in databases/nss_ldap
Date: Wed, 15 Mar 2006 15:39:56 +0100

 This is a multipart MIME message.
 
 --==_Exmh_12444497264550
 Content-Type: text/plain; charset=us-ascii
 
 
 > fails to copy the already present gid_t's
 
 Ah yes, sorry. Never tested this, obviously...
 
 > always puts the primary gid_t of the user in slot 0
 
 This shouldn't be too bad - every getgroupmembership
 backend puts it into the first slot. This means: if
 there is anything in the array before, the primary
 group is already there -- there is also no strict
 need to check for duplicates.
 What should be done is to check all added memberships
 for duplicates. At least the implementations in libc
 do so.
 
 So I've aligned the implementation more to the
 backends in libc. Can you give the appended patch a try?
 (Can't test myself atm. I've managed to corrupt my
 LDAP database.)
 
 best regards
 Matthias
 
 
 
 --==_Exmh_12444497264550
 Content-Type: text/plain ; name="nssldap.txt"; charset=us-ascii
 Content-Description: nssldap.txt
 Content-Disposition: attachment; filename="nssldap.txt"
 
 diff -r 5efa09c5b5c9 databases/nss_ldap/files/netbsd.c
 --- a/databases/nss_ldap/files/netbsd.c        Wed Mar 15 12:37:15 2006 +0100
 +++ b/databases/nss_ldap/files/netbsd.c        Wed Mar 15 15:21:18 2006 +0100
 @@ -5,6 +5,7 @@
  #include <grp.h>
  #include <nsswitch.h>
  #include <stdarg.h>
 +#include <stdlib.h>
  #include <string.h>
  
  #include "netbsd.h"
 @@ -440,6 +441,28 @@ netbsd_getgrgid_r(void *rv, void *cb_dat
        return nss2netbsderr[s];
  }
  
 +/* addgid helper from NetBSD's getgroupmembership.c */
 +static int
 +__gr_addgid(gid_t gid, gid_t *groups, int maxgrp, int *groupc)
 +{
 +      int     ret, dupc;
 +
 +                                              /* skip duplicates */
 +      for (dupc = 0; dupc < MIN(maxgrp, *groupc); dupc++) {
 +              if (groups[dupc] == gid)
 +                      return 1;
 +      }
 +
 +      ret = 1;
 +      if (*groupc < maxgrp)                   /* add this gid */
 +              groups[*groupc] = gid;
 +      else
 +              ret = 0;
 +      (*groupc)++;
 +      return ret;
 +}
 +
 +
  static int
  netbsd_getgroupmembership(void *rv, void *cb_data, va_list ap)
  {
 @@ -453,32 +476,24 @@ netbsd_getgroupmembership(void *rv, void
        int *size = va_arg(ap, int*);
        gid_t *tmpgroups;
        long int lstart, lsize;
 -      int origsize = *size;
 +      int i;
  
        tmpgroups = malloc(limit * sizeof(gid_t));
        if (!tmpgroups)
                return NS_TRYAGAIN;
        /* insert primary membership */
 -      if (*size < limit) {
 -              tmpgroups[0] = group;
 -              (*size)++;
 -      }
 -      lstart = *size;
 +      __gr_addgid(group, groups, limit, size);
 +
 +      lstart = 0;
        lsize = limit;
        s = _nss_ldap_initgroups_dyn(user, group, &lstart, &lsize,
                &tmpgroups, 0, &err);
        if (s == NSS_STATUS_SUCCESS) {
 -              if (lstart > limit) {
 -                      memcpy(groups, tmpgroups, limit * sizeof(gid_t));
 -                      *retval = -1;
 -              } else {
 -                      memcpy(groups, tmpgroups, lstart * sizeof(gid_t));
 -                      *retval = 0;
 -              }
 -              *size = lstart;
 +              for (i = 0; i < lstart; i++)
 +                      if (! __gr_addgid(tmpgroups[i], groups, limit, size))
 +                              *retval = -1;
                s = NSS_STATUS_NOTFOUND;
 -      } else
 -              *size = origsize;
 +      }
        free(tmpgroups);
                
        return nss2netbsderr[s];
 
 --==_Exmh_12444497264550--