Skip to content

Commit 5053ad2

Browse files
hlinnakahlinnaka
committed
Check return value of strdup() in libpq connection option parsing.
An out-of-memory in most of these would lead to strange behavior, like connecting to a different database than intended, but some would lead to an outright segfault. Alex Shulgin and me. Backpatch to all supported versions.
1 parent 400a4c3 commit 5053ad2

File tree

1 file changed

+133
-54
lines changed

1 file changed

+133
-54
lines changed

‎src/interfaces/libpq/fe-connect.c‎

Lines changed: 133 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ static int connectDBStart(PGconn *conn);
286286
static int connectDBComplete(PGconn *conn);
287287
static PGPing internal_ping(PGconn *conn);
288288
static PGconn *makeEmptyPGconn(void);
289-
static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
289+
static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
290290
static void freePGconn(PGconn *conn);
291291
static void closePGconn(PGconn *conn);
292292
static PQconninfoOption *conninfo_parse(const char *conninfo,
@@ -494,7 +494,11 @@ PQconnectStartParams(const char **keywords,
494494
/*
495495
* Move option values into conn structure
496496
*/
497-
fillPGconn(conn, connOptions);
497+
if (!fillPGconn(conn, connOptions))
498+
{
499+
PQconninfoFree(connOptions);
500+
return conn;
501+
}
498502

499503
/*
500504
* Free the option info - all is in conn now
@@ -574,7 +578,29 @@ PQconnectStart(const char *conninfo)
574578
return conn;
575579
}
576580

577-
static void
581+
/*
582+
* Move option values into conn structure
583+
*
584+
* Don't put anything cute here --- intelligence should be in
585+
* connectOptions2 ...
586+
*
587+
* Returns true on success. On failure, returns false and sets error message.
588+
*/
589+
#define FILL_CONN_OPTION(dst, name) \
590+
do \
591+
{ \
592+
tmp = conninfo_getval(connOptions, name); \
593+
if (tmp) \
594+
{ \
595+
dst = strdup(tmp); \
596+
if (dst == NULL) \
597+
goto oom_error; \
598+
} \
599+
else \
600+
dst = NULL; \
601+
} while(0)
602+
603+
static bool
578604
fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
579605
{
580606
char *tmp;
@@ -587,48 +613,27 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
587613
*
588614
* XXX: probably worth checking strdup() return value here...
589615
*/
590-
tmp = conninfo_getval(connOptions, "hostaddr");
591-
conn->pghostaddr = tmp ? strdup(tmp) : NULL;
592-
tmp = conninfo_getval(connOptions, "host");
593-
conn->pghost = tmp ? strdup(tmp) : NULL;
594-
tmp = conninfo_getval(connOptions, "port");
595-
conn->pgport = tmp ? strdup(tmp) : NULL;
596-
tmp = conninfo_getval(connOptions, "tty");
597-
conn->pgtty = tmp ? strdup(tmp) : NULL;
598-
tmp = conninfo_getval(connOptions, "options");
599-
conn->pgoptions = tmp ? strdup(tmp) : NULL;
600-
tmp = conninfo_getval(connOptions, "application_name");
601-
conn->appname = tmp ? strdup(tmp) : NULL;
602-
tmp = conninfo_getval(connOptions, "fallback_application_name");
603-
conn->fbappname = tmp ? strdup(tmp) : NULL;
604-
tmp = conninfo_getval(connOptions, "dbname");
605-
conn->dbName = tmp ? strdup(tmp) : NULL;
606-
tmp = conninfo_getval(connOptions, "user");
607-
conn->pguser = tmp ? strdup(tmp) : NULL;
608-
tmp = conninfo_getval(connOptions, "password");
609-
conn->pgpass = tmp ? strdup(tmp) : NULL;
610-
tmp = conninfo_getval(connOptions, "connect_timeout");
611-
conn->connect_timeout = tmp ? strdup(tmp) : NULL;
612-
tmp = conninfo_getval(connOptions, "client_encoding");
613-
conn->client_encoding_initial = tmp ? strdup(tmp) : NULL;
614-
tmp = conninfo_getval(connOptions, "keepalives");
615-
conn->keepalives = tmp ? strdup(tmp) : NULL;
616-
tmp = conninfo_getval(connOptions, "keepalives_idle");
617-
conn->keepalives_idle = tmp ? strdup(tmp) : NULL;
618-
tmp = conninfo_getval(connOptions, "keepalives_interval");
619-
conn->keepalives_interval = tmp ? strdup(tmp) : NULL;
620-
tmp = conninfo_getval(connOptions, "keepalives_count");
621-
conn->keepalives_count = tmp ? strdup(tmp) : NULL;
622-
tmp = conninfo_getval(connOptions, "sslmode");
623-
conn->sslmode = tmp ? strdup(tmp) : NULL;
624-
tmp = conninfo_getval(connOptions, "sslkey");
625-
conn->sslkey = tmp ? strdup(tmp) : NULL;
626-
tmp = conninfo_getval(connOptions, "sslcert");
627-
conn->sslcert = tmp ? strdup(tmp) : NULL;
628-
tmp = conninfo_getval(connOptions, "sslrootcert");
629-
conn->sslrootcert = tmp ? strdup(tmp) : NULL;
630-
tmp = conninfo_getval(connOptions, "sslcrl");
631-
conn->sslcrl = tmp ? strdup(tmp) : NULL;
616+
FILL_CONN_OPTION(conn->pghostaddr, "hostaddr");
617+
FILL_CONN_OPTION(conn->pghost, "host");
618+
FILL_CONN_OPTION(conn->pgport, "port");
619+
FILL_CONN_OPTION(conn->pgtty, "tty");
620+
FILL_CONN_OPTION(conn->pgoptions, "options");
621+
FILL_CONN_OPTION(conn->appname, "application_name");
622+
FILL_CONN_OPTION(conn->fbappname, "fallback_application_name");
623+
FILL_CONN_OPTION(conn->dbName, "dbname");
624+
FILL_CONN_OPTION(conn->pguser, "user");
625+
FILL_CONN_OPTION(conn->pgpass, "password");
626+
FILL_CONN_OPTION(conn->connect_timeout, "connect_timeout");
627+
FILL_CONN_OPTION(conn->client_encoding_initial, "client_encoding");
628+
FILL_CONN_OPTION(conn->keepalives, "keepalives");
629+
FILL_CONN_OPTION(conn->keepalives_idle, "keepalives_idle");
630+
FILL_CONN_OPTION(conn->keepalives_interval, "keepalives_interval");
631+
FILL_CONN_OPTION(conn->keepalives_count, "keepalives_count");
632+
FILL_CONN_OPTION(conn->sslmode, "sslmode");
633+
FILL_CONN_OPTION(conn->sslkey, "sslkey");
634+
FILL_CONN_OPTION(conn->sslcert, "sslcert");
635+
FILL_CONN_OPTION(conn->sslrootcert, "sslrootcert");
636+
FILL_CONN_OPTION(conn->sslcrl, "sslcrl");
632637
#ifdef USE_SSL
633638
tmp = conninfo_getval(connOptions, "requiressl");
634639
if (tmp && tmp[0] == '1')
@@ -637,20 +642,25 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
637642
if (conn->sslmode)
638643
free(conn->sslmode);
639644
conn->sslmode = strdup("require");
645+
if (!conn->sslmode)
646+
return false;
640647
}
641648
#endif
642-
tmp = conninfo_getval(connOptions, "requirepeer");
643-
conn->requirepeer = tmp ? strdup(tmp) : NULL;
649+
FILL_CONN_OPTION(conn->requirepeer, "requirepeer");
644650
#if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
645-
tmp = conninfo_getval(connOptions, "krbsrvname");
646-
conn->krbsrvname = tmp ? strdup(tmp) : NULL;
651+
FILL_CONN_OPTION(conn->krbsrvname, "krbsrvname");
647652
#endif
648653
#if defined(ENABLE_GSS) && defined(ENABLE_SSPI)
649-
tmp = conninfo_getval(connOptions, "gsslib");
650-
conn->gsslib = tmp ? strdup(tmp) : NULL;
654+
FILL_CONN_OPTION(conn->gsslib, "gsslib");
651655
#endif
652-
tmp = conninfo_getval(connOptions, "replication");
653-
conn->replication = tmp ? strdup(tmp) : NULL;
656+
FILL_CONN_OPTION(conn->replication, "replication");
657+
658+
return true;
659+
660+
oom_error:
661+
printfPQExpBuffer(&conn->errorMessage,
662+
libpq_gettext("out of memory\n"));
663+
return false;
654664
}
655665

656666
/*
@@ -683,7 +693,12 @@ connectOptions1(PGconn *conn, const char *conninfo)
683693
/*
684694
* Move option values into conn structure
685695
*/
686-
fillPGconn(conn, connOptions);
696+
if (!fillPGconn(conn, connOptions))
697+
{
698+
conn->status = CONNECTION_BAD;
699+
PQconninfoFree(connOptions);
700+
return false;
701+
}
687702

688703
/*
689704
* Free the option info - all is in conn now
@@ -713,6 +728,8 @@ connectOptions2(PGconn *conn)
713728
if (conn->dbName)
714729
free(conn->dbName);
715730
conn->dbName = strdup(conn->pguser);
731+
if (!conn->dbName)
732+
goto oom_error;
716733
}
717734

718735
/*
@@ -725,7 +742,12 @@ connectOptions2(PGconn *conn)
725742
conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
726743
conn->dbName, conn->pguser);
727744
if (conn->pgpass == NULL)
745+
{
728746
conn->pgpass = strdup(DefaultPassword);
747+
if (!conn->pgpass)
748+
goto oom_error;
749+
750+
}
729751
else
730752
conn->dot_pgpass_used = true;
731753
}
@@ -783,7 +805,11 @@ connectOptions2(PGconn *conn)
783805
#endif
784806
}
785807
else
808+
{
786809
conn->sslmode = strdup(DefaultSSLMode);
810+
if (!conn->sslmode)
811+
goto oom_error;
812+
}
787813

788814
/*
789815
* Resolve special "auto" client_encoding from the locale
@@ -793,6 +819,8 @@ connectOptions2(PGconn *conn)
793819
{
794820
free(conn->client_encoding_initial);
795821
conn->client_encoding_initial = strdup(pg_encoding_to_char(pg_get_encoding_from_locale(NULL, true)));
822+
if (!conn->client_encoding_initial)
823+
goto oom_error;
796824
}
797825

798826
/*
@@ -803,6 +831,12 @@ connectOptions2(PGconn *conn)
803831
conn->options_valid = true;
804832

805833
return true;
834+
835+
oom_error:
836+
conn->status = CONNECTION_BAD;
837+
printfPQExpBuffer(&conn->errorMessage,
838+
libpq_gettext("out of memory\n"));
839+
return false;
806840
}
807841

808842
/*
@@ -885,6 +919,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
885919
if (conn->dbName)
886920
free(conn->dbName);
887921
conn->dbName = strdup(dbName);
922+
if (!conn->dbName)
923+
goto oom_error;
888924
}
889925
}
890926

@@ -897,41 +933,53 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
897933
if (conn->pghost)
898934
free(conn->pghost);
899935
conn->pghost = strdup(pghost);
936+
if (!conn->pghost)
937+
goto oom_error;
900938
}
901939

902940
if (pgport && pgport[0] != '\0')
903941
{
904942
if (conn->pgport)
905943
free(conn->pgport);
906944
conn->pgport = strdup(pgport);
945+
if (!conn->pgport)
946+
goto oom_error;
907947
}
908948

909949
if (pgoptions && pgoptions[0] != '\0')
910950
{
911951
if (conn->pgoptions)
912952
free(conn->pgoptions);
913953
conn->pgoptions = strdup(pgoptions);
954+
if (!conn->pgoptions)
955+
goto oom_error;
914956
}
915957

916958
if (pgtty && pgtty[0] != '\0')
917959
{
918960
if (conn->pgtty)
919961
free(conn->pgtty);
920962
conn->pgtty = strdup(pgtty);
963+
if (!conn->pgtty)
964+
goto oom_error;
921965
}
922966

923967
if (login && login[0] != '\0')
924968
{
925969
if (conn->pguser)
926970
free(conn->pguser);
927971
conn->pguser = strdup(login);
972+
if (!conn->pguser)
973+
goto oom_error;
928974
}
929975

930976
if (pwd && pwd[0] != '\0')
931977
{
932978
if (conn->pgpass)
933979
free(conn->pgpass);
934980
conn->pgpass = strdup(pwd);
981+
if (!conn->pgpass)
982+
goto oom_error;
935983
}
936984

937985
/*
@@ -947,6 +995,12 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
947995
(void) connectDBComplete(conn);
948996

949997
return conn;
998+
999+
oom_error:
1000+
conn->status = CONNECTION_BAD;
1001+
printfPQExpBuffer(&conn->errorMessage,
1002+
libpq_gettext("out of memory\n"));
1003+
return conn;
9501004
}
9511005

9521006

@@ -3761,7 +3815,16 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
37613815
if (strcmp(options[i].keyword, optname) == 0)
37623816
{
37633817
if (options[i].val == NULL)
3818+
{
37643819
options[i].val = strdup(optval);
3820+
if (!options[i].val)
3821+
{
3822+
printfPQExpBuffer(errorMessage,
3823+
libpq_gettext("out of memory\n"));
3824+
free(result);
3825+
return 3;
3826+
}
3827+
}
37653828
found_keyword = true;
37663829
break;
37673830
}
@@ -3984,6 +4047,13 @@ parseServiceFile(const char *serviceFile,
39844047
{
39854048
if (options[i].val == NULL)
39864049
options[i].val = strdup(val);
4050+
if (!options[i].val)
4051+
{
4052+
printfPQExpBuffer(errorMessage,
4053+
libpq_gettext("out of memory\n"));
4054+
fclose(f);
4055+
return 3;
4056+
}
39874057
found_keyword = true;
39884058
break;
39894059
}
@@ -4411,6 +4481,14 @@ conninfo_array_parse(const char **keywords, const char **values,
44114481
if (options[k].val)
44124482
free(options[k].val);
44134483
options[k].val = strdup(str_option->val);
4484+
if (!options[k].val)
4485+
{
4486+
printfPQExpBuffer(errorMessage,
4487+
libpq_gettext("out of memory\n"));
4488+
PQconninfoFree(options);
4489+
PQconninfoFree(str_options);
4490+
return NULL;
4491+
}
44144492
break;
44154493
}
44164494
}
@@ -4430,6 +4508,7 @@ conninfo_array_parse(const char **keywords, const char **values,
44304508
printfPQExpBuffer(errorMessage,
44314509
libpq_gettext("out of memory\n"));
44324510
PQconninfoFree(options);
4511+
PQconninfoFree(str_options);
44334512
return NULL;
44344513
}
44354514
}

0 commit comments

Comments
 (0)