Skip to content

Commit d3b162a

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 c57cdc9 commit d3b162a

File tree

1 file changed

+113
-19
lines changed

1 file changed

+113
-19
lines changed

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

Lines changed: 113 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ static int connectDBStart(PGconn *conn);
333333
static int connectDBComplete(PGconn *conn);
334334
static PGPing internal_ping(PGconn *conn);
335335
static PGconn *makeEmptyPGconn(void);
336-
static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
336+
static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
337337
static void freePGconn(PGconn *conn);
338338
static void closePGconn(PGconn *conn);
339339
static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage);
@@ -585,7 +585,11 @@ PQconnectStartParams(const char *const * keywords,
585585
/*
586586
* Move option values into conn structure
587587
*/
588-
fillPGconn(conn, connOptions);
588+
if (!fillPGconn(conn, connOptions))
589+
{
590+
PQconninfoFree(connOptions);
591+
return conn;
592+
}
589593

590594
/*
591595
* Free the option info - all is in conn now
@@ -665,19 +669,19 @@ PQconnectStart(const char *conninfo)
665669
return conn;
666670
}
667671

668-
static void
672+
/*
673+
* Move option values into conn structure
674+
*
675+
* Don't put anything cute here --- intelligence should be in
676+
* connectOptions2 ...
677+
*
678+
* Returns true on success. On failure, returns false and sets error message.
679+
*/
680+
static bool
669681
fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
670682
{
671683
const internalPQconninfoOption *option;
672684

673-
/*
674-
* Move option values into conn structure
675-
*
676-
* Don't put anything cute here --- intelligence should be in
677-
* connectOptions2 ...
678-
*
679-
* XXX: probably worth checking strdup() return value here...
680-
*/
681685
for (option = PQconninfoOptions; option->keyword; option++)
682686
{
683687
const char *tmp = conninfo_getval(connOptions, option->keyword);
@@ -688,9 +692,22 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
688692

689693
if (*connmember)
690694
free(*connmember);
691-
*connmember = tmp ? strdup(tmp) : NULL;
695+
if (tmp)
696+
{
697+
*connmember = strdup(tmp);
698+
if (*connmember == NULL)
699+
{
700+
printfPQExpBuffer(&conn->errorMessage,
701+
libpq_gettext("out of memory\n"));
702+
return false;
703+
}
704+
}
705+
else
706+
*connmember = NULL;
692707
}
693708
}
709+
710+
return true;
694711
}
695712

696713
/*
@@ -723,7 +740,12 @@ connectOptions1(PGconn *conn, const char *conninfo)
723740
/*
724741
* Move option values into conn structure
725742
*/
726-
fillPGconn(conn, connOptions);
743+
if (!fillPGconn(conn, connOptions))
744+
{
745+
conn->status = CONNECTION_BAD;
746+
PQconninfoFree(connOptions);
747+
return false;
748+
}
727749

728750
/*
729751
* Free the option info - all is in conn now
@@ -753,6 +775,8 @@ connectOptions2(PGconn *conn)
753775
if (conn->dbName)
754776
free(conn->dbName);
755777
conn->dbName = strdup(conn->pguser);
778+
if (!conn->dbName)
779+
goto oom_error;
756780
}
757781

758782
/*
@@ -765,7 +789,12 @@ connectOptions2(PGconn *conn)
765789
conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
766790
conn->dbName, conn->pguser);
767791
if (conn->pgpass == NULL)
792+
{
768793
conn->pgpass = strdup(DefaultPassword);
794+
if (!conn->pgpass)
795+
goto oom_error;
796+
797+
}
769798
else
770799
conn->dot_pgpass_used = true;
771800
}
@@ -823,7 +852,11 @@ connectOptions2(PGconn *conn)
823852
#endif
824853
}
825854
else
855+
{
826856
conn->sslmode = strdup(DefaultSSLMode);
857+
if (!conn->sslmode)
858+
goto oom_error;
859+
}
827860

828861
/*
829862
* Resolve special "auto" client_encoding from the locale
@@ -833,6 +866,8 @@ connectOptions2(PGconn *conn)
833866
{
834867
free(conn->client_encoding_initial);
835868
conn->client_encoding_initial = strdup(pg_encoding_to_char(pg_get_encoding_from_locale(NULL, true)));
869+
if (!conn->client_encoding_initial)
870+
goto oom_error;
836871
}
837872

838873
/*
@@ -843,6 +878,12 @@ connectOptions2(PGconn *conn)
843878
conn->options_valid = true;
844879

845880
return true;
881+
882+
oom_error:
883+
conn->status = CONNECTION_BAD;
884+
printfPQExpBuffer(&conn->errorMessage,
885+
libpq_gettext("out of memory\n"));
886+
return false;
846887
}
847888

848889
/*
@@ -936,6 +977,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
936977
if (conn->dbName)
937978
free(conn->dbName);
938979
conn->dbName = strdup(dbName);
980+
if (!conn->dbName)
981+
goto oom_error;
939982
}
940983
}
941984

@@ -948,41 +991,53 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
948991
if (conn->pghost)
949992
free(conn->pghost);
950993
conn->pghost = strdup(pghost);
994+
if (!conn->pghost)
995+
goto oom_error;
951996
}
952997

953998
if (pgport && pgport[0] != '\0')
954999
{
9551000
if (conn->pgport)
9561001
free(conn->pgport);
9571002
conn->pgport = strdup(pgport);
1003+
if (!conn->pgport)
1004+
goto oom_error;
9581005
}
9591006

9601007
if (pgoptions && pgoptions[0] != '\0')
9611008
{
9621009
if (conn->pgoptions)
9631010
free(conn->pgoptions);
9641011
conn->pgoptions = strdup(pgoptions);
1012+
if (!conn->pgoptions)
1013+
goto oom_error;
9651014
}
9661015

9671016
if (pgtty && pgtty[0] != '\0')
9681017
{
9691018
if (conn->pgtty)
9701019
free(conn->pgtty);
9711020
conn->pgtty = strdup(pgtty);
1021+
if (!conn->pgtty)
1022+
goto oom_error;
9721023
}
9731024

9741025
if (login && login[0] != '\0')
9751026
{
9761027
if (conn->pguser)
9771028
free(conn->pguser);
9781029
conn->pguser = strdup(login);
1030+
if (!conn->pguser)
1031+
goto oom_error;
9791032
}
9801033

9811034
if (pwd && pwd[0] != '\0')
9821035
{
9831036
if (conn->pgpass)
9841037
free(conn->pgpass);
9851038
conn->pgpass = strdup(pwd);
1039+
if (!conn->pgpass)
1040+
goto oom_error;
9861041
}
9871042

9881043
/*
@@ -998,6 +1053,12 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
9981053
(void) connectDBComplete(conn);
9991054

10001055
return conn;
1056+
1057+
oom_error:
1058+
conn->status = CONNECTION_BAD;
1059+
printfPQExpBuffer(&conn->errorMessage,
1060+
libpq_gettext("out of memory\n"));
1061+
return conn;
10011062
}
10021063

10031064

@@ -3774,7 +3835,16 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
37743835
if (strcmp(options[i].keyword, optname) == 0)
37753836
{
37763837
if (options[i].val == NULL)
3838+
{
37773839
options[i].val = strdup(optval);
3840+
if (!options[i].val)
3841+
{
3842+
printfPQExpBuffer(errorMessage,
3843+
libpq_gettext("out of memory\n"));
3844+
free(result);
3845+
return 3;
3846+
}
3847+
}
37783848
found_keyword = true;
37793849
break;
37803850
}
@@ -3997,6 +4067,13 @@ parseServiceFile(const char *serviceFile,
39974067
{
39984068
if (options[i].val == NULL)
39994069
options[i].val = strdup(val);
4070+
if (!options[i].val)
4071+
{
4072+
printfPQExpBuffer(errorMessage,
4073+
libpq_gettext("out of memory\n"));
4074+
fclose(f);
4075+
return 3;
4076+
}
40004077
found_keyword = true;
40014078
break;
40024079
}
@@ -4416,6 +4493,14 @@ conninfo_array_parse(const char *const * keywords, const char *const * values,
44164493
if (options[k].val)
44174494
free(options[k].val);
44184495
options[k].val = strdup(str_option->val);
4496+
if (!options[k].val)
4497+
{
4498+
printfPQExpBuffer(errorMessage,
4499+
libpq_gettext("out of memory\n"));
4500+
PQconninfoFree(options);
4501+
PQconninfoFree(dbname_options);
4502+
return NULL;
4503+
}
44194504
break;
44204505
}
44214506
}
@@ -4599,20 +4684,22 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
45994684
{
46004685
int prefix_len;
46014686
char *p;
4602-
char *buf = strdup(uri); /* need a modifiable copy of the input
4603-
* URI */
4604-
char *start = buf;
4687+
char *buf;
4688+
char *start;
46054689
char prevchar = '\0';
46064690
char *user = NULL;
46074691
char *host = NULL;
46084692
bool retval = false;
46094693

4694+
/* need a modifiable copy of the input URI */
4695+
buf = strdup(uri);
46104696
if (buf == NULL)
46114697
{
46124698
printfPQExpBuffer(errorMessage,
46134699
libpq_gettext("out of memory\n"));
46144700
return false;
46154701
}
4702+
start = buf;
46164703

46174704
/* Skip the URI prefix */
46184705
prefix_len = uri_prefix_length(uri);
@@ -4954,15 +5041,17 @@ conninfo_uri_parse_params(char *params,
49545041
static char *
49555042
conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
49565043
{
4957-
char *buf = malloc(strlen(str) + 1);
4958-
char *p = buf;
5044+
char *buf;
5045+
char *p;
49595046
const char *q = str;
49605047

5048+
buf = malloc(strlen(str) + 1);
49615049
if (buf == NULL)
49625050
{
49635051
printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
49645052
return NULL;
49655053
}
5054+
p = buf;
49665055

49675056
for (;;)
49685057
{
@@ -5107,7 +5196,6 @@ conninfo_storeval(PQconninfoOption *connOptions,
51075196
else
51085197
{
51095198
value_copy = strdup(value);
5110-
51115199
if (value_copy == NULL)
51125200
{
51135201
printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
@@ -5667,6 +5755,12 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
56675755
ret = strdup(t);
56685756
fclose(fp);
56695757

5758+
if (!ret)
5759+
{
5760+
/* Out of memory. XXX: an error message would be nice. */
5761+
return NULL;
5762+
}
5763+
56705764
/* De-escape password. */
56715765
for (p1 = p2 = ret; *p1 != ':' && *p1 != '\0'; ++p1, ++p2)
56725766
{

0 commit comments

Comments
 (0)