From: Marek Schimara Date: Thu, 16 Jun 2016 09:02:27 +0000 (+0200) Subject: src/rrd_cgi.c: fix Coverity CIDs#17398,#13656 Resource leak X-Git-Tag: v1.7.0~42^2~39 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e4a90bbdd3a9dd3bb68376182db39f50cf3bbd56;p=thirdparty%2Frrdtool-1.x.git src/rrd_cgi.c: fix Coverity CIDs#17398,#13656 Resource leak CWE-404 / https://cwe.mitre.org/data/definitions/404.html --- diff --git a/src/rrd_cgi.c b/src/rrd_cgi.c index 8917f698..0b872aa8 100644 --- a/src/rrd_cgi.c +++ b/src/rrd_cgi.c @@ -1354,6 +1354,26 @@ char *rrdcgiDecodeString( return text; } +/* free_result(s_var**, number) + * + * Clean-up the 'result' variable from rrdcgiReadVariables() properly. + * We can safely call free() on result[i]->{name,value} because they are + * memset() to 0 after their allocation. + */ +static void free_result(s_var **result, int number) +{ + int i; + + for(i = 0; i < number; i++) { + if (result && result[i]) { + free(result[i]->name); + free(result[i]->value); + free(result[i]); + } + } + free(result); +} + /* rrdcgiReadVariables() * * Read from stdin if no string is provided via CGI. Variables that @@ -1379,8 +1399,10 @@ s_var **rrdcgiReadVariables( length = atoi(ip); if ((line = (char *) malloc(length + 2)) == NULL) return NULL; - if (fgets(line, length + 1, stdin) == NULL) + if (fgets(line, length + 1, stdin) == NULL) { + free(line); return NULL; + } } else return NULL; } else if (cp && !strcmp(cp, "GET")) { @@ -1405,6 +1427,10 @@ s_var **rrdcgiReadVariables( return NULL; strncat(line, tmp, tmplen); } else { + /* clean-up the storage allocated in previous iteration */ + if (line) + free(line); + if ((line = strdup(tmp)) == NULL) return NULL; } @@ -1481,17 +1507,26 @@ s_var **rrdcgiReadVariables( (size_t) (esp - cp))); k++); if (k == i) { /* No such variable yet */ - if ((result[i] = (s_var *) malloc(sizeof(s_var))) == NULL) + if ((result[i] = (s_var *) malloc(sizeof(s_var))) == NULL) { + free_result(result, i); + free(line); return NULL; + } if ((result[i]->name = - (char *) malloc((esp - cp + 1) * sizeof(char))) == NULL) + (char *) malloc((esp - cp + 1) * sizeof(char))) == NULL) { + free_result(result, i); + free(line); return NULL; + } memset(result[i]->name, 0, esp - cp + 1); strncpy(result[i]->name, cp, esp - cp); cp = ++esp; if ((result[i]->value = - (char *) malloc((ip - esp + 1) * sizeof(char))) == NULL) + (char *) malloc((ip - esp + 1) * sizeof(char))) == NULL) { + free_result(result, i); + free(line); return NULL; + } memset(result[i]->value, 0, ip - esp + 1); strncpy(result[i]->value, cp, ip - esp); result[i]->value = rrdcgiDecodeString(result[i]->value); @@ -1507,8 +1542,11 @@ s_var **rrdcgiReadVariables( } else { /* There is already such a name, suppose a multiple field */ cp = ++esp; len = strlen(result[k]->value) + (ip - esp) + 2; - if ((sptr = (char *) calloc(len, sizeof(char))) == NULL) + if ((sptr = (char *) calloc(len, sizeof(char))) == NULL) { + free_result(result, i); + free(line); return NULL; + } snprintf(sptr, len, "%s\n%s", result[k]->value, cp); free(result[k]->value); result[k]->value = rrdcgiDecodeString(sptr);