Skip to content

Commit a6eabec

Browse files
michaelpqAleksander Alekseev
michaelpq
and
Aleksander Alekseev
committed
ecpg: Improve error detection around ecpg_strdup()
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
1 parent a7ca73a commit a6eabec

File tree

6 files changed

+119
-41
lines changed

6 files changed

+119
-41
lines changed

‎src/interfaces/ecpg/ecpglib/connect.c

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
264264
struct connection *this;
265265
int i,
266266
connect_params = 0;
267-
char *dbname = name ? ecpg_strdup(name, lineno) : NULL,
267+
bool alloc_failed = (sqlca == NULL);
268+
char *dbname = name ? ecpg_strdup(name, lineno, &alloc_failed) : NULL,
268269
*host = NULL,
269270
*tmp,
270271
*port = NULL,
@@ -273,11 +274,12 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
273274
const char **conn_keywords;
274275
const char **conn_values;
275276

276-
if (sqlca == NULL)
277+
if (alloc_failed)
277278
{
278279
ecpg_raise(lineno, ECPG_OUT_OF_MEMORY,
279280
ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
280-
ecpg_free(dbname);
281+
if (dbname)
282+
ecpg_free(dbname);
281283
return false;
282284
}
283285

@@ -302,7 +304,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
302304
if (envname)
303305
{
304306
ecpg_free(dbname);
305-
dbname = ecpg_strdup(envname, lineno);
307+
dbname = ecpg_strdup(envname, lineno, &alloc_failed);
306308
}
307309
}
308310

@@ -354,7 +356,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
354356
tmp = strrchr(dbname + offset, '?');
355357
if (tmp != NULL) /* options given */
356358
{
357-
options = ecpg_strdup(tmp + 1, lineno);
359+
options = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
358360
*tmp = '\0';
359361
}
360362

@@ -363,7 +365,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
363365
{
364366
if (tmp[1] != '\0') /* non-empty database name */
365367
{
366-
realname = ecpg_strdup(tmp + 1, lineno);
368+
realname = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
367369
connect_params++;
368370
}
369371
*tmp = '\0';
@@ -373,7 +375,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
373375
if (tmp != NULL) /* port number given */
374376
{
375377
*tmp = '\0';
376-
port = ecpg_strdup(tmp + 1, lineno);
378+
port = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
377379
connect_params++;
378380
}
379381

@@ -407,7 +409,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
407409
{
408410
if (*(dbname + offset) != '\0')
409411
{
410-
host = ecpg_strdup(dbname + offset, lineno);
412+
host = ecpg_strdup(dbname + offset, lineno, &alloc_failed);
411413
connect_params++;
412414
}
413415
}
@@ -419,22 +421,22 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
419421
tmp = strrchr(dbname, ':');
420422
if (tmp != NULL) /* port number given */
421423
{
422-
port = ecpg_strdup(tmp + 1, lineno);
424+
port = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
423425
connect_params++;
424426
*tmp = '\0';
425427
}
426428

427429
tmp = strrchr(dbname, '@');
428430
if (tmp != NULL) /* host name given */
429431
{
430-
host = ecpg_strdup(tmp + 1, lineno);
432+
host = ecpg_strdup(tmp + 1, lineno, &alloc_failed);
431433
connect_params++;
432434
*tmp = '\0';
433435
}
434436

435437
if (strlen(dbname) > 0)
436438
{
437-
realname = ecpg_strdup(dbname, lineno);
439+
realname = ecpg_strdup(dbname, lineno, &alloc_failed);
438440
connect_params++;
439441
}
440442
else
@@ -465,7 +467,18 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
465467
*/
466468
conn_keywords = (const char **) ecpg_alloc((connect_params + 1) * sizeof(char *), lineno);
467469
conn_values = (const char **) ecpg_alloc(connect_params * sizeof(char *), lineno);
468-
if (conn_keywords == NULL || conn_values == NULL)
470+
471+
/* Decide on a connection name */
472+
if (connection_name != NULL || realname != NULL)
473+
{
474+
this->name = ecpg_strdup(connection_name ? connection_name : realname,
475+
lineno, &alloc_failed);
476+
}
477+
else
478+
this->name = NULL;
479+
480+
/* Deal with any failed allocations above */
481+
if (conn_keywords == NULL || conn_values == NULL || alloc_failed)
469482
{
470483
if (host)
471484
ecpg_free(host);
@@ -481,6 +494,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
481494
ecpg_free(conn_keywords);
482495
if (conn_values)
483496
ecpg_free(conn_values);
497+
if (this->name)
498+
ecpg_free(this->name);
484499
free(this);
485500
return false;
486501
}
@@ -515,17 +530,14 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
515530
ecpg_free(conn_keywords);
516531
if (conn_values)
517532
ecpg_free(conn_values);
533+
if (this->name)
534+
ecpg_free(this->name);
518535
free(this);
519536
return false;
520537
}
521538
}
522539
#endif
523540

524-
if (connection_name != NULL)
525-
this->name = ecpg_strdup(connection_name, lineno);
526-
else
527-
this->name = ecpg_strdup(realname, lineno);
528-
529541
this->cache_head = NULL;
530542
this->prep_stmts = NULL;
531543

‎src/interfaces/ecpg/ecpglib/descriptor.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,9 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
240240
act_tuple;
241241
struct variable data_var;
242242
struct sqlca_t *sqlca = ECPGget_sqlca();
243+
bool alloc_failed = (sqlca == NULL);
243244

244-
if (sqlca == NULL)
245+
if (alloc_failed)
245246
{
246247
ecpg_raise(lineno, ECPG_OUT_OF_MEMORY,
247248
ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
@@ -493,7 +494,14 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
493494
#ifdef WIN32
494495
stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
495496
#endif
496-
stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
497+
stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL),
498+
lineno, &alloc_failed);
499+
if (alloc_failed)
500+
{
501+
va_end(args);
502+
return false;
503+
}
504+
497505
setlocale(LC_NUMERIC, "C");
498506
#endif
499507

‎src/interfaces/ecpg/ecpglib/ecpglib_extern.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ void ecpg_free(void *ptr);
175175
bool ecpg_init(const struct connection *con,
176176
const char *connection_name,
177177
const int lineno);
178-
char *ecpg_strdup(const char *string, int lineno);
178+
char *ecpg_strdup(const char *string, int lineno, bool *alloc_failed);
179179
const char *ecpg_type_name(enum ECPGttype typ);
180180
int ecpg_dynamic_type(Oid type);
181181
int sqlda_dynamic_type(Oid type, enum COMPAT_MODE compat);

‎src/interfaces/ecpg/ecpglib/execute.c

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
860860
numeric *nval;
861861

862862
if (var->arrsize > 1)
863-
mallocedval = ecpg_strdup("{", lineno);
863+
mallocedval = ecpg_strdup("{", lineno, NULL);
864864
else
865-
mallocedval = ecpg_strdup("", lineno);
865+
mallocedval = ecpg_strdup("", lineno, NULL);
866866

867867
if (!mallocedval)
868868
return false;
@@ -923,9 +923,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
923923
int slen;
924924

925925
if (var->arrsize > 1)
926-
mallocedval = ecpg_strdup("{", lineno);
926+
mallocedval = ecpg_strdup("{", lineno, NULL);
927927
else
928-
mallocedval = ecpg_strdup("", lineno);
928+
mallocedval = ecpg_strdup("", lineno, NULL);
929929

930930
if (!mallocedval)
931931
return false;
@@ -970,9 +970,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
970970
int slen;
971971

972972
if (var->arrsize > 1)
973-
mallocedval = ecpg_strdup("{", lineno);
973+
mallocedval = ecpg_strdup("{", lineno, NULL);
974974
else
975-
mallocedval = ecpg_strdup("", lineno);
975+
mallocedval = ecpg_strdup("", lineno, NULL);
976976

977977
if (!mallocedval)
978978
return false;
@@ -1017,9 +1017,9 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
10171017
int slen;
10181018

10191019
if (var->arrsize > 1)
1020-
mallocedval = ecpg_strdup("{", lineno);
1020+
mallocedval = ecpg_strdup("{", lineno, NULL);
10211021
else
1022-
mallocedval = ecpg_strdup("", lineno);
1022+
mallocedval = ecpg_strdup("", lineno, NULL);
10231023

10241024
if (!mallocedval)
10251025
return false;
@@ -2001,7 +2001,8 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
20012001
return false;
20022002
}
20032003
#endif
2004-
stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
2004+
stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno,
2005+
NULL);
20052006
if (stmt->oldlocale == NULL)
20062007
{
20072008
ecpg_do_epilogue(stmt);
@@ -2030,7 +2031,14 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
20302031
statement_type = ECPGst_execute;
20312032
}
20322033
else
2033-
stmt->command = ecpg_strdup(query, lineno);
2034+
{
2035+
stmt->command = ecpg_strdup(query, lineno, NULL);
2036+
if (!stmt->command)
2037+
{
2038+
ecpg_do_epilogue(stmt);
2039+
return false;
2040+
}
2041+
}
20342042

20352043
stmt->name = NULL;
20362044

@@ -2042,7 +2050,12 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
20422050
if (command)
20432051
{
20442052
stmt->name = stmt->command;
2045-
stmt->command = ecpg_strdup(command, lineno);
2053+
stmt->command = ecpg_strdup(command, lineno, NULL);
2054+
if (!stmt->command)
2055+
{
2056+
ecpg_do_epilogue(stmt);
2057+
return false;
2058+
}
20462059
}
20472060
else
20482061
{
@@ -2175,7 +2188,12 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
21752188

21762189
if (!is_prepared_name_set && stmt->statement_type == ECPGst_prepare)
21772190
{
2178-
stmt->name = ecpg_strdup(var->value, lineno);
2191+
stmt->name = ecpg_strdup(var->value, lineno, NULL);
2192+
if (!stmt->name)
2193+
{
2194+
ecpg_do_epilogue(stmt);
2195+
return false;
2196+
}
21792197
is_prepared_name_set = true;
21802198
}
21812199
}

‎src/interfaces/ecpg/ecpglib/memory.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,15 @@ ecpg_realloc(void *ptr, long size, int lineno)
4343
return new;
4444
}
4545

46+
/*
47+
* Wrapper for strdup(), with NULL in input treated as a correct case.
48+
*
49+
* "alloc_failed" can be optionally specified by the caller to check for
50+
* allocation failures. The caller is responsible for its initialization,
51+
* as ecpg_strdup() may be called repeatedly across multiple allocations.
52+
*/
4653
char *
47-
ecpg_strdup(const char *string, int lineno)
54+
ecpg_strdup(const char *string, int lineno, bool *alloc_failed)
4855
{
4956
char *new;
5057

@@ -54,6 +61,8 @@ ecpg_strdup(const char *string, int lineno)
5461
new = strdup(string);
5562
if (!new)
5663
{
64+
if (alloc_failed)
65+
*alloc_failed = true;
5766
ecpg_raise(lineno, ECPG_OUT_OF_MEMORY, ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
5867
return NULL;
5968
}

0 commit comments

Comments
 (0)