From bd029bcb4a115d2dd409319cfc8db4f77d2dd048 Mon Sep 17 00:00:00 2001 From: "Marc G. Fournier" Date: Thu, 9 Jul 1998 03:32:10 +0000 Subject: [PATCH] From: Tom Lane 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. --- src/interfaces/libpgtcl/pgtclCmds.c | 21 ++------------------- src/interfaces/libpq/fe-auth.c | 8 +++++++- src/interfaces/libpq/fe-connect.c | 13 +------------ 3 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/interfaces/libpgtcl/pgtclCmds.c b/src/interfaces/libpgtcl/pgtclCmds.c index c0f698ec01..ec0844d8ae 100644 --- 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 022c5cb386..de4cde24e5 100644 --- 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 47aa2b9bca..7d16176306 100644 --- 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: