This patch fixes the following issues in ECPGstore_input():
- strlen() was invoked on the NULL pointer for the first iteration of the loop (line 875, 923, 966, 1009)
- `nval' is freed for every iteration of the loop at 864, but only initialized once outside the loop, resulting in potential multiple free()'s, as well as the use of a freed variable in subsequent iterations
- `str' was leaked for every subsequent iteration of the loop (line 871, 920, 963, 1006)
- the return value of PGTYPESinterval_to_asc() is leaked at line 920 and 937; the return value of PGTYPESdate_to_asc() is leaked at line 963 and 980; the return value of PGTYPEStimestamp_to_asc() is leaked at line 1006 and 1023.
- malloc failure is in general not handled well; the function simply returns without bothering to clean up allocated resources, and many return values are not checked for errors.
Also, in create_statement(), `*stmt' was dereferenced before being initialized.
Per the Coverity report run by EnterpriseDB. Thanks to Eric Astor at EDB for an initial version of this patch -- the attached version has been improved by myself.
Barring any objections, I'd like to apply this to CVS in a day or two (I want to test it first, which I haven't yet done).
-Neil
Index: src/interfaces/ecpg/ecpglib/execute.c =================================================================== RCS file: /var/lib/cvs/pgsql/src/interfaces/ecpg/ecpglib/execute.c,v retrieving revision 1.41 diff -c -r1.41 execute.c *** src/interfaces/ecpg/ecpglib/execute.c 2 Jul 2005 17:01:53 -0000 1.41 --- src/interfaces/ecpg/ecpglib/execute.c 4 Jul 2005 05:52:51 -0000 *************** *** 143,149 **** static bool create_statement(int lineno, int compat, int force_indicator, struct connection * connection, struct statement ** stmt, char *query, va_list ap) { ! struct variable **list = &((*stmt)->inlist); enum ECPGttype type; if (!(*stmt = (struct statement *) ECPGalloc(sizeof(struct statement), lineno))) --- 143,149 ---- static bool create_statement(int lineno, int compat, int force_indicator, struct connection * connection, struct statement ** stmt, char *query, va_list ap) { ! struct variable **list; enum ECPGttype type; if (!(*stmt = (struct statement *) ECPGalloc(sizeof(struct statement), lineno))) *************** *** 856,1039 **** case ECPGt_numeric: { char *str = NULL; int slen; numeric *nval = PGTYPESnumeric_new(); if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { if (var->type == ECPGt_numeric) ! PGTYPESnumeric_copy((numeric *) ((var + var->offset * element)->value), nval); else ! PGTYPESnumeric_from_decimal((decimal *) ((var + var->offset * element)->value), nval); str = PGTYPESnumeric_to_asc(nval, nval->dscale); ! PGTYPESnumeric_free(nval); slen = strlen(str); ! if (!(mallocedval = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [] "), lineno))) ! return false; ! if (!element) strcpy(mallocedval, "array ["); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { if (var->type == ECPGt_numeric) ! PGTYPESnumeric_copy((numeric *) (var->value), nval); else ! PGTYPESnumeric_from_decimal((decimal *) (var->value), nval); str = PGTYPESnumeric_to_asc(nval, nval->dscale); ! ! PGTYPESnumeric_free(nval); slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + 1, lineno))) ! return false; ! strncpy(mallocedval, str, slen); mallocedval[slen] = '\0'; } *tobeinserted_p = mallocedval; *malloced_p = true; ! free(str); } - break; case ECPGt_interval: { char *str = NULL; int slen; if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { ! str = quote_postgres(PGTYPESinterval_to_asc((interval *) ((var + var->offset * element)->value)), lineno); slen = strlen(str); ! if (!(mallocedval = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [],interval "), lineno))) return false; ! if (!element) strcpy(mallocedval, "array ["); strcpy(mallocedval + strlen(mallocedval), "interval "); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { ! str = quote_postgres(PGTYPESinterval_to_asc((interval *) (var->value)), lineno); slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + sizeof("interval ") + 1, lineno))) return false; strcpy(mallocedval, "interval "); /* also copy trailing '\0' */ strncpy(mallocedval + strlen(mallocedval), str, slen + 1); } *tobeinserted_p = mallocedval; *malloced_p = true; - free(str); } break; case ECPGt_date: { char *str = NULL; int slen; if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { ! str = quote_postgres(PGTYPESdate_to_asc(*(date *) ((var + var->offset * element)->value)), lineno); slen = strlen(str); ! if (!(mallocedval = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [],date "), lineno))) return false; ! if (!element) strcpy(mallocedval, "array ["); strcpy(mallocedval + strlen(mallocedval), "date "); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { ! str = quote_postgres(PGTYPESdate_to_asc(*(date *) (var->value)), lineno); slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + sizeof("date ") + 1, lineno))) return false; strcpy(mallocedval, "date "); /* also copy trailing '\0' */ strncpy(mallocedval + strlen(mallocedval), str, slen + 1); } *tobeinserted_p = mallocedval; *malloced_p = true; - free(str); } break; case ECPGt_timestamp: { char *str = NULL; int slen; if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { ! str = quote_postgres(PGTYPEStimestamp_to_asc(*(timestamp *) ((var + var->offset * element)->value)), lineno); slen = strlen(str); ! if (!(mallocedval = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [], timestamp "), lineno))) return false; ! if (!element) strcpy(mallocedval, "array ["); strcpy(mallocedval + strlen(mallocedval), "timestamp "); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { ! str = quote_postgres(PGTYPEStimestamp_to_asc(*(timestamp *) (var->value)), lineno); slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + sizeof("timestamp") + 1, lineno))) return false; strcpy(mallocedval, "timestamp "); /* also copy trailing '\0' */ strncpy(mallocedval + strlen(mallocedval), str, slen + 1); } *tobeinserted_p = mallocedval; *malloced_p = true; - free(str); } break; --- 856,1177 ---- case ECPGt_numeric: { char *str = NULL; + char *tmp = NULL; int slen; numeric *nval = PGTYPESnumeric_new(); + if (!nval) + return false; + if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { if (var->type == ECPGt_numeric) ! { ! if (PGTYPESnumeric_copy((numeric *) ((var + var->offset * element)->value), nval) == -1) ! goto num_error; ! } else ! { ! if (PGTYPESnumeric_from_decimal((decimal *) ((var + var->offset * element)->value), nval) == -1) ! goto num_error; ! } str = PGTYPESnumeric_to_asc(nval, nval->dscale); ! if (!str) ! goto num_error; slen = strlen(str); ! if (!mallocedval) ! mallocedval = ECPGalloc(slen + sizeof("array [] "), lineno); ! else ! { ! tmp = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [] "), lineno); ! /* if ECPGrealloc() failed, the input pointer is still valid */ ! if (!tmp) ! free(mallocedval); ! mallocedval = tmp; ! } ! ! if (!mallocedval) ! goto num_error; ! if (element == 0) strcpy(mallocedval, "array ["); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); + free(str); + str = NULL; } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { if (var->type == ECPGt_numeric) ! { ! if (PGTYPESnumeric_copy((numeric *) (var->value), nval) == -1) ! goto num_error; ! } else ! { ! if (PGTYPESnumeric_from_decimal((decimal *) (var->value), nval) == -1) ! goto num_error; ! } str = PGTYPESnumeric_to_asc(nval, nval->dscale); ! if (!str) ! goto num_error; slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + 1, lineno))) ! goto num_error; ! memcpy(mallocedval, str, slen); mallocedval[slen] = '\0'; + free(str); } + PGTYPESnumeric_free(nval); *tobeinserted_p = mallocedval; *malloced_p = true; ! break; ! ! num_error: ! if (mallocedval) ! free(mallocedval); ! if (str) ! free(str); ! PGTYPESnumeric_free(nval); ! return false; } case ECPGt_interval: { char *str = NULL; + char *tmp = NULL; int slen; if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { ! str = PGTYPESinterval_to_asc((interval *) ((var + var->offset * element)->value)); ! if (!str) ! return false; ! tmp = quote_postgres(str, lineno); ! free(str); ! if (!tmp) ! return false; ! str = tmp; slen = strlen(str); ! if (!mallocedval) ! mallocedval = ECPGalloc(slen + sizeof("array [],interval "), lineno); ! else ! { ! tmp = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [],interval "), lineno); ! /* if ECPGrealloc() failed, the input pointer is still valid */ ! if (!tmp) ! free(mallocedval); ! mallocedval = tmp; ! } ! ! if (!mallocedval) ! { ! free(str); return false; + } ! if (element == 0) strcpy(mallocedval, "array ["); strcpy(mallocedval + strlen(mallocedval), "interval "); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); + free(str); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { ! str = PGTYPESinterval_to_asc((interval *) (var->value)); ! if (!str) ! return false; ! tmp = quote_postgres(str, lineno); ! free(str); ! if (!tmp) ! return false; ! str = tmp; slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + sizeof("interval ") + 1, lineno))) + { + free(str); return false; + } strcpy(mallocedval, "interval "); /* also copy trailing '\0' */ strncpy(mallocedval + strlen(mallocedval), str, slen + 1); + free(str); } *tobeinserted_p = mallocedval; *malloced_p = true; } break; case ECPGt_date: { char *str = NULL; + char *tmp = NULL; int slen; if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { ! str = PGTYPESdate_to_asc(*(date *) ((var + var->offset * element)->value)); ! if (!str) ! return false; ! tmp = quote_postgres(str, lineno); ! free(str); ! if (!tmp) ! return false; ! str = tmp; slen = strlen(str); ! if (!mallocedval) ! mallocedval = ECPGalloc(slen + sizeof("array [],date "), lineno); ! else ! { ! tmp = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [],date "), lineno); ! /* if ECPGrealloc() failed, the input pointer is still valid */ ! if (!tmp) ! free(mallocedval); ! mallocedval = tmp; ! } ! ! if (!mallocedval) ! { ! free(str); return false; + } ! if (element == 0) strcpy(mallocedval, "array ["); strcpy(mallocedval + strlen(mallocedval), "date "); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); + free(str); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { ! str = PGTYPESdate_to_asc(*(date *) (var->value)); ! if (!str) ! return false; ! tmp = quote_postgres(str, lineno); ! free(str); ! if (!tmp) ! return false; ! str = tmp; slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + sizeof("date ") + 1, lineno))) + { + free(str); return false; + } strcpy(mallocedval, "date "); /* also copy trailing '\0' */ strncpy(mallocedval + strlen(mallocedval), str, slen + 1); + free(str); } *tobeinserted_p = mallocedval; *malloced_p = true; } break; case ECPGt_timestamp: { char *str = NULL; + char *tmp = NULL; int slen; if (var->arrsize > 1) { for (element = 0; element < var->arrsize; element++) { ! str = PGTYPEStimestamp_to_asc(*(timestamp *) ((var + var->offset * element)->value)); ! if (!str) ! return false; ! tmp = quote_postgres(str, lineno); ! free(str); ! if (!tmp) ! return false; ! str = tmp; slen = strlen(str); ! if (!mallocedval) ! mallocedval = ECPGalloc(slen + sizeof("array [], timestamp "), lineno); ! else ! { ! tmp = ECPGrealloc(mallocedval, strlen(mallocedval) + slen + sizeof("array [], timestamp "), lineno); ! /* if ECPGrealloc() failed, the input pointer is still valid */ ! if (!tmp) ! free(mallocedval); ! mallocedval = tmp; ! } ! ! if (!mallocedval) ! { ! free(str); return false; + } ! if (element == 0) strcpy(mallocedval, "array ["); strcpy(mallocedval + strlen(mallocedval), "timestamp "); strncpy(mallocedval + strlen(mallocedval), str, slen + 1); strcpy(mallocedval + strlen(mallocedval), ","); + free(str); } strcpy(mallocedval + strlen(mallocedval) - 1, "]"); } else { ! str = PGTYPEStimestamp_to_asc(*(timestamp *) (var->value)); ! if (!str) ! return false; ! tmp = quote_postgres(str, lineno); ! free(str); ! if (!tmp) ! return false; ! str = tmp; slen = strlen(str); if (!(mallocedval = ECPGalloc(slen + sizeof("timestamp") + 1, lineno))) + { + free(str); return false; + } strcpy(mallocedval, "timestamp "); /* also copy trailing '\0' */ strncpy(mallocedval + strlen(mallocedval), str, slen + 1); + free(str); } *tobeinserted_p = mallocedval; *malloced_p = true; } break;
---------------------------(end of broadcast)--------------------------- TIP 7: don't forget to increase your free space map settings