Skip to content

Commit 1f35170

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 d2e00c3 commit 1f35170

File tree

1 file changed

+129
-58
lines changed

1 file changed

+129
-58
lines changed

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

Lines changed: 129 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ static bool connectOptions2(PGconn *conn);
280280
static int connectDBStart(PGconn *conn);
281281
static int connectDBComplete(PGconn *conn);
282282
static PGconn *makeEmptyPGconn(void);
283-
static void fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
283+
static bool fillPGconn(PGconn *conn, PQconninfoOption *connOptions);
284284
static void freePGconn(PGconn *conn);
285285
static void closePGconn(PGconn *conn);
286286
static PQconninfoOption *conninfo_parse(const char *conninfo,
@@ -452,7 +452,11 @@ PQconnectStartParams(const char **keywords,
452452
/*
453453
* Move option values into conn structure
454454
*/
455-
fillPGconn(conn, connOptions);
455+
if (!fillPGconn(conn, connOptions))
456+
{
457+
PQconninfoFree(connOptions);
458+
return conn;
459+
}
456460

457461
/*
458462
* Free the option info - all is in conn now
@@ -532,59 +536,53 @@ PQconnectStart(const char *conninfo)
532536
return conn;
533537
}
534538

535-
static void
539+
/*
540+
* Move option values into conn structure
541+
*
542+
* Don't put anything cute here --- intelligence should be in
543+
* connectOptions2 ...
544+
*
545+
* Returns true on success. On failure, returns false and sets error message.
546+
*/
547+
#define FILL_CONN_OPTION(dst, name) \
548+
do \
549+
{ \
550+
tmp = conninfo_getval(connOptions, name); \
551+
if (tmp) \
552+
{ \
553+
dst = strdup(tmp); \
554+
if (dst == NULL) \
555+
goto oom_error; \
556+
} \
557+
else \
558+
dst = NULL; \
559+
} while(0)
560+
561+
static bool
536562
fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
537563
{
538564
char *tmp;
539565

540-
/*
541-
* Move option values into conn structure
542-
*
543-
* Don't put anything cute here --- intelligence should be in
544-
* connectOptions2 ...
545-
*
546-
* XXX: probably worth checking strdup() return value here...
547-
*/
548-
tmp = conninfo_getval(connOptions, "hostaddr");
549-
conn->pghostaddr = tmp ? strdup(tmp) : NULL;
550-
tmp = conninfo_getval(connOptions, "host");
551-
conn->pghost = tmp ? strdup(tmp) : NULL;
552-
tmp = conninfo_getval(connOptions, "port");
553-
conn->pgport = tmp ? strdup(tmp) : NULL;
554-
tmp = conninfo_getval(connOptions, "tty");
555-
conn->pgtty = tmp ? strdup(tmp) : NULL;
556-
tmp = conninfo_getval(connOptions, "options");
557-
conn->pgoptions = tmp ? strdup(tmp) : NULL;
558-
tmp = conninfo_getval(connOptions, "application_name");
559-
conn->appname = tmp ? strdup(tmp) : NULL;
560-
tmp = conninfo_getval(connOptions, "fallback_application_name");
561-
conn->fbappname = tmp ? strdup(tmp) : NULL;
562-
tmp = conninfo_getval(connOptions, "dbname");
563-
conn->dbName = tmp ? strdup(tmp) : NULL;
564-
tmp = conninfo_getval(connOptions, "user");
565-
conn->pguser = tmp ? strdup(tmp) : NULL;
566-
tmp = conninfo_getval(connOptions, "password");
567-
conn->pgpass = tmp ? strdup(tmp) : NULL;
568-
tmp = conninfo_getval(connOptions, "connect_timeout");
569-
conn->connect_timeout = tmp ? strdup(tmp) : NULL;
570-
tmp = conninfo_getval(connOptions, "keepalives");
571-
conn->keepalives = tmp ? strdup(tmp) : NULL;
572-
tmp = conninfo_getval(connOptions, "keepalives_idle");
573-
conn->keepalives_idle = tmp ? strdup(tmp) : NULL;
574-
tmp = conninfo_getval(connOptions, "keepalives_interval");
575-
conn->keepalives_interval = tmp ? strdup(tmp) : NULL;
576-
tmp = conninfo_getval(connOptions, "keepalives_count");
577-
conn->keepalives_count = tmp ? strdup(tmp) : NULL;
578-
tmp = conninfo_getval(connOptions, "sslmode");
579-
conn->sslmode = tmp ? strdup(tmp) : NULL;
580-
tmp = conninfo_getval(connOptions, "sslkey");
581-
conn->sslkey = tmp ? strdup(tmp) : NULL;
582-
tmp = conninfo_getval(connOptions, "sslcert");
583-
conn->sslcert = tmp ? strdup(tmp) : NULL;
584-
tmp = conninfo_getval(connOptions, "sslrootcert");
585-
conn->sslrootcert = tmp ? strdup(tmp) : NULL;
586-
tmp = conninfo_getval(connOptions, "sslcrl");
587-
conn->sslcrl = tmp ? strdup(tmp) : NULL;
566+
FILL_CONN_OPTION(conn->pghostaddr, "hostaddr");
567+
FILL_CONN_OPTION(conn->pghost, "host");
568+
FILL_CONN_OPTION(conn->pgport, "port");
569+
FILL_CONN_OPTION(conn->pgtty, "tty");
570+
FILL_CONN_OPTION(conn->pgoptions, "options");
571+
FILL_CONN_OPTION(conn->appname, "application_name");
572+
FILL_CONN_OPTION(conn->fbappname, "fallback_application_name");
573+
FILL_CONN_OPTION(conn->dbName, "dbname");
574+
FILL_CONN_OPTION(conn->pguser, "user");
575+
FILL_CONN_OPTION(conn->pgpass, "password");
576+
FILL_CONN_OPTION(conn->connect_timeout, "connect_timeout");
577+
FILL_CONN_OPTION(conn->keepalives, "keepalives");
578+
FILL_CONN_OPTION(conn->keepalives_idle, "keepalives_idle");
579+
FILL_CONN_OPTION(conn->keepalives_interval, "keepalives_interval");
580+
FILL_CONN_OPTION(conn->keepalives_count, "keepalives_count");
581+
FILL_CONN_OPTION(conn->sslmode, "sslmode");
582+
FILL_CONN_OPTION(conn->sslkey, "sslkey");
583+
FILL_CONN_OPTION(conn->sslcert, "sslcert");
584+
FILL_CONN_OPTION(conn->sslrootcert, "sslrootcert");
585+
FILL_CONN_OPTION(conn->sslcrl, "sslcrl");
588586
#ifdef USE_SSL
589587
tmp = conninfo_getval(connOptions, "requiressl");
590588
if (tmp && tmp[0] == '1')
@@ -593,18 +591,24 @@ fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
593591
if (conn->sslmode)
594592
free(conn->sslmode);
595593
conn->sslmode = strdup("require");
594+
if (!conn->sslmode)
595+
return false;
596596
}
597597
#endif
598598
#if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
599-
tmp = conninfo_getval(connOptions, "krbsrvname");
600-
conn->krbsrvname = tmp ? strdup(tmp) : NULL;
599+
FILL_CONN_OPTION(conn->krbsrvname, "krbsrvname");
601600
#endif
602601
#if defined(ENABLE_GSS) && defined(ENABLE_SSPI)
603-
tmp = conninfo_getval(connOptions, "gsslib");
604-
conn->gsslib = tmp ? strdup(tmp) : NULL;
602+
FILL_CONN_OPTION(conn->gsslib, "gsslib");
605603
#endif
606-
tmp = conninfo_getval(connOptions, "replication");
607-
conn->replication = tmp ? strdup(tmp) : NULL;
604+
FILL_CONN_OPTION(conn->replication, "replication");
605+
606+
return true;
607+
608+
oom_error:
609+
printfPQExpBuffer(&conn->errorMessage,
610+
libpq_gettext("out of memory\n"));
611+
return false;
608612
}
609613

610614
/*
@@ -637,7 +641,12 @@ connectOptions1(PGconn *conn, const char *conninfo)
637641
/*
638642
* Move option values into conn structure
639643
*/
640-
fillPGconn(conn, connOptions);
644+
if (!fillPGconn(conn, connOptions))
645+
{
646+
conn->status = CONNECTION_BAD;
647+
PQconninfoFree(connOptions);
648+
return false;
649+
}
641650

642651
/*
643652
* Free the option info - all is in conn now
@@ -667,6 +676,8 @@ connectOptions2(PGconn *conn)
667676
if (conn->dbName)
668677
free(conn->dbName);
669678
conn->dbName = strdup(conn->pguser);
679+
if (!conn->dbName)
680+
goto oom_error;
670681
}
671682

672683
/*
@@ -679,7 +690,12 @@ connectOptions2(PGconn *conn)
679690
conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
680691
conn->dbName, conn->pguser);
681692
if (conn->pgpass == NULL)
693+
{
682694
conn->pgpass = strdup(DefaultPassword);
695+
if (!conn->pgpass)
696+
goto oom_error;
697+
698+
}
683699
else
684700
conn->dot_pgpass_used = true;
685701
}
@@ -737,7 +753,11 @@ connectOptions2(PGconn *conn)
737753
#endif
738754
}
739755
else
756+
{
740757
conn->sslmode = strdup(DefaultSSLMode);
758+
if (!conn->sslmode)
759+
goto oom_error;
760+
}
741761

742762
/*
743763
* Only if we get this far is it appropriate to try to connect. (We need a
@@ -747,6 +767,12 @@ connectOptions2(PGconn *conn)
747767
conn->options_valid = true;
748768

749769
return true;
770+
771+
oom_error:
772+
conn->status = CONNECTION_BAD;
773+
printfPQExpBuffer(&conn->errorMessage,
774+
libpq_gettext("out of memory\n"));
775+
return false;
750776
}
751777

752778
/*
@@ -829,6 +855,8 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
829855
if (conn->dbName)
830856
free(conn->dbName);
831857
conn->dbName = strdup(dbName);
858+
if (!conn->dbName)
859+
goto oom_error;
832860
}
833861
}
834862

@@ -841,41 +869,53 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
841869
if (conn->pghost)
842870
free(conn->pghost);
843871
conn->pghost = strdup(pghost);
872+
if (!conn->pghost)
873+
goto oom_error;
844874
}
845875

846876
if (pgport && pgport[0] != '\0')
847877
{
848878
if (conn->pgport)
849879
free(conn->pgport);
850880
conn->pgport = strdup(pgport);
881+
if (!conn->pgport)
882+
goto oom_error;
851883
}
852884

853885
if (pgoptions && pgoptions[0] != '\0')
854886
{
855887
if (conn->pgoptions)
856888
free(conn->pgoptions);
857889
conn->pgoptions = strdup(pgoptions);
890+
if (!conn->pgoptions)
891+
goto oom_error;
858892
}
859893

860894
if (pgtty && pgtty[0] != '\0')
861895
{
862896
if (conn->pgtty)
863897
free(conn->pgtty);
864898
conn->pgtty = strdup(pgtty);
899+
if (!conn->pgtty)
900+
goto oom_error;
865901
}
866902

867903
if (login && login[0] != '\0')
868904
{
869905
if (conn->pguser)
870906
free(conn->pguser);
871907
conn->pguser = strdup(login);
908+
if (!conn->pguser)
909+
goto oom_error;
872910
}
873911

874912
if (pwd && pwd[0] != '\0')
875913
{
876914
if (conn->pgpass)
877915
free(conn->pgpass);
878916
conn->pgpass = strdup(pwd);
917+
if (!conn->pgpass)
918+
goto oom_error;
879919
}
880920

881921
/*
@@ -891,6 +931,12 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions,
891931
(void) connectDBComplete(conn);
892932

893933
return conn;
934+
935+
oom_error:
936+
conn->status = CONNECTION_BAD;
937+
printfPQExpBuffer(&conn->errorMessage,
938+
libpq_gettext("out of memory\n"));
939+
return conn;
894940
}
895941

896942

@@ -3530,7 +3576,16 @@ ldapServiceLookup(const char *purl, PQconninfoOption *options,
35303576
if (strcmp(options[i].keyword, optname) == 0)
35313577
{
35323578
if (options[i].val == NULL)
3579+
{
35333580
options[i].val = strdup(optval);
3581+
if (!options[i].val)
3582+
{
3583+
printfPQExpBuffer(errorMessage,
3584+
libpq_gettext("out of memory\n"));
3585+
free(result);
3586+
return 3;
3587+
}
3588+
}
35343589
found_keyword = true;
35353590
break;
35363591
}
@@ -3753,6 +3808,13 @@ parseServiceFile(const char *serviceFile,
37533808
{
37543809
if (options[i].val == NULL)
37553810
options[i].val = strdup(val);
3811+
if (!options[i].val)
3812+
{
3813+
printfPQExpBuffer(errorMessage,
3814+
libpq_gettext("out of memory\n"));
3815+
fclose(f);
3816+
return 3;
3817+
}
37563818
found_keyword = true;
37573819
break;
37583820
}
@@ -4180,6 +4242,14 @@ conninfo_array_parse(const char **keywords, const char **values,
41804242
if (options[k].val)
41814243
free(options[k].val);
41824244
options[k].val = strdup(str_option->val);
4245+
if (!options[k].val)
4246+
{
4247+
printfPQExpBuffer(errorMessage,
4248+
libpq_gettext("out of memory\n"));
4249+
PQconninfoFree(options);
4250+
PQconninfoFree(str_options);
4251+
return NULL;
4252+
}
41834253
break;
41844254
}
41854255
}
@@ -4199,6 +4269,7 @@ conninfo_array_parse(const char **keywords, const char **values,
41994269
printfPQExpBuffer(errorMessage,
42004270
libpq_gettext("out of memory\n"));
42014271
PQconninfoFree(options);
4272+
PQconninfoFree(str_options);
42024273
return NULL;
42034274
}
42044275
}

0 commit comments

Comments
 (0)