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 8917f698dfc531e30f4b6b8f707848bffcce1396..0b872aa89649de97694c1afc942c5aa6689053c3 100644 (file)
--- 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);