use snprintf, strdup, ... where possible to make for safer operation -- Martin Pelikan

git-svn-id: svn://svn.oetiker.ch/rrdtool/trunk/program@2299 a5681a0c-68f1-0310-ab6d-d61299d08faa

diff --git a/src/rrd_cgi.c b/src/rrd_cgi.c
index df4a9dc..a0c9b3b 100644 (file)
--- a/src/rrd_cgi.c
+++ b/src/rrd_cgi.c
@@ -387,14 +387,10 @@ static void calfree(
 char     *stralloc(
     const char *str)
 {
-    char     *nstr;
-
     if (!str) {
         return NULL;
     }
-    nstr = malloc((strlen(str) + 1));
-    strcpy(nstr, str);
-    return (nstr);
+    return strdup(str);
 }
 
 static int readfile(
@@ -595,12 +591,13 @@ char     *rrdsetenv(
     const char **args)
 {
     if (argc >= 2) {
-        char     *xyz = malloc((strlen(args[0]) + strlen(args[1]) + 2));
+        const size_t len = strlen(args[0]) + strlen(args[1]) + 2;
+        char *xyz = malloc(len);
 
         if (xyz == NULL) {
             return stralloc("[ERROR: allocating setenv buffer]");
         };
-        sprintf(xyz, "%s=%s", args[0], args[1]);
+        snprintf(xyz, len, "%s=%s", args[0], args[1]);
         if (putenv(xyz) == -1) {
             free(xyz);
             return stralloc("[ERROR: failed to do putenv]");
@@ -788,9 +785,10 @@ char     *includefile(
 
         readfile(filename, &buffer, 0);
         if (rrd_test_error()) {
-            char     *err = malloc((strlen(rrd_get_error()) + DS_NAM_SIZE));
+            const size_t len = strlen(rrd_get_error()) + DS_NAM_SIZE;
+            char *err = malloc(len);
 
-            sprintf(err, "[ERROR: %s]", rrd_get_error());
+            snprintf(err, len, "[ERROR: %s]", rrd_get_error());
             rrd_clear_error();
             return err;
         } else {
@@ -954,10 +952,9 @@ char     *drawgraph(
         return stralloc(calcpr[0]);
     } else {
         if (rrd_test_error()) {
-            char     *err =
-                malloc((strlen(rrd_get_error()) +
-                        DS_NAM_SIZE) * sizeof(char));
-            sprintf(err, "[ERROR: %s]", rrd_get_error());
+            const size_t len = strlen(rrd_get_error()) + DS_NAM_SIZE;
+            char *err = malloc(len);
+            snprintf(err, len, "[ERROR: %s]", rrd_get_error());
             rrd_clear_error();
             return err;
         }
@@ -998,10 +995,9 @@ char     *printtimelast(
 
         last = rrd_last(argc, (char **) args - 1);
         if (rrd_test_error()) {
-            char     *err =
-                malloc((strlen(rrd_get_error()) +
-                        DS_NAM_SIZE) * sizeof(char));
-            sprintf(err, "[ERROR: %s]", rrd_get_error());
+            const size_t len = strlen(rrd_get_error()) + DS_NAM_SIZE;
+            char *err = malloc(len);
+            snprintf(err, len, "[ERROR: %s]", rrd_get_error());
             rrd_clear_error();
             return err;
         }
@@ -1388,6 +1384,7 @@ s_var   **rrdcgiReadVariables(
     s_var   **result;
     int       i, k, len;
     char      tmp[101];
+    size_t    tmplen;
 
     cp = getenv("REQUEST_METHOD");
     ip = getenv("CONTENT_LENGTH");
@@ -1404,9 +1401,8 @@ s_var   **rrdcgiReadVariables(
     } else if (cp && !strcmp(cp, "GET")) {
         esp = getenv("QUERY_STRING");
         if (esp && strlen(esp)) {
-            if ((line = (char *) malloc(strlen(esp) + 2)) == NULL)
+            if ((line = strdup(esp)) == NULL)
                 return NULL;
-            sprintf(line, "%s", esp);
         } else
             return NULL;
     } else {
@@ -1414,22 +1410,18 @@ s_var   **rrdcgiReadVariables(
         printf("(offline mode: enter name=value pairs on standard input)\n");
         memset(tmp, 0, sizeof(tmp));
         while ((cp = fgets(tmp, 100, stdin)) != NULL) {
-            if (strlen(tmp)) {
-                if (tmp[strlen(tmp) - 1] == '\n')
-                    tmp[strlen(tmp) - 1] = '&';
-                if (length) {
-                    length += strlen(tmp);
-                    len = (length + 1) * sizeof(char);
+            if ((tmplen = strlen(tmp)) != 0) {
+                if (tmp[tmplen - 1] == '\n')
+                    tmp[tmplen - 1] = '&';
+                length += tmplen;
+                len = (length + 1) * sizeof(char);
+                if ((unsigned) length > tmplen) {
                     if ((line = (char *) realloc(line, len)) == NULL)
                         return NULL;
-                    strcat(line, tmp);
+                    strncat(line, tmp, tmplen);
                 } else {
-                    length = strlen(tmp);
-                    len = (length + 1) * sizeof(char);
-                    if ((line = (char *) malloc(len)) == NULL)
+                    if ((line = strdup(tmp)) == NULL)
                         return NULL;
-                    memset(line, 0, len);
-                    strcpy(line, tmp);
                 }
             }
             memset(tmp, 0, sizeof(tmp));
@@ -1527,14 +1519,10 @@ s_var   **rrdcgiReadVariables(
                 i++;
             } else {    /* There is already such a name, suppose a mutiple field */
                 cp = ++esp;
-                len =
-                    (strlen(result[k]->value) + (ip - esp) +
-                     2) * sizeof(char);
-                if ((sptr = (char *) malloc(len)) == NULL)
+                len = strlen(result[k]->value) + (ip - esp) + 2;
+                if ((sptr = (char *) calloc(len, sizeof(char))) == NULL)
                     return NULL;
-                memset(sptr, 0, len);
-                sprintf(sptr, "%s\n", result[k]->value);
-                strncat(sptr, cp, ip - esp);
+                snprintf(sptr, len, "%s\n%s", result[k]->value, cp);
                 free(result[k]->value);
                 result[k]->value = rrdcgiDecodeString(sptr);
             }

diff --git a/src/rrd_client.c b/src/rrd_client.c
index 3347b82..f271f3d 100644 (file)
--- a/src/rrd_client.c
+++ b/src/rrd_client.c
@@ -913,8 +913,7 @@ rrd_info_t * rrdc_info (const char *filename) /* {{{ */
         break;
     case RD_I_STR:
         chomp(s);
-        info.u_str = (char*)malloc(sizeof(char) * (strlen(s) + 1));
-        strcpy(info.u_str,s);
+        info.u_str = strdup(s);
         break;
     case RD_I_BLO:
         rrd_set_error ("rrdc_info: BLOB objects are not supported");

diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c
index 0610d38..0ea5bf7 100644 (file)
--- a/src/rrd_daemon.c
+++ b/src/rrd_daemon.c
@@ -599,7 +599,7 @@ static int send_response (listen_socket_t *sock, response_code rc,
   else
     lines = -1;
 
-  rclen = sprintf(buffer, "%d ", lines);
+  rclen = snprintf(buffer, sizeof buffer, "%d ", lines);
   va_start(argp, fmt);
 #ifdef HAVE_VSNPRINTF
   len = vsnprintf(buffer+rclen, sizeof(buffer)-rclen, fmt, argp);

diff --git a/src/rrd_getopt.c b/src/rrd_getopt.c
index 46f7313..7d157a0 100644 (file)
--- a/src/rrd_getopt.c
+++ b/src/rrd_getopt.c
@@ -396,7 +396,7 @@ static const char* _getopt_initialize(int argc,
            considered as options.  */
         char      var[100];
 
-        sprintf(var, "_%d_GNU_nonoption_argv_flags_", getpid());
+        snprintf(var, sizeof var, "_%d_GNU_nonoption_argv_flags_", getpid());
         nonoption_flags = getenv(var);
         if (nonoption_flags == NULL)
             nonoption_flags_len = 0;

diff --git a/src/rrd_graph.c b/src/rrd_graph.c
index 8851dfe..317becc 100644 (file)
--- a/src/rrd_graph.c
+++ b/src/rrd_graph.c
@@ -1676,14 +1676,9 @@ int print_calc(
                              im->gdes[i].format);
                         return -1;
                     }
-#ifdef HAVE_SNPRINTF
                     snprintf(im->gdes[i].legend,
                              FMT_LEG_LEN - 2,
                              im->gdes[i].format, printval, si_symb);
-#else
-                    sprintf(im->gdes[i].legend,
-                            im->gdes[i].format, printval, si_symb);
-#endif
                 }
                 graphelement = 1;
             }
@@ -1771,7 +1766,7 @@ int leg_place(
         for (i = 0; i < im->gdes_c; i++) {
             char      prt_fctn; /*special printfunctions */
             if(calc_width){
-                strcpy(saved_legend, im->gdes[i].legend);
+                strncpy(saved_legend, im->gdes[i].legend, sizeof saved_legend);
             }
 
             fill_last = fill;
@@ -1938,7 +1933,7 @@ int leg_place(
             }
 
             if(calc_width){
-                strcpy(im->gdes[i].legend, saved_legend);
+                strncpy(im->gdes[i].legend, saved_legend, sizeof im->gdes[0].legend);
             }
         }
 
@@ -2021,7 +2016,7 @@ int calc_horizontal_grid(
 
                 if (im->unitslength < len + 2)
                     im->unitslength = len + 2;
-                sprintf(im->ygrid_scale.labfmt,
+                snprintf(im->ygrid_scale.labfmt, sizeof im->ygrid_scale.labfmt, 
                         "%%%d.%df%s", len,
                         -fractionals, (im->symbol != ' ' ? " %c" : ""));
             } else {
@@ -2029,7 +2024,7 @@ int calc_horizontal_grid(
 
                 if (im->unitslength < len + 2)
                     im->unitslength = len + 2;
-                sprintf(im->ygrid_scale.labfmt,
+                snprintf(im->ygrid_scale.labfmt, sizeof im->ygrid_scale.labfmt,
                         "%%%d.0f%s", len, (im->symbol != ' ' ? " %c" : ""));
             }
         } else {        /* classic rrd grid */
@@ -2093,15 +2088,15 @@ int draw_horizontal_grid(
                     && (YN < im->yorigin - im->ysize || YN > im->yorigin))) {
                 if (im->symbol == ' ') {
                     if (im->extra_flags & ALTYGRID) {
-                        sprintf(graph_label,
+                        snprintf(graph_label, sizeof graph_label,
                                 im->ygrid_scale.labfmt,
                                 scaledstep * (double) i);
                     } else {
                         if (MaxY < 10) {
-                            sprintf(graph_label, "%4.1f",
+                            snprintf(graph_label, sizeof graph_label, "%4.1f",
                                     scaledstep * (double) i);
                         } else {
-                            sprintf(graph_label, "%4.0f",
+                            snprintf(graph_label, sizeof graph_label, "%4.0f",
                                     scaledstep * (double) i);
                         }
                     }
@@ -2109,15 +2104,15 @@ int draw_horizontal_grid(
                     char      sisym = (i == 0 ? ' ' : im->symbol);
 
                     if (im->extra_flags & ALTYGRID) {
-                        sprintf(graph_label,
+                        snprintf(graph_label, sizeof graph_label,
                                 im->ygrid_scale.labfmt,
                                 scaledstep * (double) i, sisym);
                     } else {
                         if (MaxY < 10) {
-                            sprintf(graph_label, "%4.1f %c",
+                            snprintf(graph_label, sizeof graph_label, "%4.1f %c",
                                     scaledstep * (double) i, sisym);
                         } else {
-                            sprintf(graph_label, "%4.0f %c",
+                            snprintf(graph_label, sizeof graph_label, "%4.0f %c",
                                     scaledstep * (double) i, sisym);
                         }
                     }
@@ -2134,13 +2129,13 @@ int draw_horizontal_grid(
                             sval /= second_axis_magfact;
 
                             if(MaxY < 10) {
-                                sprintf(graph_label_right,"%5.1f %s",sval,second_axis_symb);
+                                snprintf(graph_label_right, sizeof graph_label_right, "%5.1f %s",sval,second_axis_symb);
                             } else {
-                                sprintf(graph_label_right,"%5.0f %s",sval,second_axis_symb);
+                                snprintf(graph_label_right, sizeof graph_label_right, "%5.0f %s",sval,second_axis_symb);
                             }
                         }
                         else {
-                           sprintf(graph_label_right,im->second_axis_format,sval,"");
+                           snprintf(graph_label_right, sizeof graph_label_right, im->second_axis_format,sval,"");
                         }
                         gfx_text ( im,
                                X1+7, Y0,
@@ -2326,9 +2321,9 @@ int horizontal_log_grid(
                 symbol = si_symbol[scale + si_symbcenter];
             else
                 symbol = '?';
-            sprintf(graph_label, "%3.0f %c", pvalue, symbol);
+            snprintf(graph_label, sizeof graph_label, "%3.0f %c", pvalue, symbol);
         } else {
-            sprintf(graph_label, "%3.0e", value);
+            snprintf(graph_label, sizeof graph_label, "%3.0e", value);
         }
         if (im->second_axis_scale != 0){
                 char graph_label_right[100];
@@ -2338,14 +2333,14 @@ int horizontal_log_grid(
                                 double mfac = 1;
                                 char   *symb = "";
                                 auto_scale(im,&sval,&symb,&mfac);
-                                sprintf(graph_label_right,"%4.0f %s", sval,symb);
+                                snprintf(graph_label_right, sizeof graph_label_right, "%4.0f %s", sval,symb);
                         }
                         else {
-                                sprintf(graph_label_right,"%3.0e", sval);
+                                snprintf(graph_label_right, sizeof graph_label_right, "%3.0e", sval);
                         }
                 }
                 else {
-                      sprintf(graph_label_right,im->second_axis_format,sval,"");
+                      snprintf(graph_label_right, sizeof graph_label_right, im->second_axis_format,sval,"");
                 }
 
                 gfx_text ( im,
@@ -4051,9 +4046,7 @@ int rrd_graph(
                 return 0;
             }
             /* imginfo goes to position 0 in the prdata array */
-            (*prdata)[prlines - 1] = (char*)malloc((strlen(walker->value.u_str)
-                                             + 2) * sizeof(char));
-            strcpy((*prdata)[prlines - 1], walker->value.u_str);
+            (*prdata)[prlines - 1] = strdup(walker->value.u_str);
             (*prdata)[prlines] = NULL;
         }
         /* skip anything else */
@@ -4081,10 +4074,8 @@ int rrd_graph(
                 rrd_set_error("realloc prdata");
                 return 0;
             }
-            (*prdata)[prlines - 1] = (char*)malloc((strlen(walker->value.u_str)
-                                             + 2) * sizeof(char));
+            (*prdata)[prlines - 1] = strdup(walker->value.u_str);
             (*prdata)[prlines] = NULL;
-            strcpy((*prdata)[prlines - 1], walker->value.u_str);
         } else if (strcmp(walker->key, "image") == 0) {
             if ( fwrite(walker->value.u_blo.ptr, walker->value.u_blo.size, 1,
                    (stream ? stream : stdout)) == 0 && ferror(stream ? stream : stdout)){

diff --git a/src/rrd_graph_helper.c b/src/rrd_graph_helper.c
index 086f5c6..912dea6 100644 (file)
--- a/src/rrd_graph_helper.c
+++ b/src/rrd_graph_helper.c
@@ -73,17 +73,16 @@ char* checkUnusedValues(parsedargs_t* pa){
   char *res=NULL;
   for(int i=0;i<pa->kv_cnt;i++) {
     if (!pa->kv_args[i].flag) {
-      int len=0;
-      if (res) {len=strlen(res); }
-      char* t=realloc(res,len+3
-                     +strlen(pa->kv_args[i].key)
-                     +strlen(pa->kv_args[i].value)
-                     );
+      const size_t klen = strlen(pa->kv_args[i].key);
+      const size_t vlen = strlen(pa->kv_args[i].value);
+      const size_t len = res ? strlen(res) : 0;
+
+      char *t = realloc(res,len + 3 + klen + vlen);
       if (! t) { return res; }
       res=t;
-      strcat(res,pa->kv_args[i].key);
+      strncat(res,pa->kv_args[i].key, klen);
       strcat(res,"=");
-      strcat(res,pa->kv_args[i].value);
+      strncat(res,pa->kv_args[i].value, vlen);
       strcat(res,":");
     }
   }

diff --git a/src/rrd_info.c b/src/rrd_info.c
index 2f6c07f..5722025 100644 (file)
--- a/src/rrd_info.c
+++ b/src/rrd_info.c
@@ -71,8 +71,7 @@ rrd_info_t
         next->value.u_int = value.u_int;
         break;
     case RD_I_STR:
-        next->value.u_str = (char*)malloc(sizeof(char) * (strlen(value.u_str) + 1));
-        strcpy(next->value.u_str, value.u_str);
+        next->value.u_str = strdup(value.u_str);
         break;
     case RD_I_BLO:
         next->value.u_blo.size = value.u_blo.size;

diff --git a/src/rrd_parsetime.c b/src/rrd_parsetime.c
index 1b59f45..d854dfb 100644 (file)
--- a/src/rrd_parsetime.c
+++ b/src/rrd_parsetime.c
@@ -599,7 +599,7 @@ static char *tod(
         scc = scc_sv;
         sct = sct_sv;
         sc_tokid = sc_tokid_sv;
-        sprintf(sc_token, "%d", hour);
+        snprintf(sc_token, sc_len, "%d", hour);
         return TIME_OK;
     }
     if (sc_tokid == COLON) {
@@ -631,7 +631,7 @@ static char *tod(
         scc = scc_sv;
         sct = sct_sv;
         sc_tokid = sc_tokid_sv;
-        sprintf(sc_token, "%d", hour);
+        snprintf(sc_token, sc_len, "%d", hour);
         return TIME_OK;
     }
     ptv->tm.  tm_hour = hour;

diff --git a/src/rrd_rpncalc.c b/src/rrd_rpncalc.c
index 0677b30..ab01c8a 100644 (file)
--- a/src/rrd_rpncalc.c
+++ b/src/rrd_rpncalc.c
@@ -117,7 +117,7 @@ void rpn_compact2str(
 #if defined(_WIN32) && !defined(__CYGWIN__) && !defined(__CYGWIN32__)
             _itoa(rpnc[i].val, buffer, 10);
 #else
-            sprintf(buffer, "%d", rpnc[i].val);
+            snprintf(buffer, sizeof buffer, "%d", rpnc[i].val);
 #endif
             add_op(OP_NUMBER, buffer)
         }

diff --git a/src/rrd_xport.c b/src/rrd_xport.c
index b2afd1c..ebe72aa 100644 (file)
--- a/src/rrd_xport.c
+++ b/src/rrd_xport.c
@@ -308,10 +308,9 @@ int rrd_xport_fn(
             *step_list_ptr = im->gdes[im->gdes[i].vidx].step;
             /* printf("%s:%lu\n",im->gdes[i].legend,*step_list_ptr); */
             step_list_ptr++;
+
             /* reserve room for one legend entry */
-            /* is FMT_LEG_LEN + 5 the correct size? */
-            if ((legend_list[j] =
-                (char*)malloc(sizeof(char) * (FMT_LEG_LEN + 5))) == NULL) {
+            if ((legend_list[j] = strdup(im->gdes[i].legend)) == NULL) {
                 free(ref_list);
                 *data = NULL;
                 while (--j > -1)
@@ -322,11 +321,9 @@ int rrd_xport_fn(
                 return (-1);
             }
 
-            if (im->gdes[i].legend)
-                /* omit bounds check, should have the same size */
-                strcpy(legend_list[j++], im->gdes[i].legend);
-            else
-                legend_list[j++][0] = '\0';
+            if (im->gdes[i].legend == 0)
+                legend_list[j][0] = '\0';
+            ++j;
        }
     }
     *step_list_ptr=0;