Hi Evgeniy,
> In case of out_of_memory, the ecpg_strdup function may return NULL.
> Checks should be added in src/interfaces/ecpg/ecpglib/execute.c.
> Patch attached.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
The patch looks correct, but I believe it's incomplete. It misses
several other places where ecpg_strdup() is called without proper
checks. A correct patch would look like the one attached.
While working on it I noticed a potentially problematic strcmp call,
marked with XXX in the patch. I didn't address this issue in v2.
Thoughts?
From 4af966774361eac189e011d92e985883ee03ee5a Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksan...@timescale.com>
Date: Fri, 11 Jul 2025 17:59:50 +0300
Subject: [PATCH v2] Add proper checks for ecpg_strdup() return value
Evgeniy Gorbanev, Aleksander Alekseev. Reviewed by TODO FIXME.
---
src/interfaces/ecpg/ecpglib/connect.c | 26 ++++++++++++++++------
src/interfaces/ecpg/ecpglib/execute.c | 17 +++++++++++++++
src/interfaces/ecpg/ecpglib/prepare.c | 31 +++++++++++++++++++++++++++
3 files changed, 68 insertions(+), 6 deletions(-)
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 2bbb70333dc..fc1034ccd7d 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -58,6 +58,7 @@ ecpg_get_connection_nr(const char *connection_name)
for (con = all_connections; con != NULL; con = con->next)
{
+ /* XXX strcmp() will segfault if con->name is NULL */
if (strcmp(connection_name, con->name) == 0)
break;
}
@@ -460,7 +461,21 @@ 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)
+
+ /* Allocate memory for connection name */
+ if (connection_name != NULL)
+ this->name = ecpg_strdup(connection_name, lineno);
+ else
+ this->name = ecpg_strdup(realname, lineno);
+
+ /*
+ * Note that NULL is a correct value for realname and ecpg_strdup(NULL,
+ * ...) just returns NULL. For named reasons the ckecks for this->name are
+ * a bit complicated here.
+ */
+ if (conn_keywords == NULL || conn_values == NULL ||
+ (connection_name != NULL && this->name == NULL) || /* first call failed */
+ (connection_name == NULL && realname != NULL && this->name == NULL)) /* second call failed */
{
if (host)
ecpg_free(host);
@@ -476,6 +491,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;
}
@@ -510,17 +527,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;
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index f52da06de9a..6524c646c13 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -2030,7 +2030,14 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
statement_type = ECPGst_execute;
}
else
+ {
stmt->command = ecpg_strdup(query, lineno);
+ if (!stmt->command)
+ {
+ ecpg_do_epilogue(stmt);
+ return false;
+ }
+ }
stmt->name = NULL;
@@ -2043,6 +2050,11 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
{
stmt->name = stmt->command;
stmt->command = ecpg_strdup(command, lineno);
+ if (!stmt->command)
+ {
+ ecpg_do_epilogue(stmt);
+ return false;
+ }
}
else
{
@@ -2176,6 +2188,11 @@ 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);
+ if (!stmt->name)
+ {
+ ecpg_do_epilogue(stmt);
+ return false;
+ }
is_prepared_name_set = true;
}
}
diff --git a/src/interfaces/ecpg/ecpglib/prepare.c b/src/interfaces/ecpg/ecpglib/prepare.c
index ea1146f520f..7b6019deed7 100644
--- a/src/interfaces/ecpg/ecpglib/prepare.c
+++ b/src/interfaces/ecpg/ecpglib/prepare.c
@@ -86,8 +86,21 @@ ecpg_register_prepared_stmt(struct statement *stmt)
prep_stmt->lineno = lineno;
prep_stmt->connection = con;
prep_stmt->command = ecpg_strdup(stmt->command, lineno);
+ 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);
+ if (!this->name)
+ {
+ ecpg_free(prep_stmt->command);
+ ecpg_free(prep_stmt);
+ ecpg_free(this);
+ return false;
+ }
this->stmt = prep_stmt;
this->prepared = true;
@@ -178,6 +191,12 @@ prepare_common(int lineno, struct connection *con, const char *name, const char
stmt->lineno = lineno;
stmt->connection = con;
stmt->command = ecpg_strdup(variable, lineno);
+ 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 '?' */
@@ -185,6 +204,13 @@ prepare_common(int lineno, struct connection *con, const char *name, const char
/* add prepared statement to our list */
this->name = ecpg_strdup(name, lineno);
+ if (!this->name)
+ {
+ ecpg_free(stmt->command);
+ ecpg_free(stmt);
+ ecpg_free(this);
+ return false;
+ }
this->stmt = stmt;
/* and finally really prepare the statement */
@@ -541,6 +567,8 @@ AddStmtToCache(int lineno, /* line # of statement */
entry = &stmtCacheEntries[entNo];
entry->lineno = lineno;
entry->ecpgQuery = ecpg_strdup(ecpgQuery, lineno);
+ if (!entry->ecpgQuery)
+ return -1;
entry->connection = connection;
entry->execs = 0;
memcpy(entry->stmtID, stmtID, sizeof(entry->stmtID));
@@ -595,6 +623,9 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha
*name = ecpg_strdup(stmtID, lineno);
}
+ if (!*name)
+ return false;
+
/* increase usage counter */
stmtCacheEntries[entNo].execs++;
--
2.43.0
