The original patch to disallow non-passworded connections to non-superusers

failed to cover all the ways in which a connection can be initiated in dblink.
Plug the remaining holes.  Also, disallow transient connections in functions
for which that feature makes no sense (because they are only sensible as
part of a sequence of operations on the same connection).  Joe Conway

Security: CVE-2007-6601
This commit is contained in:
Tom Lane 2008-01-03 21:28:18 +00:00
parent 3af35f8d40
commit 58c7bef913
3 changed files with 91 additions and 44 deletions

View File

@ -8,7 +8,7 @@
* Darko Prenosil <Darko.Prenosil@finteh.hr> * Darko Prenosil <Darko.Prenosil@finteh.hr>
* Shridhar Daithankar <shridhar_daithankar@persistent.co.in> * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
* *
* $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.60.2.1 2007/07/09 01:32:30 joe Exp $ * $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.60.2.2 2008/01/03 21:28:18 tgl Exp $
* Copyright (c) 2001-2006, PostgreSQL Global Development Group * Copyright (c) 2001-2006, PostgreSQL Global Development Group
* ALL RIGHTS RESERVED; * ALL RIGHTS RESERVED;
* *
@ -91,6 +91,7 @@ static HeapTuple get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 p
static Oid get_relid_from_relname(text *relname_text); static Oid get_relid_from_relname(text *relname_text);
static char *generate_relation_name(Oid relid); static char *generate_relation_name(Oid relid);
static char *connstr_strip_password(const char *connstr); static char *connstr_strip_password(const char *connstr);
static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr);
/* Global */ /* Global */
static remoteConn *pconn = NULL; static remoteConn *pconn = NULL;
@ -177,6 +178,7 @@ typedef struct remoteConnHashEnt
else \ else \
{ \ { \
connstr = conname_or_str; \ connstr = conname_or_str; \
dblink_security_check(conn, rconn, connstr); \
conn = PQconnectdb(connstr); \ conn = PQconnectdb(connstr); \
if (PQstatus(conn) == CONNECTION_BAD) \ if (PQstatus(conn) == CONNECTION_BAD) \
{ \ { \
@ -191,6 +193,16 @@ typedef struct remoteConnHashEnt
} \ } \
} while (0) } while (0)
#define DBLINK_GET_NAMED_CONN \
do { \
char *conname = GET_STR(PG_GETARG_TEXT_P(0)); \
rconn = getConnectionByName(conname); \
if(rconn) \
conn = rconn->conn; \
else \
DBLINK_CONN_NOT_AVAIL; \
} while (0)
#define DBLINK_INIT \ #define DBLINK_INIT \
do { \ do { \
if (!pconn) \ if (!pconn) \
@ -231,27 +243,8 @@ dblink_connect(PG_FUNCTION_ARGS)
if (connname) if (connname)
rconn = (remoteConn *) palloc(sizeof(remoteConn)); rconn = (remoteConn *) palloc(sizeof(remoteConn));
/* for non-superusers, check that server requires a password */ /* check password used if not superuser */
if (!superuser()) dblink_security_check(conn, rconn, connstr);
{
/* this attempt must fail */
conn = PQconnectdb(connstr_strip_password(connstr));
if (PQstatus(conn) == CONNECTION_OK)
{
PQfinish(conn);
if (rconn)
pfree(rconn);
ereport(ERROR,
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
errmsg("password is required"),
errdetail("Non-superuser cannot connect if the server does not request a password."),
errhint("Target server's authentication method must be changed.")));
}
else
PQfinish(conn);
}
conn = PQconnectdb(connstr); conn = PQconnectdb(connstr);
MemoryContextSwitchTo(oldcontext); MemoryContextSwitchTo(oldcontext);
@ -1053,17 +1046,11 @@ PG_FUNCTION_INFO_V1(dblink_is_busy);
Datum Datum
dblink_is_busy(PG_FUNCTION_ARGS) dblink_is_busy(PG_FUNCTION_ARGS)
{ {
char *msg;
PGconn *conn = NULL; PGconn *conn = NULL;
char *conname = NULL;
char *connstr = NULL;
remoteConn *rconn = NULL; remoteConn *rconn = NULL;
bool freeconn = false;
DBLINK_INIT; DBLINK_INIT;
DBLINK_GET_CONN; DBLINK_GET_NAMED_CONN;
if (!conn)
DBLINK_CONN_NOT_AVAIL;
PQconsumeInput(conn); PQconsumeInput(conn);
PG_RETURN_INT32(PQisBusy(conn)); PG_RETURN_INT32(PQisBusy(conn));
@ -1084,26 +1071,20 @@ PG_FUNCTION_INFO_V1(dblink_cancel_query);
Datum Datum
dblink_cancel_query(PG_FUNCTION_ARGS) dblink_cancel_query(PG_FUNCTION_ARGS)
{ {
char *msg;
int res = 0; int res = 0;
PGconn *conn = NULL; PGconn *conn = NULL;
char *conname = NULL;
char *connstr = NULL;
remoteConn *rconn = NULL; remoteConn *rconn = NULL;
bool freeconn = false;
PGcancel *cancel; PGcancel *cancel;
char errbuf[256]; char errbuf[256];
DBLINK_INIT; DBLINK_INIT;
DBLINK_GET_CONN; DBLINK_GET_NAMED_CONN;
if (!conn)
DBLINK_CONN_NOT_AVAIL;
cancel = PQgetCancel(conn); cancel = PQgetCancel(conn);
res = PQcancel(cancel, errbuf, 256); res = PQcancel(cancel, errbuf, 256);
PQfreeCancel(cancel); PQfreeCancel(cancel);
if (res == 0) if (res == 1)
PG_RETURN_TEXT_P(GET_TEXT("OK")); PG_RETURN_TEXT_P(GET_TEXT("OK"));
else else
PG_RETURN_TEXT_P(GET_TEXT(errbuf)); PG_RETURN_TEXT_P(GET_TEXT(errbuf));
@ -1126,18 +1107,13 @@ dblink_error_message(PG_FUNCTION_ARGS)
{ {
char *msg; char *msg;
PGconn *conn = NULL; PGconn *conn = NULL;
char *conname = NULL;
char *connstr = NULL;
remoteConn *rconn = NULL; remoteConn *rconn = NULL;
bool freeconn = false;
DBLINK_INIT; DBLINK_INIT;
DBLINK_GET_CONN; DBLINK_GET_NAMED_CONN;
if (!conn)
DBLINK_CONN_NOT_AVAIL;
msg = PQerrorMessage(conn); msg = PQerrorMessage(conn);
if (!msg) if (msg == NULL || msg[0] == '\0')
PG_RETURN_TEXT_P(GET_TEXT("OK")); PG_RETURN_TEXT_P(GET_TEXT("OK"));
else else
PG_RETURN_TEXT_P(GET_TEXT(msg)); PG_RETURN_TEXT_P(GET_TEXT(msg));
@ -2428,3 +2404,28 @@ connstr_strip_password(const char *connstr)
return result.data; return result.data;
} }
static void
dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr)
{
if (!superuser())
{
/* this attempt must fail */
conn = PQconnectdb(connstr_strip_password(connstr));
if (PQstatus(conn) == CONNECTION_OK)
{
PQfinish(conn);
if (rconn)
pfree(rconn);
ereport(ERROR,
(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
errmsg("password is required"),
errdetail("Non-superuser cannot connect if the server does not request a password."),
errhint("Target server's authentication method must be changed.")));
}
else
PQfinish(conn);
}
}

View File

@ -722,6 +722,12 @@ SELECT dblink_get_connections();
{dtest1,dtest2,dtest3} {dtest1,dtest2,dtest3}
(1 row) (1 row)
SELECT dblink_is_busy('dtest1');
dblink_is_busy
----------------
0
(1 row)
SELECT dblink_disconnect('dtest1'); SELECT dblink_disconnect('dtest1');
dblink_disconnect dblink_disconnect
------------------- -------------------
@ -756,3 +762,34 @@ SELECT * from result;
10 | k | {a10,b10,c10} 10 | k | {a10,b10,c10}
(11 rows) (11 rows)
SELECT dblink_connect('dtest1', 'dbname=contrib_regression');
dblink_connect
----------------
OK
(1 row)
SELECT * from
dblink_send_query('dtest1', 'select * from foo where f1 < 3') as t1;
t1
----
1
(1 row)
SELECT dblink_cancel_query('dtest1');
dblink_cancel_query
---------------------
OK
(1 row)
SELECT dblink_error_message('dtest1');
dblink_error_message
----------------------
OK
(1 row)
SELECT dblink_disconnect('dtest1');
dblink_disconnect
-------------------
OK
(1 row)

View File

@ -342,9 +342,18 @@ UNION
ORDER by f1; ORDER by f1;
SELECT dblink_get_connections(); SELECT dblink_get_connections();
SELECT dblink_is_busy('dtest1');
SELECT dblink_disconnect('dtest1'); SELECT dblink_disconnect('dtest1');
SELECT dblink_disconnect('dtest2'); SELECT dblink_disconnect('dtest2');
SELECT dblink_disconnect('dtest3'); SELECT dblink_disconnect('dtest3');
SELECT * from result; SELECT * from result;
SELECT dblink_connect('dtest1', 'dbname=contrib_regression');
SELECT * from
dblink_send_query('dtest1', 'select * from foo where f1 < 3') as t1;
SELECT dblink_cancel_query('dtest1');
SELECT dblink_error_message('dtest1');
SELECT dblink_disconnect('dtest1');