Proposed patch to disallow password=foo in database name parameter - Mailing list pgsql-patches

From Tom Lane
Subject Proposed patch to disallow password=foo in database name parameter
Date
Msg-id 26840.1197341802@sss.pgh.pa.us
Whole thread Raw
Responses Re: Proposed patch to disallow password=foo in database name parameter
Re: Proposed patch to disallow password=foo in database name parameter
List pgsql-patches
As of PG 8.3, libpq allows a conninfo string to be passed in via the
dbName parameter of PQsetdbLogin.  This is to allow access to conninfo
facilities in old programs that are still using PQsetdbLogin (including
most of our own standard clients ... ahem).  For instance

    psql "service = foo"

Andrew Dunstan pointed out a possible security hole in this: it will
allow people to do

    psql "dbname = mydb password = mypassword"

which would leave their password exposed on the program's command line.

While we cannot absolutely prevent client apps from doing stupid things,
it seems like it might be a good idea to prevent passwords from being
passed in through dbName.  The attached patch (which depends on some
pretty-recent changes in CVS HEAD) accomplishes this.

Anybody think this is good, bad, or silly?  Does the issue need
explicit documentation, and if so where and how?

            regards, tom lane

Index: fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.354
diff -c -r1.354 fe-connect.c
*** fe-connect.c    9 Dec 2007 19:01:40 -0000    1.354
--- fe-connect.c    11 Dec 2007 02:46:22 -0000
***************
*** 599,604 ****
--- 599,618 ----
      {
          if (!connectOptions1(conn, dbName))
              return conn;
+
+         /*
+          * We disallow supplying a password through dbName, because a large
+          * number of applications allow dbName to be set from a command-line
+          * parameter, and putting a password on your command line is a horrid
+          * idea from a security point of view.
+          */
+         if (conn->pgpass_from_client)
+         {
+             conn->status = CONNECTION_BAD;
+             printfPQExpBuffer(&conn->errorMessage,
+                               libpq_gettext("password must not be set within database name parameter\n"));
+             return conn;
+         }
      }
      else
      {

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgbench - startup delay
Next
From: "Joshua D. Drake"
Date:
Subject: Re: Proposed patch to disallow password=foo in database name parameter