ecpg: Improve error detection around ecpg_strdup()
authorMichael Paquier <michael@paquier.xyz>
Tue, 22 Jul 2025 23:18:36 +0000 (08:18 +0900)
committerMichael Paquier <michael@paquier.xyz>
Tue, 22 Jul 2025 23:18:36 +0000 (08:18 +0900)
Various code paths of the ECPG code did not check for memory allocation
failures, including the specific case where ecpg_strdup() considers a
NULL value given in input as a valid behavior.  strdup() returning
itself NULL on failure, there was no way to make the difference between
what could be valid and what should fail.

With the different cases in mind, ecpg_strdup() is redesigned and gains
a new optional argument, giving its callers the possibility to
differentiate allocation failures and valid cases where the caller is
giving a NULL value in input.  Most of the ECPG code does not expect a
NULL value, at the exception of ECPGget_desc() (setlocale) and
ECPGconnect(), like dbname being unspecified, with repeated strdup
calls.

The code is adapted to work with this new routine.  Note the case of
ecpg_auto_prepare(), where the code order is switched so as we handle
failures with ecpg_strdup() before manipulating any cached data,
avoiding inconsistencies.

This class of failure is unlikely a problem in practice, so no backpatch
is done.  Random OOM failures in ECPGconnect() could cause the driver to
connect to a different server than the one wanted by the caller, because
it could fallback to default values instead of the parameters defined
depending on the combinations of allocation failures and successes.

Author: Evgeniy Gorbanev <gorbanyoves@basealt.ru>
Co-authored-by: Aleksander Alekseev <aleksander@tigerdata.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/a6b193c1-6994-4d9c-9059-aca4aaf41ddd@basealt.ru

src/interfaces/ecpg/ecpglib/connect.c
src/interfaces/ecpg/ecpglib/descriptor.c
src/interfaces/ecpg/ecpglib/ecpglib_extern.h
src/interfaces/ecpg/ecpglib/execute.c
src/interfaces/ecpg/ecpglib/memory.c
src/interfaces/ecpg/ecpglib/prepare.c

index 713cbbf6360bee59d431dbdc459e6906120b1c7f..78de9f298ba62f18a7144fdeeca8c844b3dc7dec 100644 (file)
@@ -264,7 +264,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
    struct connection *this;
    int         i,
                connect_params = 0;
-   char       *dbname = name ? ecpg_strdup(name, lineno) : NULL,
+   bool        alloc_failed = (sqlca == NULL);
+   char       *dbname = name ? ecpg_strdup(name, lineno, &alloc_failed) : NULL,
               *host = NULL,
               *tmp,
               *port = NULL,
@@ -273,11 +274,12 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
    const char **conn_keywords;
    const char **conn_values;
 
-   if (sqlca == NULL)
+   if (alloc_failed)
    {
        ecpg_raise(lineno, ECPG_OUT_OF_MEMORY,
                   ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
-       ecpg_free(dbname);
+       if (dbname)
+           ecpg_free(dbname);
        return false;
    }
 
@@ -302,7 +304,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
        if (envname)
        {
            ecpg_free(dbname);
-           dbname = ecpg_strdup(envname, lineno);
+           dbname = ecpg_strdup(envname, lineno, &alloc_failed);
        }
    }
 
@@ -354,7 +356,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                tmp = strrchr(dbname + offset, '?');
                if (tmp != NULL)    /* options given */
                {
-                   options = ecpg_strdup(tmp + 1, lineno);
+                   options = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
                    *tmp = '\0';
                }
 
@@ -363,7 +365,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                {
                    if (tmp[1] != '\0') /* non-empty database name */
                    {
-                       realname = ecpg_strdup(tmp + 1, lineno);
+                       realname = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
                        connect_params++;
                    }
                    *tmp = '\0';
@@ -373,7 +375,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                if (tmp != NULL)    /* port number given */
                {
                    *tmp = '\0';
-                   port = ecpg_strdup(tmp + 1, lineno);
+                   port = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
                    connect_params++;
                }
 
@@ -407,7 +409,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                {
                    if (*(dbname + offset) != '\0')
                    {
-                       host = ecpg_strdup(dbname + offset, lineno);
+                       host = ecpg_strdup(dbname + offset, lineno, &alloc_failed);
                        connect_params++;
                    }
                }
@@ -419,7 +421,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
            tmp = strrchr(dbname, ':');
            if (tmp != NULL)    /* port number given */
            {
-               port = ecpg_strdup(tmp + 1, lineno);
+               port = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
                connect_params++;
                *tmp = '\0';
            }
@@ -427,14 +429,14 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
            tmp = strrchr(dbname, '@');
            if (tmp != NULL)    /* host name given */
            {
-               host = ecpg_strdup(tmp + 1, lineno);
+               host = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
                connect_params++;
                *tmp = '\0';
            }
 
            if (strlen(dbname) > 0)
            {
-               realname = ecpg_strdup(dbname, lineno);
+               realname = ecpg_strdup(dbname, lineno, &alloc_failed);
                connect_params++;
            }
            else
@@ -465,7 +467,18 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
     */
    conn_keywords = (const char **) ecpg_alloc((connect_params + 1) * sizeof(char *), lineno);
    conn_values = (const char **) ecpg_alloc(connect_params * sizeof(char *), lineno);
-   if (conn_keywords == NULL || conn_values == NULL)
+
+   /* Decide on a connection name */
+   if (connection_name != NULL || realname != NULL)
+   {
+       this->name = ecpg_strdup(connection_name ? connection_name : realname,
+                                lineno, &alloc_failed);
+   }
+   else
+       this->name = NULL;
+
+   /* Deal with any failed allocations above */
+   if (conn_keywords == NULL || conn_values == NULL || alloc_failed)
    {
        if (host)
            ecpg_free(host);
@@ -481,6 +494,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
            ecpg_free(conn_keywords);
        if (conn_values)
            ecpg_free(conn_values);
+       if (this->name)
+           ecpg_free(this->name);
        free(this);
        return false;
    }
@@ -515,17 +530,14 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                ecpg_free(conn_keywords);
            if (conn_values)
                ecpg_free(conn_values);
+           if (this->name)
+               ecpg_free(this->name);
            free(this);
            return false;
        }
    }
 #endif
 
-   if (connection_name != NULL)
-       this->name = ecpg_strdup(connection_name, lineno);
-   else
-       this->name = ecpg_strdup(realname, lineno);
-
    this->cache_head = NULL;
    this->prep_stmts = NULL;
 
index 651d5c8b2ed3cc4c761da3b9b770d40188308acf..466428edfebee89bf99d70b24fdf48cf24ccc03d 100644 (file)
@@ -240,8 +240,9 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
                act_tuple;
    struct variable data_var;
    struct sqlca_t *sqlca = ECPGget_sqlca();
+   bool        alloc_failed = (sqlca == NULL);
 
-   if (sqlca == NULL)
+   if (alloc_failed)
    {
        ecpg_raise(lineno, ECPG_OUT_OF_MEMORY,
                   ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
@@ -493,7 +494,14 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
 #ifdef WIN32
        stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
 #endif
-       stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
+       stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL),
+                                    lineno, &alloc_failed);
+       if (alloc_failed)
+       {
+           va_end(args);
+           return false;
+       }
+
        setlocale(LC_NUMERIC, "C");
 #endif
 
index 75cc68275bdacb951ff3cbc4348970c918329d3a..949ff66cefc9291f6a9f260640a12c8c6c141a9e 100644 (file)
@@ -175,7 +175,7 @@ void        ecpg_free(void *ptr);
 bool       ecpg_init(const struct connection *con,
                      const char *connection_name,
                      const int lineno);
-char      *ecpg_strdup(const char *string, int lineno);
+char      *ecpg_strdup(const char *string, int lineno, bool *alloc_failed);
 const char *ecpg_type_name(enum ECPGttype typ);
 int            ecpg_dynamic_type(Oid type);
 int            sqlda_dynamic_type(Oid type, enum COMPAT_MODE compat);
index f52da06de9a1d4f4bc2052d6c3b3c2b686e21461..84a4a9fc5781fd4fb4f97dd3f83c1255065ea5b9 100644 (file)
@@ -860,9 +860,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
                    numeric    *nval;
 
                    if (var->arrsize > 1)
-                       mallocedval = ecpg_strdup("{", lineno);
+                       mallocedval = ecpg_strdup("{", lineno, NULL);
                    else
-                       mallocedval = ecpg_strdup("", lineno);
+                       mallocedval = ecpg_strdup("", lineno, NULL);
 
                    if (!mallocedval)
                        return false;
@@ -923,9 +923,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
                    int         slen;
 
                    if (var->arrsize > 1)
-                       mallocedval = ecpg_strdup("{", lineno);
+                       mallocedval = ecpg_strdup("{", lineno, NULL);
                    else
-                       mallocedval = ecpg_strdup("", lineno);
+                       mallocedval = ecpg_strdup("", lineno, NULL);
 
                    if (!mallocedval)
                        return false;
@@ -970,9 +970,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
                    int         slen;
 
                    if (var->arrsize > 1)
-                       mallocedval = ecpg_strdup("{", lineno);
+                       mallocedval = ecpg_strdup("{", lineno, NULL);
                    else
-                       mallocedval = ecpg_strdup("", lineno);
+                       mallocedval = ecpg_strdup("", lineno, NULL);
 
                    if (!mallocedval)
                        return false;
@@ -1017,9 +1017,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
                    int         slen;
 
                    if (var->arrsize > 1)
-                       mallocedval = ecpg_strdup("{", lineno);
+                       mallocedval = ecpg_strdup("{", lineno, NULL);
                    else
-                       mallocedval = ecpg_strdup("", lineno);
+                       mallocedval = ecpg_strdup("", lineno, NULL);
 
                    if (!mallocedval)
                        return false;
@@ -2001,7 +2001,8 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
        return false;
    }
 #endif
-   stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
+   stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno,
+                                 NULL);
    if (stmt->oldlocale == NULL)
    {
        ecpg_do_epilogue(stmt);
@@ -2030,7 +2031,14 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
        statement_type = ECPGst_execute;
    }
    else
-       stmt->command = ecpg_strdup(query, lineno);
+   {
+       stmt->command = ecpg_strdup(query, lineno, NULL);
+       if (!stmt->command)
+       {
+           ecpg_do_epilogue(stmt);
+           return false;
+       }
+   }
 
    stmt->name = NULL;
 
@@ -2042,7 +2050,12 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
        if (command)
        {
            stmt->name = stmt->command;
-           stmt->command = ecpg_strdup(command, lineno);
+           stmt->command = ecpg_strdup(command, lineno, NULL);
+           if (!stmt->command)
+           {
+               ecpg_do_epilogue(stmt);
+               return false;
+           }
        }
        else
        {
@@ -2175,7 +2188,12 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
 
            if (!is_prepared_name_set && stmt->statement_type == ECPGst_prepare)
            {
-               stmt->name = ecpg_strdup(var->value, lineno);
+               stmt->name = ecpg_strdup(var->value, lineno, NULL);
+               if (!stmt->name)
+               {
+                   ecpg_do_epilogue(stmt);
+                   return false;
+               }
                is_prepared_name_set = true;
            }
        }
index 6979be2c988acd4cc63339ee1fefd9cd97096bb9..2112e55b6e42c1c96ee2c52f91aee21eba823eee 100644 (file)
@@ -43,8 +43,15 @@ ecpg_realloc(void *ptr, long size, int lineno)
    return new;
 }
 
+/*
+ * Wrapper for strdup(), with NULL in input treated as a correct case.
+ *
+ * "alloc_failed" can be optionally specified by the caller to check for
+ * allocation failures.  The caller is responsible for its initialization,
+ * as ecpg_strdup() may be called repeatedly across multiple allocations.
+ */
 char *
-ecpg_strdup(const char *string, int lineno)
+ecpg_strdup(const char *string, int lineno, bool *alloc_failed)
 {
    char       *new;
 
@@ -54,6 +61,8 @@ ecpg_strdup(const char *string, int lineno)
    new = strdup(string);
    if (!new)
    {
+       if (alloc_failed)
+           *alloc_failed = true;
        ecpg_raise(lineno, ECPG_OUT_OF_MEMORY, ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
        return NULL;
    }
index ea1146f520f34e64f61603355412622181ad0c15..dd6fd1fe7f40777ab1f18cab537ea2bb9b1f2877 100644 (file)
@@ -85,9 +85,22 @@ ecpg_register_prepared_stmt(struct statement *stmt)
    /* create statement */
    prep_stmt->lineno = lineno;
    prep_stmt->connection = con;
-   prep_stmt->command = ecpg_strdup(stmt->command, lineno);
+   prep_stmt->command = ecpg_strdup(stmt->command, lineno, NULL);
+   if (!prep_stmt->command)
+   {
+       ecpg_free(prep_stmt);
+       ecpg_free(this);
+       return false;
+   }
    prep_stmt->inlist = prep_stmt->outlist = NULL;
-   this->name = ecpg_strdup(stmt->name, lineno);
+   this->name = ecpg_strdup(stmt->name, lineno, NULL);
+   if (!this->name)
+   {
+       ecpg_free(prep_stmt->command);
+       ecpg_free(prep_stmt);
+       ecpg_free(this);
+       return false;
+   }
    this->stmt = prep_stmt;
    this->prepared = true;
 
@@ -177,14 +190,27 @@ prepare_common(int lineno, struct connection *con, const char *name, const char
    /* create statement */
    stmt->lineno = lineno;
    stmt->connection = con;
-   stmt->command = ecpg_strdup(variable, lineno);
+   stmt->command = ecpg_strdup(variable, lineno, NULL);
+   if (!stmt->command)
+   {
+       ecpg_free(stmt);
+       ecpg_free(this);
+       return false;
+   }
    stmt->inlist = stmt->outlist = NULL;
 
    /* if we have C variables in our statement replace them with '?' */
    replace_variables(&(stmt->command), lineno);
 
    /* add prepared statement to our list */
-   this->name = ecpg_strdup(name, lineno);
+   this->name = ecpg_strdup(name, lineno, NULL);
+   if (!this->name)
+   {
+       ecpg_free(stmt->command);
+       ecpg_free(stmt);
+       ecpg_free(this);
+       return false;
+   }
    this->stmt = stmt;
 
    /* and finally really prepare the statement */
@@ -540,7 +566,9 @@ AddStmtToCache(int lineno,      /* line # of statement */
    /* add the query to the entry */
    entry = &stmtCacheEntries[entNo];
    entry->lineno = lineno;
-   entry->ecpgQuery = ecpg_strdup(ecpgQuery, lineno);
+   entry->ecpgQuery = ecpg_strdup(ecpgQuery, lineno, NULL);
+   if (!entry->ecpgQuery)
+       return -1;
    entry->connection = connection;
    entry->execs = 0;
    memcpy(entry->stmtID, stmtID, sizeof(entry->stmtID));
@@ -567,6 +595,9 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha
        ecpg_log("ecpg_auto_prepare on line %d: statement found in cache; entry %d\n", lineno, entNo);
 
        stmtID = stmtCacheEntries[entNo].stmtID;
+       *name = ecpg_strdup(stmtID, lineno, NULL);
+       if (*name == NULL)
+           return false;
 
        con = ecpg_get_connection(connection_name);
        prep = ecpg_find_prepared_statement(stmtID, con, NULL);
@@ -574,7 +605,6 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha
        if (!prep && !prepare_common(lineno, con, stmtID, query))
            return false;
 
-       *name = ecpg_strdup(stmtID, lineno);
    }
    else
    {
@@ -584,6 +614,9 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha
 
        /* generate a statement ID */
        sprintf(stmtID, "ecpg%d", nextStmtID++);
+       *name = ecpg_strdup(stmtID, lineno, NULL);
+       if (*name == NULL)
+           return false;
 
        if (!ECPGprepare(lineno, connection_name, 0, stmtID, query))
            return false;
@@ -591,8 +624,6 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha
        entNo = AddStmtToCache(lineno, stmtID, connection_name, compat, query);
        if (entNo < 0)
            return false;
-
-       *name = ecpg_strdup(stmtID, lineno);
    }
 
    /* increase usage counter */