add checks for valid user and group names in incoming requests and for data returned from LDAP

git-svn-id: http://arthurdejong.org/svn/nss-pam-ldapd/nss-ldapd@689 ef36b2f9-881f-0410-afb5-c4e39611909c
author: Arthur de Jong <arthur@arthurdejong.org> 2008-04-20 21:50:00 +0200
committer: Arthur de Jong <arthur@arthurdejong.org> 2008-04-20 21:50:00 +0200
commit: 6c2b57e33df882f17fa4cf8dfc6a50f45af7e1c6 (patch)
tree: 0ce72d74e85acfb17f33ce755eb61cf6fa7ab440
parent: dbc26827cca506111622f78cf15e29bf526c3dec (diff)
3 files changed, 155 insertions, 30 deletions
diff --git a/nslcd/common.h b/nslcd/common.h
index c7e3b13..87d6298 100644
--- a/nslcd/common.h
+++ b/nslcd/common.h
@@ -76,6 +76,9 @@ int read_address(TFILE *fp,char *addr,int *addrlen,int *af);
   if (read_address(fp,addr,&(len),&(af))) \
     return -1;
 
+/* checks to see if the specified string is a valid username */
+MUST_USE int isvalidusername(const char *name);
+
 /* transforms the DN info a uid doing an LDAP lookup if needed */
 MUST_USE char *dn2uid(MYLDAP_SESSION *session,const char *dn,char *buf,size_t buflen);
 
diff --git a/nslcd/group.c b/nslcd/group.c
index d8a8d5d..10238e0 100644
--- a/nslcd/group.c
+++ b/nslcd/group.c
@@ -141,9 +141,45 @@ static void group_init(void)
   group_attrs[5]=NULL;
 }
 
+/*
+   Checks to see if the specified name is a valid group name.
+
+   This test is based on the definition from POSIX (IEEE Std 1003.1, 2004,
+   3.189 Group Name and 3.276 Portable Filename Character Set):
+   http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_189
+   http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_276
+
+   The standard defines group names valid if they only contain characters from
+   the set [A-Za-z0-9._-] where the hyphen should not be used as first
+   character.
+*/
+static int isvalidgroupname(const char *name)
+{
+  int i;
+  if ((name==NULL)||(name[0]=='\0'))
+    return 0;
+  /* check first character */
+  if ( ! ( (name[0]>='A' && name[0] <= 'Z') ||
+           (name[0]>='a' && name[0] <= 'a') ||
+           (name[0]>='0' && name[0] <= '9') ||
+           name[0]=='.' || name[0]=='_' ) )
+    return 0;
+  /* check other characters */
+  for (i=1;name[i]!='\0';i++)
+  {
+    if ( ! ( (name[0]>='A' && name[0] <= 'Z') ||
+             (name[0]>='a' && name[0] <= 'a') ||
+             (name[0]>='0' && name[0] <= '9') ||
+             name[0]=='.' || name[0]=='_'  || name[0]=='-') )
+      return 0;
+  }
+  /* no test failed so it must be good */
+  return -1;
+}
+
 static int do_write_group(
-    TFILE *fp,const char **names,gid_t gids[],int numgids,const char *passwd,
-    SET *members)
+    TFILE *fp,MYLDAP_ENTRY *entry,const char **names,gid_t gids[],int numgids,
+    const char *passwd,SET *members)
 {
   int32_t tmpint32;
   int i,j;
@@ -159,21 +195,31 @@ static int do_write_group(
   }
   /* write entries for all names and gids */
   for (i=0;names[i]!=NULL;i++)
-    for (j=0;j<numgids;j++)
+  {
+    if (!isvalidgroupname(names[i]))
+    {
+      log_log(LOG_WARNING,"group entry %s contains invalid group name: \"%s\"",
+                          myldap_get_dn(entry),names[i]);
+    }
+    else
     {
-      WRITE_INT32(fp,NSLCD_RESULT_SUCCESS);
-      WRITE_STRING(fp,names[i]);
-      WRITE_STRING(fp,passwd);
-      WRITE_TYPE(fp,gids[j],gid_t);
-      /* write a list of values */
-      WRITE_INT32(fp,nummembers);
-      if (members!=NULL)
+      for (j=0;j<numgids;j++)
       {
-        set_loop_first(members);
-        while ((tmp=set_loop_next(members))!=NULL)
-          { WRITE_STRING(fp,tmp); }
+        WRITE_INT32(fp,NSLCD_RESULT_SUCCESS);
+        WRITE_STRING(fp,names[i]);
+        WRITE_STRING(fp,passwd);
+        WRITE_TYPE(fp,gids[j],gid_t);
+        /* write a list of values */
+        WRITE_INT32(fp,nummembers);
+        if (members!=NULL)
+        {
+          set_loop_first(members);
+          while ((tmp=set_loop_next(members))!=NULL)
+            { WRITE_STRING(fp,tmp); }
+        }
       }
     }
+  }
   return 0;
 }
 
@@ -193,8 +239,8 @@ static SET *getmembers(MYLDAP_ENTRY *entry,MYLDAP_SESSION *session)
   if (values!=NULL)
     for (i=0;values[i]!=NULL;i++)
     {
-      /* only add non-emtpty values */
-      if (values[i][0]!='\0')
+      /* only add valid usernames */
+      if (isvalidusername(values[i]))
         set_add(set,values[i]);
     }
   /* add the uniqueMember values */
@@ -202,11 +248,11 @@ static SET *getmembers(MYLDAP_ENTRY *entry,MYLDAP_SESSION *session)
   if (values!=NULL)
     for (i=0;values[i]!=NULL;i++)
     {
-      /* transform the DN into a uid */
+      /* transform the DN into a uid (dn2uid() already checks validity) */
       if (dn2uid(session,values[i],buf,sizeof(buf))!=NULL)
         set_add(set,buf);
     }
-  /* terminate the list with an empty string */
+  /* return the members */
   return set;
 }
 
@@ -279,7 +325,7 @@ static int write_group(TFILE *fp,MYLDAP_ENTRY *entry,const char *reqname,
     members=NULL;
   /* write entries (split to a separate function so we can ensure the call
      to free() below in case a write fails) */
-  rc=do_write_group(fp,names,gids,numgids,passwd,members);
+  rc=do_write_group(fp,entry,names,gids,numgids,passwd,members);
   /* free and return */
   if (members!=NULL)
     set_free(members);
@@ -290,7 +336,11 @@ NSLCD_HANDLE(
   group,byname,
   char name[256];
   char filter[1024];
-  READ_STRING_BUF2(fp,name,sizeof(name));,
+  READ_STRING_BUF2(fp,name,sizeof(name));
+  if (!isvalidgroupname(name)) {
+    log_log(LOG_WARNING,"nslcd_group_byname(%s): invalid group name",name);
+    return -1;
+  },
   log_log(LOG_DEBUG,"nslcd_group_byname(%s)",name);,
   NSLCD_ACTION_GROUP_BYNAME,
   mkfilter_group_byname(name,filter,sizeof(filter)),
@@ -312,7 +362,11 @@ NSLCD_HANDLE(
   group,bymember,
   char name[256];
   char filter[1024];
-  READ_STRING_BUF2(fp,name,sizeof(name)),
+  READ_STRING_BUF2(fp,name,sizeof(name));
+  if (!isvalidusername(name)) {
+    log_log(LOG_WARNING,"nslcd_group_bymember(%s): invalid user name",name);
+    return -1;
+  },
   log_log(LOG_DEBUG,"nslcd_group_bymember(%s)",name);,
   NSLCD_ACTION_GROUP_BYMEMBER,
   mkfilter_group_bymember(session,name,filter,sizeof(filter)),
diff --git a/nslcd/passwd.c b/nslcd/passwd.c
index 4e06396..c9baa5d 100644
--- a/nslcd/passwd.c
+++ b/nslcd/passwd.c
@@ -118,6 +118,49 @@ static void passwd_init(void)
   passwd_attrs[9]=NULL;
 }
 
+/*
+   Checks to see if the specified name is a valid user name.
+
+   This test is based on the definition from POSIX (IEEE Std 1003.1, 2004, 3.426 User Name
+   and 3.276 Portable Filename Character Set):
+   http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_426
+   http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap03.html#tag_03_276
+
+   The standard defines user names valid if they contain characters from
+   the set [A-Za-z0-9._-] where the hyphen should not be used as first
+   character. As an extension this test allows the dolar '$' sign as the last
+   character to support Samba special accounts.
+*/
+int isvalidusername(const char *name)
+{
+  int i;
+  if ((name==NULL)||(name[0]=='\0'))
+    return 0;
+  /* check first character */
+  if ( ! ( (name[0]>='A' && name[0] <= 'Z') ||
+           (name[0]>='a' && name[0] <= 'a') ||
+           (name[0]>='0' && name[0] <= '9') ||
+           name[0]=='.' || name[0]=='_' ) )
+    return 0;
+  /* check other characters */
+  for (i=1;name[i]!='\0';i++)
+  {
+    if ( name[i]=='$' )
+    {
+      /* if the char is $ we require it to be the last char */
+      if (name[i+1]!='\0')
+        return 0;
+    }
+    else if ( ! ( (name[0]>='A' && name[0] <= 'Z') ||
+                  (name[0]>='a' && name[0] <= 'a') ||
+                  (name[0]>='0' && name[0] <= '9') ||
+                  name[0]=='.' || name[0]=='_'  || name[0]=='-') )
+      return 0;
+  }
+  /* no test failed so it must be good */
+  return -1;
+}
+
 char *dn2uid(MYLDAP_SESSION *session,const char *dn,char *buf,size_t buflen)
 {
   MYLDAP_SEARCH *search;
@@ -133,7 +176,12 @@ char *dn2uid(MYLDAP_SESSION *session,const char *dn,char *buf,size_t buflen)
   attrs[1]=NULL;
   /* try to look up uid within DN string */
   if (myldap_cpy_rdn_value(dn,attmap_passwd_uid,buf,buflen)!=NULL)
+  {
+    /* check if it is valid */
+    if (!isvalidusername(buf))
+      return NULL;
     return buf;
+  }
   /* we have to look up the entry */
   search=myldap_search(session,dn,LDAP_SCOPE_BASE,passwd_filter,attrs);
   if (search==NULL)
@@ -160,6 +208,9 @@ char *dn2uid(MYLDAP_SESSION *session,const char *dn,char *buf,size_t buflen)
   else
     buf=NULL;
   myldap_search_close(search);
+  /* check username */
+  if ((buf!=NULL)&&!isvalidusername(buf))
+    return NULL;
   return buf;
 }
 
@@ -171,6 +222,9 @@ char *uid2dn(MYLDAP_SESSION *session,const char *uid,char *buf,size_t buflen)
   int rc;
   const char *dn;
   char filter[1024];
+  /* if it isn't a valid username, just bail out now */
+  if (!isvalidusername(uid))
+    return NULL;
   /* set up attributes (we don't care, we just want the DN) */
   attrs[0]=NULL;
   /* initialize default base, scrope, etc */
@@ -347,17 +401,27 @@ static int write_passwd(TFILE *fp,MYLDAP_ENTRY *entry,const char *requser,
   }
   /* write the entries */
   for (i=0;usernames[i]!=NULL;i++)
-    for (j=0;j<numuids;j++)
+  {
+    if (!isvalidusername(usernames[i]))
     {
-      WRITE_INT32(fp,NSLCD_RESULT_SUCCESS);
-      WRITE_STRING(fp,usernames[i]);
-      WRITE_STRING(fp,passwd);
-      WRITE_TYPE(fp,uids[j],uid_t);
-      WRITE_TYPE(fp,gid,gid_t);
-      WRITE_STRING(fp,gecos);
-      WRITE_STRING(fp,homedir);
-      WRITE_STRING(fp,shell);
+      log_log(LOG_WARNING,"passwd entry %s contains invalid user name: \"%s\"",
+                          myldap_get_dn(entry),usernames[i]);
     }
+    else
+    {
+      for (j=0;j<numuids;j++)
+      {
+        WRITE_INT32(fp,NSLCD_RESULT_SUCCESS);
+        WRITE_STRING(fp,usernames[i]);
+        WRITE_STRING(fp,passwd);
+        WRITE_TYPE(fp,uids[j],uid_t);
+        WRITE_TYPE(fp,gid,gid_t);
+        WRITE_STRING(fp,gecos);
+        WRITE_STRING(fp,homedir);
+        WRITE_STRING(fp,shell);
+      }
+    }
+  }
   return 0;
 }
 
@@ -365,7 +429,11 @@ NSLCD_HANDLE(
   passwd,byname,
   char name[256];
   char filter[1024];
-  READ_STRING_BUF2(fp,name,sizeof(name));,
+  READ_STRING_BUF2(fp,name,sizeof(name));
+  if (!isvalidusername(name)) {
+    log_log(LOG_WARNING,"nslcd_passwd_byname(%s): invalid user name",name);
+    return -1;
+  },
   log_log(LOG_DEBUG,"nslcd_passwd_byname(%s)",name);,
   NSLCD_ACTION_PASSWD_BYNAME,
   mkfilter_passwd_byname(name,filter,sizeof(filter)),
author	Arthur de Jong <arthur@arthurdejong.org>	2008-04-20 21:50:00 +0200
committer	Arthur de Jong <arthur@arthurdejong.org>	2008-04-20 21:50:00 +0200
commit	6c2b57e33df882f17fa4cf8dfc6a50f45af7e1c6 (patch)
tree	0ce72d74e85acfb17f33ce755eb61cf6fa7ab440
parent	dbc26827cca506111622f78cf15e29bf526c3dec (diff)