strlcpy restricted to array length. 63/13163/14
authorPat Riehecky <riehecky@fnal.gov>
Wed, 6 Jun 2018 15:55:18 +0000 (10:55 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 26 Feb 2021 15:18:40 +0000 (10:18 -0500)
For kaprocs, size of 'caller.userID.name' is defined by a different macro
than size of 'name'.  They can become out of sync, so restricting to
size of dest.
For scout and afsmon-win, the if statement determined that the string
was longer than the dest buffer.  So we are using the size of the buffer
as the max length to setup for truncation.
(via cppcheck)

Change-Id: I38a2bff1d59d17ea02e136c35cd5b132a75a8ed8
Reviewed-on: https://gerrit.openafs.org/13163
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/afsmonitor/afsmon-win.c
src/kauth/kaprocs.c
src/scout/scout.c

index c5417ff..ed787a0 100644 (file)
@@ -15,8 +15,8 @@
 #include <afsconfig.h>
 #include <afs/param.h>
 
-
 #include <stdio.h>
+#include <string.h>
 #include <signal.h>
 #include <math.h>
 #include <afs/cmd.h>
@@ -36,6 +36,7 @@
 #include <afs/xstat_fs.h>
 #include <afs/xstat_cm.h>
 
+#include <roken.h>
 
 #include "afsmonitor.h"
 #include "afsmon-labels.h"
@@ -325,8 +326,6 @@ initLightObject(char *a_name, int a_x, int a_y, int a_width,
 {                              /*initLightObject */
     struct onode *newlightp;   /*Ptr to new light onode */
     struct gator_light_crparams light_crparams;        /*Light creation params */
-    char *truncname;           /*Truncated name, if needed */
-    int name_len;              /*True length of name */
 
 /* the following debug statement floods the debug file */
 #ifdef DEBUG_DETAILED
@@ -345,20 +344,17 @@ initLightObject(char *a_name, int a_x, int a_y, int a_width,
      * received.
      */
     light_crparams.onode_params.cr_type = GATOR_OBJ_LIGHT;
-    name_len = strlen(a_name);
 
-    if (name_len <= a_width)
-       sprintf(light_crparams.onode_params.cr_name, "%s", a_name);
-    else {
-       /*
-        * We need to truncate the given name, leaving a `*' at the end to
-        * show us it's been truncated.
-        */
-       truncname = light_crparams.onode_params.cr_name;
-       strncpy(truncname, a_name, a_width - 1);
-       truncname[a_width - 1] = '*';
-       truncname[a_width] = 0;
-    }
+    if (a_width >= sizeof(light_crparams.onode_params.cr_name))
+       a_width = sizeof(light_crparams.onode_params.cr_name) - 1;
+
+    if (a_width < 1)
+       a_width = 1;
+
+    if (strlcpy(light_crparams.onode_params.cr_name, a_name, a_width + 1) >= a_width + 1)
+       /* The name is truncated, put a '*' at the end to note */
+       light_crparams.onode_params.cr_name[a_width - 1] = '*';
+
     light_crparams.onode_params.cr_x = a_x;
     light_crparams.onode_params.cr_y = a_y;
     light_crparams.onode_params.cr_width = a_width;
index f7c639e..dbfba02 100644 (file)
@@ -1827,8 +1827,8 @@ GetTicket(int version,
     }
 
     if (import) {
-       strcpy(caller.userID.name, name);
-       strcpy(caller.userID.instance, instance);
+       strlcpy(caller.userID.name, name, sizeof(caller.userID.name));
+       strlcpy(caller.userID.instance, instance, sizeof(caller.userID.instance));
        caller.max_ticket_lifetime = htonl(MAXKTCTICKETLIFETIME);
        caller.flags = htonl(KAFNORMAL);
        caller.user_expiration = htonl(NEVERDATE);
index 132c872..e474662 100644 (file)
@@ -31,6 +31,8 @@
 #include <afs/fsprobe.h>               /*Interface for fsprobe module */
 #include <afs/afsutil.h>
 
+#include <string.h>
+
 /*
  * Command line parameter indicies.
  */
@@ -266,7 +268,6 @@ mini_initLightObject(char *a_name, int a_x, int a_y, int a_width, struct gwin *a
     struct onode *newlightp;   /*Ptr to new light onode */
     /*We only support curses right now */
     struct gator_light_crparams light_crparams;        /*Light creation params */
-    char *truncname;           /*Truncated name, if needed */
     int name_len;              /*True length of name */
 
     if (scout_debug) {
@@ -286,18 +287,14 @@ mini_initLightObject(char *a_name, int a_x, int a_y, int a_width, struct gwin *a
     if (scout_debug)
        fprintf(scout_debugfd, "[%s] Name '%s' has %d chars\n", rn, a_name,
                name_len);
-    if (name_len <= a_width)
-       sprintf(light_crparams.onode_params.cr_name, "%s", a_name);
-    else {
-       /*
-        * We need to truncate the given name, leaving a `*' at the end to
-        * show us it's been truncated.
-        */
-       truncname = light_crparams.onode_params.cr_name;
-       strncpy(truncname, a_name, a_width - 1);
-       truncname[a_width - 1] = '*';
-       truncname[a_width] = 0;
-    }
+
+    if (a_width >= sizeof(light_crparams.onode_params.cr_name))
+       a_width = sizeof(light_crparams.onode_params.cr_name) - 1;
+
+    if (strlcpy(light_crparams.onode_params.cr_name, a_name, a_width + 1) >= a_width + 1)
+       /* The name is truncated, put a '*' at the end to note */
+       light_crparams.onode_params.cr_name[a_width - 1] = '*';
+
     light_crparams.onode_params.cr_x = a_x;
     light_crparams.onode_params.cr_y = a_y;
     light_crparams.onode_params.cr_width = a_width;