From 255d055e6123bcce76ef9e531a0c4963da4f0fef Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Wed, 6 Aug 2008 06:09:34 +0000 Subject: [PATCH] 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 --- src/WINNT/afsd/afskfw.c | 14 +++++------ src/WINNT/afsd/afskfw.h | 2 +- src/WINNT/afsd/cm_cell.c | 19 ++++++++------ src/WINNT/afsd/cm_config.c | 51 ++++++++++++++++++++++---------------- src/WINNT/afsd/cm_dns.c | 4 +-- src/WINNT/afsd/cm_ioctl.c | 2 +- src/WINNT/afsd/fs.c | 14 +++++------ 7 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/WINNT/afsd/afskfw.c b/src/WINNT/afsd/afskfw.c index c44dc199d..8b53df900 100644 --- 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 1c46bcd30..d9fb66a90 100644 --- a/src/WINNT/afsd/afskfw.h +++ b/src/WINNT/afsd/afskfw.h @@ -40,7 +40,7 @@ extern "C" { #include #include -#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 bd605e381..c2c3b1337 100644 --- 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 4a885a2fc..6e357e541 100644 --- 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 ee92c6e1f..e23daeb94 100644 --- 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 3949a129e..5e7cfbd98 100644 --- 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 3022855de..91a209304 100644 --- 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); -- 2.39.5