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

Reply via email to