From d7ebb697457fc4b8562bb1475a6832f1badb15f8 Mon Sep 17 00:00:00 2001 From: Prasanna Kumar Kalever Date: Mon, 29 Jan 2018 17:02:07 +0530 Subject: replace strcat and strcpy with their secure versions Change-Id: If98ce7b7e50901ee130bbe190a12664ec0adb8c2 Signed-off-by: Prasanna Kumar Kalever --- cli/gluster-block.c | 41 +++++++++++++------------ daemon/gluster-blockd.c | 2 +- rpc/block_svc_routines.c | 78 ++++++++++++++++++++++++------------------------ rpc/glfs-operations.c | 22 +++++++------- utils/capabilities.c | 2 +- utils/lru.c | 4 +-- utils/utils.c | 20 +++++++++++++ utils/utils.h | 10 +++++++ 8 files changed, 106 insertions(+), 73 deletions(-) diff --git a/cli/gluster-block.c b/cli/gluster-block.c index 98f5bba..33d4003 100644 --- a/cli/gluster-block.c +++ b/cli/gluster-block.c @@ -81,7 +81,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt) } saun.sun_family = AF_UNIX; - strcpy(saun.sun_path, GB_UNIX_ADDRESS); + GB_STRCPYSTATIC(saun.sun_path, GB_UNIX_ADDRESS); if (connect(sockfd, (struct sockaddr *) &saun, sizeof(struct sockaddr_un)) < 0) { @@ -235,7 +235,7 @@ static bool glusterBlockIsNameAcceptable(char *name) { int i = 0; - if (!name || strlen(name) == 0 || strlen(name) > 255) + if (!name || strlen(name) >= 255) return FALSE; for (i = 0; i < strlen(name); i++) { if (!isalnum(name[i]) && (name[i] != '_') && (name[i] != '-')) @@ -259,6 +259,7 @@ glusterBlockIsAddrAcceptable(char *addr) static int glusterBlockParseVolumeBlock(char *volumeblock, char *volume, char *block, + size_t vol_len, size_t block_len, char *helpstr, char *op) { int ret = -1; @@ -292,8 +293,8 @@ glusterBlockParseVolumeBlock(char *volumeblock, char *volume, char *block, "and should be less than 255 characters long\n", block); goto out; } - strncpy(volume, tmp, strlen(tmp)); - strncpy(block, sep+1, strlen(sep+1)); + GB_STRCPY(volume, tmp, vol_len); + GB_STRCPY(block, sep+1, block_len); ret = 0; out: @@ -314,7 +315,8 @@ glusterBlockModify(int argcount, char **options, int json) mobj.json_resp = json; if (glusterBlockParseVolumeBlock (options[optind++], mobj.volume, - mobj.block_name, GB_MODIFY_HELP_STR, + mobj.block_name, sizeof(mobj.volume), + sizeof(mobj.block_name), GB_MODIFY_HELP_STR, "modify")) { goto out; } @@ -365,9 +367,9 @@ glusterBlockCreate(int argcount, char **options, int json) /* default mpath */ cobj.mpath = 1; - if (glusterBlockParseVolumeBlock (options[optind++], cobj.volume, - cobj.block_name, GB_CREATE_HELP_STR, - "create")) { + if (glusterBlockParseVolumeBlock(options[optind++], cobj.volume, cobj.block_name, + sizeof(cobj.volume), sizeof(cobj.block_name), + GB_CREATE_HELP_STR, "create")) { goto out; } @@ -470,7 +472,7 @@ glusterBlockList(int argcount, char **options, int json) GB_ARGCHECK_OR_RETURN(argcount, 3, "list", GB_LIST_HELP_STR); cobj.json_resp = json; - strcpy(cobj.volume, options[2]); + GB_STRCPYSTATIC(cobj.volume, options[2]); ret = glusterBlockCliRPC_1(&cobj, LIST_CLI); if (ret) { @@ -505,9 +507,9 @@ glusterBlockDelete(int argcount, char **options, int json) cobj.json_resp = json; - if (glusterBlockParseVolumeBlock (options[2], cobj.volume, - cobj.block_name, GB_DELETE_HELP_STR, - "delete")) { + if (glusterBlockParseVolumeBlock (options[2], cobj.volume, cobj.block_name, + sizeof(cobj.volume), sizeof(cobj.block_name), + GB_DELETE_HELP_STR, "delete")) { goto out; } @@ -533,9 +535,9 @@ glusterBlockInfo(int argcount, char **options, int json) GB_ARGCHECK_OR_RETURN(argcount, 3, "info", GB_INFO_HELP_STR); cobj.json_resp = json; - if (glusterBlockParseVolumeBlock (options[2], cobj.volume, - cobj.block_name, GB_INFO_HELP_STR, - "info")) { + if (glusterBlockParseVolumeBlock (options[2], cobj.volume, cobj.block_name, + sizeof(cobj.volume), sizeof(cobj.block_name), + GB_INFO_HELP_STR, "info")) { goto out; } @@ -567,7 +569,8 @@ glusterBlockReplace(int argcount, char **options, int json) } if (glusterBlockParseVolumeBlock(options[2], robj.volume, robj.block_name, - helpMsg, "replace")) { + sizeof(robj.volume), sizeof(robj.block_name), + helpMsg, "replace")) { goto out; } @@ -576,16 +579,16 @@ glusterBlockReplace(int argcount, char **options, int json) options[3], GB_REPLACE_HELP_STR); goto out; } - strcpy(robj.old_node, options[3]); + GB_STRCPYSTATIC(robj.old_node, options[3]); if (!glusterBlockIsAddrAcceptable(options[4])) { MSG("host addr (%s) should be a valid ip address\n%s\n", options[4], GB_REPLACE_HELP_STR); goto out; } - strcpy(robj.new_node, options[4]); + GB_STRCPYSTATIC(robj.new_node, options[4]); - if (!strncmp(robj.old_node, robj.new_node, 255)) { + if (!strcmp(robj.old_node, robj.new_node)) { MSG(" (%s) and (%s) cannot be same\n%s\n", robj.old_node, robj.new_node, GB_REPLACE_HELP_STR); goto out; diff --git a/daemon/gluster-blockd.c b/daemon/gluster-blockd.c index 863856f..6ccd105 100644 --- a/daemon/gluster-blockd.c +++ b/daemon/gluster-blockd.c @@ -77,7 +77,7 @@ glusterBlockCliThreadProc (void *vargp) } saun.sun_family = AF_UNIX; - strcpy(saun.sun_path, GB_UNIX_ADDRESS); + GB_STRCPYSTATIC(saun.sun_path, GB_UNIX_ADDRESS); if (unlink(GB_UNIX_ADDRESS) && errno != ENOENT) { LOG("mgmt", GB_LOG_ERROR, "unlink(%s) failed (%s)", diff --git a/rpc/block_svc_routines.c b/rpc/block_svc_routines.c index 4230964..0c32293 100644 --- a/rpc/block_svc_routines.c +++ b/rpc/block_svc_routines.c @@ -163,8 +163,8 @@ removeDuplicateSubstr(char **line) element = strtok(temp, " "); while (element) { if (!strstr(out, element)) { - strcat(out, element); - strcat(out, " "); + strncat(out, element, strlen(element)); + strncat(out, " ", 1); } element = strtok(NULL, " "); } @@ -480,7 +480,7 @@ glusterBlockCallRPC_1(char *host, void *cobj, switch(opt) { case CREATE_SRV: - strcpy(((blockCreate *)cobj)->ipaddr, host); + GB_STRCPYSTATIC(((blockCreate *)cobj)->ipaddr, host); *rpc_sent = TRUE; if (block_create_1((blockCreate *)cobj, &reply, clnt) != RPC_SUCCESS) { @@ -543,7 +543,7 @@ glusterBlockCallRPC_1(char *host, void *cobj, return -1; } for (i = 0; i < obj->capMax; i++) { - strncpy(obj->response[i].cap, caps[i].cap, 256); + GB_STRCPYSTATIC(obj->response[i].cap, caps[i].cap); obj->response[i].status = caps[i].status; } *out = (char *) obj; @@ -675,18 +675,18 @@ glusterBlockMimicOldCaps(void) return NULL; } - strncpy(caps->response[0].cap, "create", 256); - strncpy(caps->response[1].cap, "create_ha", 256); - strncpy(caps->response[2].cap, "create_prealloc", 256); - strncpy(caps->response[3].cap, "create_auth", 256); + GB_STRCPYSTATIC(caps->response[0].cap, "create"); + GB_STRCPYSTATIC(caps->response[1].cap, "create_ha"); + GB_STRCPYSTATIC(caps->response[2].cap, "create_prealloc"); + GB_STRCPYSTATIC(caps->response[3].cap, "create_auth"); - strncpy(caps->response[4].cap, "delete", 256); - strncpy(caps->response[5].cap, "delete_force", 256); + GB_STRCPYSTATIC(caps->response[4].cap, "delete"); + GB_STRCPYSTATIC(caps->response[5].cap, "delete_force"); - strncpy(caps->response[6].cap, "modify", 256); - strncpy(caps->response[7].cap, "modify_auth", 256); + GB_STRCPYSTATIC(caps->response[6].cap, "modify"); + GB_STRCPYSTATIC(caps->response[7].cap, "modify_auth"); - strncpy(caps->response[8].cap, "json", 256); + GB_STRCPYSTATIC(caps->response[8].cap, "json"); for (i = 0; i < GB_OLD_CAP_MAX; i++) { caps->response[i].status = true; @@ -735,7 +735,7 @@ blockRemoteCapabilitiesRespParse(size_t count, blockRemoteObj *args, goto out; } for (k = 0; k < caps[j]->capMax; k++) { - if (!strncmp(gbCapabilitiesLookup[i], caps[j]->response[k].cap, 256) && + if (!strcmp(gbCapabilitiesLookup[i], caps[j]->response[k].cap) && caps[j]->response[k].status) { CAP_MATCH=true; break; @@ -1684,21 +1684,21 @@ glusterBlockReplaceNodeRemoteAsync(struct glfs *glfs, blockReplaceCli *blk, goto out; } - strcpy(cobj->ipaddr, blk->new_node); - strcpy(cobj->volume, info->volume); - strcpy(cobj->gbid, info->gbid); + GB_STRCPYSTATIC(cobj->ipaddr, blk->new_node); + GB_STRCPYSTATIC(cobj->volume, info->volume); + GB_STRCPYSTATIC(cobj->gbid, info->gbid); cobj->size = info->size; - strcpy(cobj->passwd, info->passwd); - strcpy(cobj->block_name, block); + GB_STRCPYSTATIC(cobj->passwd, info->passwd); + GB_STRCPYSTATIC(cobj->block_name, block); - strcpy(robj->volume, info->volume); - strcpy(robj->gbid, info->gbid); - strcpy(robj->block_name, block); - strcpy(robj->ipaddr, blk->new_node); - strcpy(robj->ripaddr, blk->old_node); + GB_STRCPYSTATIC(robj->volume, info->volume); + GB_STRCPYSTATIC(robj->gbid, info->gbid); + GB_STRCPYSTATIC(robj->block_name, block); + GB_STRCPYSTATIC(robj->ipaddr, blk->new_node); + GB_STRCPYSTATIC(robj->ripaddr, blk->old_node); - strcpy(dobj->block_name, block); - strcpy(dobj->gbid, info->gbid); + GB_STRCPYSTATIC(dobj->block_name, block); + GB_STRCPYSTATIC(dobj->gbid, info->gbid); /* Fill args[] */ if (GB_ALLOC_N(args, info->mpath + 1) < 0) { @@ -2291,8 +2291,8 @@ glusterBlockCleanUp(struct glfs *glfs, char *blockname, goto out; } - strcpy(dobj.block_name, blockname); - strcpy(dobj.gbid, info->gbid); + GB_STRCPYSTATIC(dobj.block_name, blockname); + GB_STRCPYSTATIC(dobj.gbid, info->gbid); count = glusterBlockDeleteFillArgs(info, deleteall, NULL, NULL, NULL); asyncret = glusterBlockDeleteRemoteAsync(info, glfs, &dobj, count, @@ -2671,9 +2671,9 @@ block_modify_cli_1_svc_st(blockModifyCli *blk, struct svc_req *rqstp) goto out; } - strcpy(mobj.block_name, blk->block_name); - strcpy(mobj.volume, blk->volume); - strcpy(mobj.gbid, info->gbid); + GB_STRCPYSTATIC(mobj.block_name, blk->block_name); + GB_STRCPYSTATIC(mobj.volume, blk->volume); + GB_STRCPYSTATIC(mobj.gbid, info->gbid); if (blk->auth_mode) { if(info->passwd[0] == '\0') { @@ -2681,9 +2681,9 @@ block_modify_cli_1_svc_st(blockModifyCli *blk, struct svc_req *rqstp) uuid_unparse(uuid, passwd); GB_METAUPDATE_OR_GOTO(lock, glfs, blk->block_name, blk->volume, ret, errMsg, out, "PASSWORD: %s\n", passwd); - strcpy(mobj.passwd, passwd); + GB_STRCPYSTATIC(mobj.passwd, passwd); } else { - strcpy(mobj.passwd, info->passwd); + GB_STRCPYSTATIC(mobj.passwd, info->passwd); } mobj.auth_mode = 1; } else { @@ -3022,10 +3022,10 @@ block_create_cli_1_svc_st(blockCreateCli *blk, struct svc_req *rqstp) GB_METAUPDATE_OR_GOTO(lock, glfs, blk->block_name, blk->volume, errCode, errMsg, exist, "ENTRYCREATE: SUCCESS\n"); - strcpy(cobj.volume, blk->volume); - strcpy(cobj.block_name, blk->block_name); + GB_STRCPYSTATIC(cobj.volume, blk->volume); + GB_STRCPYSTATIC(cobj.block_name, blk->block_name); cobj.size = blk->size; - strcpy(cobj.gbid, gbid); + GB_STRCPYSTATIC(cobj.gbid, gbid); if (GB_STRDUP(cobj.block_hosts, blk->block_hosts) < 0) { errCode = ENOMEM; goto exist; @@ -3035,7 +3035,7 @@ block_create_cli_1_svc_st(blockCreateCli *blk, struct svc_req *rqstp) uuid_generate(uuid); uuid_unparse(uuid, passwd); - strcpy(cobj.passwd, passwd); + GB_STRCPYSTATIC(cobj.passwd, passwd); cobj.auth_mode = 1; GB_METAUPDATE_OR_GOTO(lock, glfs, blk->block_name, blk->volume, @@ -3916,8 +3916,8 @@ block_list_cli_1_svc_st(blockListCli *blk, struct svc_req *rqstp) while ((entry = glfs_readdir (tgmdfd))) { if (strcmp(entry->d_name, ".") && - strcmp(entry->d_name, "..") && - strcmp(entry->d_name, "meta.lock")) { + strcmp(entry->d_name, "..") && + strcmp(entry->d_name, "meta.lock")) { if (blk->json_resp) { json_object_array_add(json_array, GB_JSON_OBJ_TO_STR(entry->d_name)); diff --git a/rpc/glfs-operations.c b/rpc/glfs-operations.c index b1f68ac..838d3c7 100644 --- a/rpc/glfs-operations.c +++ b/rpc/glfs-operations.c @@ -301,22 +301,22 @@ blockStuffMetaInfo(MetaInfo *info, char *line) switch (blockMetaKeyEnumParse(opt)) { case GB_META_VOLUME: - strcpy(info->volume, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->volume, strchr(line, ' ') + 1); break; case GB_META_GBID: - strcpy(info->gbid, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->gbid, strchr(line, ' ') + 1); break; case GB_META_SIZE: - sscanf(strchr(line, ' ')+1, "%zu", &info->size); + sscanf(strchr(line, ' '), "%zu", &info->size); break; case GB_META_HA: - sscanf(strchr(line, ' ')+1, "%zu", &info->mpath); + sscanf(strchr(line, ' '), "%zu", &info->mpath); break; case GB_META_ENTRYCREATE: - strcpy(info->entry, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->entry, strchr(line, ' ') + 1); break; case GB_META_PASSWD: - strcpy(info->passwd, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->passwd, strchr(line, ' ') + 1); break; default: @@ -325,15 +325,15 @@ blockStuffMetaInfo(MetaInfo *info, char *line) goto out; if(GB_ALLOC(info->list[0]) < 0) goto out; - strcpy(info->list[0]->addr, opt); - strcpy(info->list[0]->status, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->list[0]->addr, opt); + GB_STRCPYSTATIC(info->list[0]->status, strchr(line, ' ') + 1); info->nhosts = 1; } else { if(GB_REALLOC_N(info->list, info->nhosts+1) < 0) goto out; for (i = 0; i < info->nhosts; i++) { if(!strcmp(info->list[i]->addr, opt)) { - strcpy(info->list[i]->status, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->list[i]->status, strchr(line, ' ') + 1); flag = 1; break; } @@ -341,8 +341,8 @@ blockStuffMetaInfo(MetaInfo *info, char *line) if (!flag) { if(GB_ALLOC(info->list[info->nhosts]) < 0) goto out; - strcpy(info->list[info->nhosts]->addr, opt); - strcpy(info->list[info->nhosts]->status, strchr(line, ' ')+1); + GB_STRCPYSTATIC(info->list[info->nhosts]->addr, opt); + GB_STRCPYSTATIC(info->list[info->nhosts]->status, strchr(line, ' ') + 1); info->nhosts++; } } diff --git a/utils/capabilities.c b/utils/capabilities.c index d44dc8f..187b166 100644 --- a/utils/capabilities.c +++ b/utils/capabilities.c @@ -71,7 +71,7 @@ gbSetCapabilties(blockResponse **c) ret = gbCapabilitiesEnumParse(p); if (ret != GB_CAP_MAX) { - strncpy(caps[count].cap, gbCapabilitiesLookup[ret], 256); + GB_STRCPYSTATIC(caps[count].cap, gbCapabilitiesLookup[ret]); /* Part after ':' and before '\n' */ p = sep + 1; diff --git a/utils/lru.c b/utils/lru.c index 7fd036e..4348da6 100644 --- a/utils/lru.c +++ b/utils/lru.c @@ -16,7 +16,7 @@ static int lruCount; size_t glfsLruCount = 5; /* default lru cache size */ typedef struct Entry { - char volume[256]; + char volume[255]; glfs_t *glfs; struct list_head list; @@ -56,7 +56,7 @@ appendNewEntry(const char *volname, glfs_t *fs) if (GB_ALLOC(tmp) < 0) { return -1; } - strcpy(tmp->volume, volname); + GB_STRCPYSTATIC(tmp->volume, volname); tmp->glfs = fs; list_add(&(tmp->list), &Cache); diff --git a/utils/utils.c b/utils/utils.c index e76c112..13d61cc 100644 --- a/utils/utils.c +++ b/utils/utils.c @@ -340,3 +340,23 @@ gbStrdup(char **dest, const char *src, return 0; } + + +char * +gbStrcpy(char *dest, const char *src, size_t destbytes, + const char *filename, const char *funcname, size_t linenr) +{ + char *ret; + size_t n = strlen(src); + + if (n > (destbytes - 1)) + return NULL; + + ret = strncpy(dest, src, n); + /* strncpy NULL terminates if the last character is \0. Therefore + * force the last byte to be \0 + */ + dest[n] = '\0'; + + return ret; +} diff --git a/utils/utils.h b/utils/utils.h index 7693b5c..3362c10 100644 --- a/utils/utils.h +++ b/utils/utils.h @@ -327,6 +327,13 @@ extern struct gbConf gbConf; gbStrdup(&(dst), src, \ __FILE__, __FUNCTION__, __LINE__) +# define GB_STRCPY(dst, src, destbytes) \ + gbStrcpy((dst), (src), (destbytes), \ + __FILE__, __FUNCTION__, __LINE__) + +# define GB_STRCPYSTATIC(dst, src) \ + GB_STRCPY((dst), (src), (sizeof(dst))) + # define GB_FREE(ptr) \ gbFree(1 ? (void *) &(ptr) : (ptr)) @@ -532,6 +539,9 @@ int gbReallocN(void *ptrptr, size_t size, size_t count, int gbStrdup(char **dest, const char *src, const char *filename, const char *funcname, size_t linenr); +char* gbStrcpy(char *dest, const char *src, size_t destbytes, + const char *filename, const char *funcname, size_t linenr); + void gbFree(void *ptrptr); #endif /* _UTILS_H */ -- cgit