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

diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 0376345d81a86b095c3fc6fd50cd05b32939449a..92891a035570624985d2725833262838005a3dda 100644 (file)
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -8,7 +8,7 @@
  * Darko Prenosil <Darko.Prenosil@finteh.hr>
  * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
  *
- * $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.67 2008/01/01 19:45:45 momjian Exp $
+ * $PostgreSQL: pgsql/contrib/dblink/dblink.c,v 1.68 2008/01/03 21:27:59 tgl Exp $
  * Copyright (c) 2001-2008, PostgreSQL Global Development Group
  * ALL RIGHTS RESERVED;
  *
@@ -91,6 +91,7 @@ static int16 get_attnum_pk_pos(int2vector *pkattnums, int16 pknumatts, int16 key
 static HeapTuple get_tuple_of_interest(Oid relid, int2vector *pkattnums, int16 pknumatts, char **src_pkattvals);
 static Oid get_relid_from_relname(text *relname_text);
 static char *generate_relation_name(Oid relid);
+static void dblink_security_check(PGconn *conn, remoteConn *rconn);
 
 /* Global */
 static remoteConn *pconn = NULL;
@@ -187,10 +188,21 @@ typedef struct remoteConnHashEnt
                             errmsg("could not establish connection"), \
                             errdetail("%s", msg))); \
                } \
+               dblink_security_check(conn, rconn); \
                freeconn = true; \
            } \
    } 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 \
    do { \
            if (!pconn) \
@@ -247,21 +259,8 @@ dblink_connect(PG_FUNCTION_ARGS)
                 errdetail("%s", msg)));
    }
 
-   if (!superuser())
-   {
-       if (!PQconnectionUsedPassword(conn))
-       {
-           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.")));
-       }
-   }
+   /* check password used if not superuser */
+   dblink_security_check(conn, rconn);
 
    if (connname)
    {
@@ -1047,17 +1046,11 @@ PG_FUNCTION_INFO_V1(dblink_is_busy);
 Datum
 dblink_is_busy(PG_FUNCTION_ARGS)
 {
-   char       *msg;
    PGconn     *conn = NULL;
-   char       *conname = NULL;
-   char       *connstr = NULL;
    remoteConn *rconn = NULL;
-   bool        freeconn = false;
 
    DBLINK_INIT;
-   DBLINK_GET_CONN;
-   if (!conn)
-       DBLINK_CONN_NOT_AVAIL;
+   DBLINK_GET_NAMED_CONN;
 
    PQconsumeInput(conn);
    PG_RETURN_INT32(PQisBusy(conn));
@@ -1078,26 +1071,20 @@ PG_FUNCTION_INFO_V1(dblink_cancel_query);
 Datum
 dblink_cancel_query(PG_FUNCTION_ARGS)
 {
-   char       *msg;
    int         res = 0;
    PGconn     *conn = NULL;
-   char       *conname = NULL;
-   char       *connstr = NULL;
    remoteConn *rconn = NULL;
-   bool        freeconn = false;
    PGcancel   *cancel;
    char        errbuf[256];
 
    DBLINK_INIT;
-   DBLINK_GET_CONN;
-   if (!conn)
-       DBLINK_CONN_NOT_AVAIL;
+   DBLINK_GET_NAMED_CONN;
    cancel = PQgetCancel(conn);
 
    res = PQcancel(cancel, errbuf, 256);
    PQfreeCancel(cancel);
 
-   if (res == 0)
+   if (res == 1)
        PG_RETURN_TEXT_P(GET_TEXT("OK"));
    else
        PG_RETURN_TEXT_P(GET_TEXT(errbuf));
@@ -1120,18 +1107,13 @@ dblink_error_message(PG_FUNCTION_ARGS)
 {
    char       *msg;
    PGconn     *conn = NULL;
-   char       *conname = NULL;
-   char       *connstr = NULL;
    remoteConn *rconn = NULL;
-   bool        freeconn = false;
 
    DBLINK_INIT;
-   DBLINK_GET_CONN;
-   if (!conn)
-       DBLINK_CONN_NOT_AVAIL;
+   DBLINK_GET_NAMED_CONN;
 
    msg = PQerrorMessage(conn);
-   if (!msg)
+   if (msg == NULL || msg[0] == '\0')
        PG_RETURN_TEXT_P(GET_TEXT("OK"));
    else
        PG_RETURN_TEXT_P(GET_TEXT(msg));
@@ -2299,3 +2281,23 @@ deleteConnection(const char *name)
                 errmsg("undefined connection name")));
 
 }
+
+static void
+dblink_security_check(PGconn *conn, remoteConn *rconn)
+{
+   if (!superuser())
+   {
+       if (!PQconnectionUsedPassword(conn))
+       {
+           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.")));
+       }
+   }
+}

diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
index bdd2f9926f64448d09c5b7349c92e5bdb9047d8a..fd35d76af9672cf22db450c34e551064d6b8b109 100644 (file)
--- a/contrib/dblink/expected/dblink.out
+++ b/contrib/dblink/expected/dblink.out
@@ -724,6 +724,12 @@ SELECT dblink_get_connections();
  {dtest1,dtest2,dtest3}
 (1 row)
 
+SELECT dblink_is_busy('dtest1');
+ dblink_is_busy 
+----------------
+              0
+(1 row)
+
 SELECT dblink_disconnect('dtest1');
  dblink_disconnect 
 -------------------
@@ -758,3 +764,34 @@ SELECT * from result;
  10 | k  | {a10,b10,c10}
 (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)
+

diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
index 277500e6f164ed75a4f5c3209f55819294cb5a15..48e1daca54d2b800ba97dd8dbc637319b89b9d6d 100644 (file)
--- a/contrib/dblink/sql/dblink.sql
+++ b/contrib/dblink/sql/dblink.sql
@@ -344,9 +344,18 @@ UNION
 ORDER by f1;
 
 SELECT dblink_get_connections();
+SELECT dblink_is_busy('dtest1');
 
 SELECT dblink_disconnect('dtest1');
 SELECT dblink_disconnect('dtest2');
 SELECT dblink_disconnect('dtest3');
+
 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');