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.
This commit is contained in:
Marc G. Fournier 1998-07-09 03:32:10 +00:00
parent ce812671b1
commit bd029bcb4a
3 changed files with 10 additions and 32 deletions

View File

@ -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;
}
}

View File

@ -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,

View File

@ -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: