replace strcat and strcpy with their secure versions

Change-Id: If98ce7b7e50901ee130bbe190a12664ec0adb8c2
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

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("<old-node> (%s) and <new-node> (%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 */