From 948659f1493a73dd00df14791a094b6b2e757a3a Mon Sep 17 00:00:00 2001 From: oetiker Date: Mon, 13 Aug 2012 06:19:17 +0000 Subject: [PATCH] use snprintf, strdup, ... where possible to make for safer operation -- Martin Pelikan git-svn-id: svn://svn.oetiker.ch/rrdtool/trunk@2299 a5681a0c-68f1-0310-ab6d-d61299d08faa --- program/src/rrd_cgi.c | 64 ++++++++++++++-------------------- program/src/rrd_client.c | 3 +- program/src/rrd_daemon.c | 2 +- program/src/rrd_getopt.c | 2 +- program/src/rrd_graph.c | 49 +++++++++++--------------- program/src/rrd_graph_helper.c | 15 ++++---- program/src/rrd_info.c | 3 +- program/src/rrd_parsetime.c | 4 +-- program/src/rrd_rpncalc.c | 2 +- program/src/rrd_xport.c | 13 +++---- 10 files changed, 65 insertions(+), 92 deletions(-) diff --git a/program/src/rrd_cgi.c b/program/src/rrd_cgi.c index df4a9dcd..a0c9b3b2 100644 --- a/program/src/rrd_cgi.c +++ b/program/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/program/src/rrd_client.c b/program/src/rrd_client.c index 3347b825..f271f3db 100644 --- a/program/src/rrd_client.c +++ b/program/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/program/src/rrd_daemon.c b/program/src/rrd_daemon.c index 0610d388..0ea5bf76 100644 --- a/program/src/rrd_daemon.c +++ b/program/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/program/src/rrd_getopt.c b/program/src/rrd_getopt.c index 46f73134..7d157a0e 100644 --- a/program/src/rrd_getopt.c +++ b/program/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/program/src/rrd_graph.c b/program/src/rrd_graph.c index 8851dfe9..317beccc 100644 --- a/program/src/rrd_graph.c +++ b/program/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/program/src/rrd_graph_helper.c b/program/src/rrd_graph_helper.c index 086f5c6f..912dea6c 100644 --- a/program/src/rrd_graph_helper.c +++ b/program/src/rrd_graph_helper.c @@ -73,17 +73,16 @@ char* checkUnusedValues(parsedargs_t* pa){ char *res=NULL; for(int i=0;ikv_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/program/src/rrd_info.c b/program/src/rrd_info.c index 2f6c07ff..57220253 100644 --- a/program/src/rrd_info.c +++ b/program/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/program/src/rrd_parsetime.c b/program/src/rrd_parsetime.c index 1b59f454..d854dfbc 100644 --- a/program/src/rrd_parsetime.c +++ b/program/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/program/src/rrd_rpncalc.c b/program/src/rrd_rpncalc.c index 0677b309..ab01c8ad 100644 --- a/program/src/rrd_rpncalc.c +++ b/program/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/program/src/rrd_xport.c b/program/src/rrd_xport.c index b2afd1cb..ebe72aad 100644 --- a/program/src/rrd_xport.c +++ b/program/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; -- 2.30.2