Skip to content

Commit 55a2cc8

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 b17dbf2 commit 55a2cc8

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

src/bin/psql/command.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,14 +1638,17 @@ do_connect(char *dbname, char *user, char *host, char *port)
16381638
/*
16391639
* Any change in the parameters read above makes us discard the password.
16401640
* We also discard it if we're to use a conninfo rather than the
1641-
* positional syntax.
1641+
* positional syntax. Note that currently, PQhost() can return NULL for a
1642+
* default Unix-socket connection, so we have to allow NULL for host.
16421643
*/
1643-
keep_password =
1644-
(o_conn &&
1645-
(strcmp(user, PQuser(o_conn)) == 0) &&
1646-
(!host || strcmp(host, PQhost(o_conn)) == 0) &&
1647-
(strcmp(port, PQport(o_conn)) == 0) &&
1648-
!has_connection_string);
1644+
if (has_connection_string)
1645+
keep_password = false;
1646+
else
1647+
keep_password =
1648+
(user && PQuser(o_conn) && strcmp(user, PQuser(o_conn)) == 0) &&
1649+
((host && PQhost(o_conn) && strcmp(host, PQhost(o_conn)) == 0) ||
1650+
(host == NULL && PQhost(o_conn) == NULL)) &&
1651+
(port && PQport(o_conn) && strcmp(port, PQport(o_conn)) == 0);
16491652

16501653
/*
16511654
* Grab dbname from old connection unless supplied by caller. No password
@@ -1657,8 +1660,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
16571660
/*
16581661
* If the user asked to be prompted for a password, ask for one now. If
16591662
* not, use the password from the old connection, provided the username
1660-
* has not changed. Otherwise, try to connect without a password first,
1661-
* and then ask for a password if needed.
1663+
* etc have not changed. Otherwise, try to connect without a password
1664+
* first, and then ask for a password if needed.
16621665
*
16631666
* XXX: this behavior leads to spurious connection attempts recorded in
16641667
* the postmaster's log. But libpq offers no API that would let us obtain

0 commit comments

Comments
 (0)