From: Tom Lane <tgl@sss.pgh.pa.us>

The attached patches respond to discussion that was on pgsql-hackers
around the beginning of June (see thread "libpgtcl bug (and symptomatic
treatment)").  The changes are:

1. Remove code in connectDB that throws away the password after making
a connection.  This doesn't really add much security IMHO --- a bad guy
with access to your client's address space can likely extract the
password anyway, to say nothing of what he might do directly.  And
there's the serious shortcoming that it prevents PQreset() from working
if the database requires a password.

2. Fix coredump problem: fe_sendauth did not guard against being handed
a NULL password pointer.  (This is the proximate cause of the coredump-
during-PQreset problem that Magosanyi Arpad complained of last month.)

3. Remove highly questionable "error recovery" logic in libpgtcl's
pg_exec statement.

I believe the consensus of the discussion last month was in favor of
#1 and #3, but I'm just now getting around to making the change.
I realized that #2 was a bug in process of looking at the change.

diff --git a/src/interfaces/libpgtcl/pgtclCmds.c b/src/interfaces/libpgtcl/pgtclCmds.c
index c0f698ec0185da4e14c8a499be416ffa20f5f212..ec0844d8aed0cefbc1754230ad1ef58bab3ff949 100644 (file)
--- a/src/interfaces/libpgtcl/pgtclCmds.c
+++ b/src/interfaces/libpgtcl/pgtclCmds.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclCmds.c,v 1.27 1998/06/16 06:53:27 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/interfaces/libpgtcl/Attic/pgtclCmds.c,v 1.28 1998/07/09 03:32:09 scrappy Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -441,24 +441,7 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int argc, char* argv[])
     }
     else {
        /* error occurred during the query */
-       Tcl_SetResult(interp, conn->errorMessage, TCL_STATIC);
-       if (connStatus != CONNECTION_OK) {
-           /* Is this REALLY a good idea?  I don't think so! */
-           PQreset(conn);
-           if (conn->status == CONNECTION_OK) {
-               result = PQexec(conn, argv[2]);
-               PgNotifyTransferEvents(connid);
-               if (result) {
-                   int rId = PgSetResultId(interp, argv[1], result);
-                   if (result->resultStatus == PGRES_COPY_IN ||
-                       result->resultStatus == PGRES_COPY_OUT) {
-                           connid->res_copyStatus = RES_COPY_INPROGRESS;
-                           connid->res_copy = rId;
-                   }
-                   return TCL_OK;
-               }
-           }
-       }
+       Tcl_SetResult(interp, conn->errorMessage, TCL_VOLATILE);
        return TCL_ERROR;
     }
 }

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 022c5cb3860c01229c92eba4a2a0bc787f2a2898..de4cde24e5bfeb775b5aa473f86912a755a287b2 100644 (file)
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v 1.18 1998/07/03 04:24:11 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v 1.19 1998/07/09 03:32:09 scrappy Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -522,6 +522,12 @@ fe_sendauth(AuthRequest areq, PGconn *conn, const char *hostname,
 
                case AUTH_REQ_PASSWORD:
                case AUTH_REQ_CRYPT:
+                       if (password == NULL || *password == '\0')
+                       {
+                               (void) sprintf(PQerrormsg,
+                                "fe_sendauth: no password supplied\n");
+                               return (STATUS_ERROR);
+                       }
                        if (pg_password_sendauth(conn, password, areq) != STATUS_OK)
                        {
                                (void) sprintf(PQerrormsg,

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 47aa2b9bca02f994fbd1e329aba31e6953430911..7d161763064d9d227fdc9535636aeb8783da5026 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.73 1998/07/09 03:29:07 scrappy Exp $
+ *       $Header: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v 1.74 1998/07/09 03:32:10 scrappy Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -768,17 +768,6 @@ connectDB(PGconn *conn)
 
        PQsetenv(conn);
 
-       /* Free the password so it's not hanging out in memory forever */
-       /* XXX Is this *really* a good idea?  The security gain is marginal
-        * if not totally illusory, and it breaks PQreset() for databases
-        * that use passwords.
-        */
-       if (conn->pgpass != NULL)
-       {
-               free(conn->pgpass);
-               conn->pgpass = NULL;
-       }
-
        return CONNECTION_OK;
 
 connect_errReturn: