ecpg: fix ECPGstore_input() - Mailing list pgsql-patches

From Neil Conway
Subject ecpg: fix ECPGstore_input()
Date
Msg-id 42CBFCB1.9050405@samurai.com
Whole thread Raw
List pgsql-patches
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;


pgsql-patches by date:

Previous
From: Neil Conway
Date:
Subject: Re: User's exception plpgsql
Next
From: Andrew Dunstan
Date:
Subject: plperl SRF sanity check fix