8000 Be more paranoid about null return values from libpq status functions. · home201448/postgres@64b7079 · GitHub
[go: up one dir, main page]

Skip to content

Commit 64b7079

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 6638c9a commit 64b7079

File tree

1 file changed

+13
-10
lines changed

1 file changed

+13
-10
lines changed

src/bin/psql/command.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,15 +1587,18 @@ do_connect(char *dbname, char *user, char *host, char *port)
15871587

15881588
/*
15891589
* Any change in the parameters read above makes us discard the password.
1590-
* We also discard it if we're to use a conninfo rather than the positional
1591-
* syntax.
1590+
* We also discard it if we're to use a conninfo rather than the
1591+
* positional syntax. Note that currently, PQhost() can return NULL for a
1592+
* default Unix-socket connection, so we have to allow NULL for host.
15921593
*/
1593-
keep_password =
1594-
(o_conn &&
1595-
(strcmp(user, PQuser(o_conn)) == 0) &&
1596-
(!host || strcmp(host, PQhost(o_conn)) == 0) &&
1597-
(strcmp(port, PQport(o_conn)) == 0) &&
1598-
!has_connection_string);
1594+
if (has_connection_string)
1595+
keep_password = false;
1596+
else
1597+
keep_password =
1598+
(user && PQuser(o_conn) && strcmp(user, PQuser(o_conn)) == 0) &&
1599+
((host && PQhost(o_conn) && strcmp(host, PQhost(o_conn)) == 0) ||
1600+
(host == NULL && PQhost(o_conn) == NULL)) &&
1601+
(port && PQport(o_conn) && strcmp(port, PQport(o_conn)) == 0);
15991602

16001603
/*
16011604
* Grab dbname from old connection unless supplied by caller. No password
@@ -1607,8 +1610,8 @@ do_connect(char *dbname, char *user, char *host, char *port)
16071610
/*
16081611
* If the user asked to be prompted for a password, ask for one now. If
16091612
* not, use the password from the old connection, provided the username
1610-
* has not changed. Otherwise, try to connect without a password first,
1611-
* and then ask for a password if needed.
1613+
* etc have not changed. Otherwise, try to connect without a password
1614+
* first, and then ask for a password if needed.
16121615
*
16131616
* XXX: this behavior leads to spurious connection attempts recorded in
16141617
* the postmaster's log. But libpq offers no API that would let us obtain

0 commit comments

Comments
 (0)
0