Skip to content

Commit 3d357b4

Browse files
committed
Be more paranoid about null return values from libpq status functions.
PQhost() can return NULL in non-error situations, namely when a Unix-socket connection has been selected by default. That behavior is a tad debatable perhaps, but for the moment we should make sure that psql copes with it. Unfortunately, do_connect() failed to: it could pass a NULL pointer to strcmp(), resulting in crashes on most platforms. This was reported as a security issue by ChenQin of Topsec Security Team, but the consensus of the security list is that it's just a garden-variety bug with no security implications. For paranoia's sake, I made the keep_password test not trust PQuser or PQport either, even though I believe those will never return NULL given a valid PGconn. Back-patch to all supported branches.
1 parent f91c4e3 commit 3d357b4

File tree

1 file changed

+13
-10
lines changed

1 file changed

+13
-10
lines changed

src/bin/psql/command.c

+13-10
Original file line numberDiff line numberDiff line change
@@ -1623,15 +1623,18 @@ do_connect(char *dbname, char *user, char *host, char *port)
16231623

16241624
/*
16251625
* Any change in the parameters read above makes us discard the password.
1626-
* We also discard it if we're to use a conninfo rather than the positional
1627-
* syntax.
1626+
* We also discard it if we're to use a conninfo rather than the
1627+
* positional syntax. Note that currently, PQhost() can return NULL for a
1628+
* default Unix-socket connection, so we have to allow NULL for host.
16281629
*/
1629-
keep_password =
1630-
(o_conn &&
1631-
(strcmp(user, PQuser(o_conn)) == 0) &&
1632-
(!host || strcmp(host, PQhost(o_conn)) == 0) &&
1633-
(strcmp(port, PQport(o_conn)) == 0) &&
1634-
!has_connection_string);
1630+
if (has_connection_string)
1631+
keep_password = false;
1632+
else
1633+
keep_password =
1634+
(user && PQuser(o_conn) && strcmp(user, PQuser(o_conn)) == 0) &&
1635+
((host && PQhost(o_conn) && strcmp(host, PQhost(o_conn)) == 0) ||
1636+
(host == NULL && PQhost(o_conn) == NULL)) &&
1637+
(port && PQport(o_conn) && strcmp(port, PQport(o_conn)) == 0);
16351638

16361639
/*
16371640
* Grab dbname from old connection unless supplied by caller. No password
@@ -1643,8 +1646,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
16431646
/*
16441647
* If the user asked to be prompted for a password, ask for one now. If
16451648
* not, use the password from the old connection, provided the username
1646-
* has not changed. Otherwise, try to connect without a password first,
1647-
* and then ask for a password if needed.
1649+
* etc have not changed. Otherwise, try to connect without a password
1650+
* first, and then ask for a password if needed.
16481651
*
16491652
* XXX: this behavior leads to spurious connection attempts recorded in
16501653
* the postmaster's log. But libpq offers no API that would let us obtain

0 commit comments

Comments
 (0)