windows-cell-name-length-consistency-20080806

LICENSE MIT

make all cell name lengths consistent so that safer string copy/cat
functions can be used to prevent buffer overruns

diff --git a/src/WINNT/afsd/afskfw.c b/src/WINNT/afsd/afskfw.c
index c44dc199d04b7e7b7dd8ab08ef660ec6be9ed27d..8b53df9009bd0657f423e35e0b9829aacb4af91f 100644 (file)
--- a/src/WINNT/afsd/afskfw.c
+++ b/src/WINNT/afsd/afskfw.c
@@ -1324,7 +1324,7 @@ KFW_AFS_get_cred( char * username,
     krb5_principal principal = NULL;
     char * pname = NULL;
     krb5_error_code code;
-    char local_cell[MAXCELLCHARS+1];
+    char local_cell[CELL_MAXNAMELEN+1];
     char **cells = NULL;
     int  cell_count=0;
     struct afsconf_cell cellconfig;
@@ -1614,7 +1614,7 @@ KFW_AFS_renew_expiring_tokens(void)
     int cell_count;
     char ** cells=NULL;
     const char * realm = NULL;
-    char local_cell[MAXCELLCHARS+1]="";
+    char local_cell[CELL_MAXNAMELEN+1]="";
     struct afsconf_cell cellconfig;
 
     if (!pkrb5_init_context)
@@ -1742,7 +1742,7 @@ KFW_AFS_renew_token_for_cell(char * cell)
         krb5_ccache                    cc  = 0;
         const char * realm = NULL;
         struct afsconf_cell cellconfig;
-        char local_cell[MAXCELLCHARS+1];
+        char local_cell[CELL_MAXNAMELEN+1];
 
         while ( count-- ) {
             code = pkrb5_parse_name(ctx, principals[count], &princ);
@@ -2658,7 +2658,7 @@ ViceIDToUsername(char *username,
                  struct ktc_principal *aserver, 
                  struct ktc_token *atoken)
 {
-    static char lastcell[MAXCELLCHARS+1] = { 0 };
+    static char lastcell[CELL_MAXNAMELEN+1] = { 0 };
     static char confdir[512] = { 0 };
 #ifdef AFS_ID_TO_NAME
     char username_copy[BUFSIZ];
@@ -2773,8 +2773,8 @@ KFW_AFS_klog(
     struct ktc_principal       aclient;
     char       realm_of_user[REALM_SZ]; /* Kerberos realm of user */
     char       realm_of_cell[REALM_SZ]; /* Kerberos realm of cell */
-    char       local_cell[MAXCELLCHARS+1];
-    char       Dmycell[MAXCELLCHARS+1];
+    char       local_cell[CELL_MAXNAMELEN+1];
+    char       Dmycell[CELL_MAXNAMELEN+1];
     struct ktc_token   atoken;
     struct ktc_token   btoken;
     struct afsconf_cell        ak_cellconfig; /* General information about the cell */
@@ -3364,7 +3364,7 @@ int
 KFW_AFS_get_cellconfig(char *cell, struct afsconf_cell *cellconfig, char *local_cell)
 {
     int        rc;
-    char newcell[MAXCELLCHARS+1];
+    char newcell[CELL_MAXNAMELEN+1];
 
     local_cell[0] = (char)0;
     memset(cellconfig, 0, sizeof(*cellconfig));

diff --git a/src/WINNT/afsd/afskfw.h b/src/WINNT/afsd/afskfw.h
index 1c46bcd303b0b6315d9463316ca448d9cd16d774..d9fb66a9074ae770c4a9c2937db036d7ad7d7717 100644 (file)
--- a/src/WINNT/afsd/afskfw.h
+++ b/src/WINNT/afsd/afskfw.h
@@ -40,7 +40,7 @@ extern "C" {
 #include <cm_config.h>
 #include <rxkad.h>
 
-#define MAXCELLCHARS   64
+#define CELL_MAXNAMELEN 256
 #define MAXHOSTCHARS   64
 #define MAXHOSTSPERCELL 8
 #define TRANSARCAFSDAEMON "TransarcAFSDaemon"

diff --git a/src/WINNT/afsd/cm_cell.c b/src/WINNT/afsd/cm_cell.c
index bd605e3814e4b58234fdc87c1d8bd7b855eac858..c2c3b13371e0f25ff94de677c276046d99f1e8f2 100644 (file)
--- a/src/WINNT/afsd/cm_cell.c
+++ b/src/WINNT/afsd/cm_cell.c
@@ -137,7 +137,7 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, afs_uint32 flags)
 {
     cm_cell_t *cp, *cp2;
     long code;
-    char fullname[200]="";
+    char fullname[CELL_MAXNAMELEN]="";
     int  hasWriteLock = 0;
     afs_uint32 hash;
     cm_cell_rock_t rock;
@@ -158,7 +158,8 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, afs_uint32 flags)
     if (!cp) {
         for (cp = cm_data.allCellsp; cp; cp=cp->allNextp) {
             if (strnicmp(namep, cp->name, strlen(namep)) == 0) {
-                strcpy(fullname, cp->name);
+                strncpy(fullname, cp->name, CELL_MAXNAMELEN);
+                fullname[CELL_MAXNAMELEN-1] = '\0';
                 break;
             }
         }   
@@ -177,7 +178,8 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, afs_uint32 flags)
          */
         for (cp = cm_data.cellNameHashTablep[hash]; cp; cp=cp->nameNextp) {
             if (cm_stricmp_utf8(namep, cp->name) == 0) {
-                strcpy(fullname, cp->name);
+                strncpy(fullname, cp->name, CELL_MAXNAMELEN);
+                fullname[CELL_MAXNAMELEN-1] = '\0';
                 break;
             }
         }   
@@ -187,7 +189,8 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, afs_uint32 flags)
 
         for (cp = cm_data.allCellsp; cp; cp=cp->allNextp) {
             if (strnicmp(namep, cp->name, strlen(namep)) == 0) {
-                strcpy(fullname, cp->name);
+                strncpy(fullname, cp->name, CELL_MAXNAMELEN);
+                fullname[CELL_MAXNAMELEN-1] = '\0';
                 break;
             }
         }   
@@ -289,9 +292,10 @@ cm_cell_t *cm_GetCell_Gen(char *namep, char *newnamep, afs_uint32 flags)
         lock_ReleaseWrite(&cm_cellLock);
     
     /* fullname is not valid if cp == NULL */
-    if (cp && newnamep)
-        strcpy(newnamep, fullname);
-    
+    if (cp && newnamep) {
+        strncpy(newnamep, fullname, CELL_MAXNAMELEN);
+        newnamep[CELL_MAXNAMELEN-1]='\0';
+    }
     return cp;
 }
 
@@ -387,6 +391,7 @@ void cm_InitCell(int newFile, long maxCells)
 
             /* copy in name */
             strncpy(cellp->name, "Freelance.Local.Cell", CELL_MAXNAMELEN); /*safe*/
+            cellp->name[CELL_MAXNAMELEN-1] = '\0';
 
             /* thread on global list */
             cellp->allNextp = cm_data.allCellsp;

diff --git a/src/WINNT/afsd/cm_config.c b/src/WINNT/afsd/cm_config.c
index 4a885a2fcc4eb3d18f69e5e5f72913a9a7dac8ca..6e357e541315940fa6c0589c10179a9b6c976236 100644 (file)
--- a/src/WINNT/afsd/cm_config.c
+++ b/src/WINNT/afsd/cm_config.c
@@ -111,11 +111,13 @@ IsWindowsModule(const char * name)
  * If the caller wants the "real" cell name, it puts a non-null pointer in
  * newCellNamep.  Anomaly:  if cellNamep is ambiguous, we may modify
  * newCellNamep but return an error code.
+ *
+ * newCellNamep is required to be CELL_MAXNAMELEN+1 in size.
  */
 long cm_SearchCellFile(char *cellNamep, char *newCellNamep,
                        cm_configProc_t *procp, void *rockp)
 {
-    char wdir[257];
+    char wdir[MAX_PATH]="";
     FILE *tfilep = NULL, *bestp, *tempp;
     char *tp;
     char lineBuffer[257];
@@ -131,7 +133,8 @@ long cm_SearchCellFile(char *cellNamep, char *newCellNamep,
        return -3;
 
     cm_GetCellServDB(wdir, sizeof(wdir));
-    tfilep = fopen(wdir, "r");
+    if (*wdir)
+        tfilep = fopen(wdir, "r");
 
     if (!tfilep) 
         return -2;
@@ -148,7 +151,7 @@ long cm_SearchCellFile(char *cellNamep, char *newCellNamep,
         tp = fgets(lineBuffer, sizeof(lineBuffer), tfilep);
         if (tracking)
            (void) fgets(lineBuffer, sizeof(lineBuffer), bestp);
-        if (   tp == NULL) {
+        if (tp == NULL) {
            if (feof(tfilep)) {
                /* hit EOF */
                if (partial) {
@@ -193,7 +196,8 @@ long cm_SearchCellFile(char *cellNamep, char *newCellNamep,
             if (stricmp(lineBuffer+1, cellNamep) == 0) {
                /* found the cell we're looking for */
                if (newCellNamep) {
-                   strcpy(newCellNamep, lineBuffer+1);
+                   strncpy(newCellNamep, lineBuffer+1,CELL_MAXNAMELEN+1);
+                    newCellNamep[CELL_MAXNAMELEN] = '\0';
                     strlwr(newCellNamep);
                 }
                 inRightCell = 1;
@@ -211,7 +215,8 @@ long cm_SearchCellFile(char *cellNamep, char *newCellNamep,
                    return -5;
                }
                if (newCellNamep) {
-                   strcpy(newCellNamep, lineBuffer+1);
+                   strncpy(newCellNamep, lineBuffer+1,CELL_MAXNAMELEN+1);
+                    newCellNamep[CELL_MAXNAMELEN] = '\0';
                     strlwr(newCellNamep);
                 }
                inRightCell = 0;
@@ -285,6 +290,7 @@ long cm_SearchCellFile(char *cellNamep, char *newCellNamep,
     return (foundCell) ? 0 : -11;
 }
 
+/* newCellNamep is required to be CELL_MAXNAMELEN+1 in size */
 long cm_SearchCellByDNS(char *cellNamep, char *newCellNamep, int *ttl,
                cm_configProc_t *procp, void *rockp)
 {
@@ -310,7 +316,8 @@ long cm_SearchCellByDNS(char *cellNamep, char *newCellNamep, int *ttl,
             if (procp)
                 (*procp)(rockp, &vlSockAddr, cellHostNames[i]);
             if (newCellNamep) {
-                strcpy(newCellNamep,cellNamep);
+                strncpy(newCellNamep,cellNamep,CELL_MAXNAMELEN+1);
+                newCellNamep[CELL_MAXNAMELEN] = '\0';
                 strlwr(newCellNamep);
             }
         }
@@ -329,19 +336,21 @@ long cm_SearchCellByDNS(char *cellNamep, char *newCellNamep, int *ttl,
  */
 long cm_GetCellServDB(char *cellNamep, afs_uint32 len)
 {
-    int tlen;
+    size_t tlen;
     
     cm_GetConfigDir(cellNamep, len);
 
     /* add trailing backslash, if required */
     tlen = (int)strlen(cellNamep);
-    if (cellNamep[tlen-1] != '\\') {
-        strncat(cellNamep, "\\", len);
+    if (tlen) {
+        if (cellNamep[tlen-1] != '\\') {
+            strncat(cellNamep, "\\", len);
+            cellNamep[len-1] = '\0';
+        }
+        
+        strncat(cellNamep, AFS_CELLSERVDB, len);
         cellNamep[len-1] = '\0';
     }
-        
-    strncat(cellNamep, AFS_CELLSERVDB, len);
-    cellNamep[len-1] = '\0';
     return 0;
 }
 
@@ -368,16 +377,16 @@ long cm_GetRootCellName(char *cellNamep)
 
 cm_configFile_t *cm_CommonOpen(char *namep, char *rwp)
 {
-    char wdir[256];
-    FILE *tfilep;
+    char wdir[MAX_PATH]="";
+    FILE *tfilep = NULL;
 
     cm_GetConfigDir(wdir, sizeof(wdir));
-
-    strncat(wdir, namep, sizeof(wdir));
-    wdir[sizeof(wdir)-1] = '\0';
+    if (*wdir) {
+        strncat(wdir, namep, sizeof(wdir));
+        wdir[sizeof(wdir)-1] = '\0';
         
-    tfilep = fopen(wdir, rwp);
-
+        tfilep = fopen(wdir, rwp);
+    }
     return ((cm_configFile_t *) tfilep);        
 }      
 
@@ -512,8 +521,8 @@ long cm_AppendNewCellLine(cm_configFile_t *filep, char *linep)
 
 long cm_CloseCellFile(cm_configFile_t *filep)
 {
-    char wdir[260];
-    char sdir[260];
+    char wdir[MAX_PATH];
+    char sdir[MAX_PATH];
     long code;
     long closeCode;
     closeCode = fclose((FILE *)filep);

diff --git a/src/WINNT/afsd/cm_dns.c b/src/WINNT/afsd/cm_dns.c
index ee92c6e1fbb9577822803d498930c9623004b548..e23daeb94ea1cbdb109dba01cad30a90c9ac60e3 100644 (file)
--- a/src/WINNT/afsd/cm_dns.c
+++ b/src/WINNT/afsd/cm_dns.c
@@ -533,8 +533,8 @@ void processReplyBuffer_AFSDB(SOCKET commSock, PDNS_HDR replyBuff, int *cellHost
       fprintf(stderr, "processRep_AFSDB: resolved name %s to addr %x\n", hostName, addr);
 #endif /* DEBUG */
       memcpy(&cellHostAddrs[srvCount], &addr.s_addr, sizeof(addr.s_addr));
-         strncpy(cellHostNames[srvCount], hostName, MAXCELLCHARS);
-         cellHostNames[srvCount][MAXCELLCHARS-1] = '\0';
+         strncpy(cellHostNames[srvCount], hostName, CELL_MAXNAMELEN);
+         cellHostNames[srvCount][CELL_MAXNAMELEN-1] = '\0';
       srvCount++;
     }
     else {

diff --git a/src/WINNT/afsd/cm_ioctl.c b/src/WINNT/afsd/cm_ioctl.c
index 3949a129e50a58e3c6388351f60eb91e63f2a9be..5e7cfbd984310451b3a6ceaa59ee03938dd0e2de 100644 (file)
--- a/src/WINNT/afsd/cm_ioctl.c
+++ b/src/WINNT/afsd/cm_ioctl.c
@@ -1758,7 +1758,7 @@ cm_IoctlCreateMountPoint(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_scac
     cm_attr_t tattr;
     clientchar_t *cp;
     fschar_t mpInfo[512];           /* mount point string */
-    fschar_t fullCell[MAXCELLCHARS];
+    fschar_t fullCell[CELL_MAXNAMELEN];
     fschar_t *fscell = NULL;
     fschar_t *fsvolume = NULL;
     clientchar_t volume[VL_MAXNAMELEN];

diff --git a/src/WINNT/afsd/fs.c b/src/WINNT/afsd/fs.c
index 3022855de68ec29d1403f54add8f83075ac59bdb..91a2093046cf856c32102cc2297023b2fd47b0d0 100644 (file)
--- a/src/WINNT/afsd/fs.c
+++ b/src/WINNT/afsd/fs.c
@@ -42,7 +42,7 @@
 #define        MAXSIZE 2048
 #define MAXINSIZE 1300    /* pioctl complains if data is larger than this */
 #define VMSGSIZE 128      /* size of msg buf in volume hdr */
-#define MAXCELLCHARS           64
+#define CELL_MAXNAMELEN                256
 #define MAXHOSTCHARS           64
 
 static char space[MAXSIZE];
@@ -1155,7 +1155,7 @@ GetCell(char *fname, char *cellname)
     struct ViceIoctl blob;
 
     blob.in_size = 0;
-    blob.out_size = MAXCELLCHARS;
+    blob.out_size = CELL_MAXNAMELEN;
     blob.out = cellname;
 
     code = pioctl_utf8(fname, VIOC_FILE_CELL_NAME, &blob, 1);
@@ -1171,7 +1171,7 @@ BadName(char *aname, char *fname)
 {
     afs_int32 tc, code, id;
     char *nm;
-    char cell[MAXCELLCHARS];
+    char cell[CELL_MAXNAMELEN];
     char confDir[257];
 
     for ( nm = aname; tc = *nm; nm++) {
@@ -1604,7 +1604,7 @@ ExamineCmd(struct cmd_syndesc *as, void *arock)
         cm_fid_t fid;
         afs_uint32 filetype;
        afs_uint32 owner[2];
-       char cell[MAXCELLCHARS];
+       char cell[CELL_MAXNAMELEN];
 
         /* once per file */
         memset(&fid, 0, sizeof(fid));
@@ -1632,7 +1632,7 @@ ExamineCmd(struct cmd_syndesc *as, void *arock)
 
         code = pioctl_utf8(ti->data, VIOC_GETFILETYPE, &blob, 1);
 
-        blob.out_size = MAXCELLCHARS;
+        blob.out_size = CELL_MAXNAMELEN;
         blob.out = cell;
 
         code = pioctl_utf8(ti->data, VIOC_FILE_CELL_NAME, &blob, 1);
@@ -2750,7 +2750,7 @@ WhichCellCmd(struct cmd_syndesc *as, void *arock)
     for(ti=as->parms[0].items; ti; ti=ti->next) {
         cm_fid_t fid;
         afs_uint32 filetype;
-       char cell[MAXCELLCHARS];
+       char cell[CELL_MAXNAMELEN];
 
         /* once per file */
         memset(&fid, 0, sizeof(fid));
@@ -2778,7 +2778,7 @@ WhichCellCmd(struct cmd_syndesc *as, void *arock)
 
         code = pioctl_utf8(ti->data, VIOC_GETFILETYPE, &blob, 1);
 
-        blob.out_size = MAXCELLCHARS;
+        blob.out_size = CELL_MAXNAMELEN;
         blob.out = cell;
 
         code = pioctl_utf8(ti->data, VIOC_FILE_CELL_NAME, &blob, 1);