Field conninfo strings throughout src/bin/scripts.

These programs nominally accepted conninfo strings, but they would
proceed to use the original dbname parameter as though it were an
unadorned database name.  This caused "reindexdb dbname=foo" to issue an
SQL command that always failed, and other programs printed a conninfo
string in error messages that purported to print a database name.  Fix
both problems by using PQdb() to retrieve actual database names.
Continue to print the full conninfo string when reporting a connection
failure.  It is informative there, and if the database name is the sole
problem, the server-side error message will include the name.  Beyond
those user-visible fixes, this allows a subsequent commit to synthesize
and use conninfo strings without that implementation detail leaking into
messages.  As a side effect, the "vacuuming database" message now
appears after, not before, the connection attempt.  Back-patch to 9.1
(all supported versions).

Reviewed by Michael Paquier and Peter Eisentraut.

Security: CVE-2016-5424

diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index f87a9cee295e41540ed51e506c5d6805aec6ed85..bf888bd4e9751a72c26f989fbb02f0ddc8d8082b 100644 (file)
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -209,10 +209,10 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
    {
        if (table)
            fprintf(stderr, _("%s: clustering of table \"%s\" in database \"%s\" failed: %s"),
-                   progname, table, dbname, PQerrorMessage(conn));
+                   progname, table, PQdb(conn), PQerrorMessage(conn));
        else
            fprintf(stderr, _("%s: clustering of database \"%s\" failed: %s"),
-                   progname, dbname, PQerrorMessage(conn));
+                   progname, PQdb(conn), PQerrorMessage(conn));
        PQfinish(conn);
        exit(1);
    }

diff --git a/src/bin/scripts/createlang.c b/src/bin/scripts/createlang.c
index f349624092246206fc2afbaee55769abd1c084fe..74402c3b795df0c225a1879f5e6a6004546207a9 100644 (file)
--- a/src/bin/scripts/createlang.c
+++ b/src/bin/scripts/createlang.c
@@ -192,10 +192,10 @@ main(int argc, char *argv[])
    result = executeQuery(conn, sql.data, progname, echo);
    if (PQntuples(result) > 0)
    {
-       PQfinish(conn);
        fprintf(stderr,
          _("%s: language \"%s\" is already installed in database \"%s\"\n"),
-               progname, langname, dbname);
+               progname, langname, PQdb(conn));
+       PQfinish(conn);
        /* separate exit status for "already installed" */
        exit(2);
    }

diff --git a/src/bin/scripts/droplang.c b/src/bin/scripts/droplang.c
index e35fdc0092cdc953863a2aa0ffece3dd3d9ae73d..d281ab691c4dd57c26e10cd4181b77905e9abc46 100644 (file)
--- a/src/bin/scripts/droplang.c
+++ b/src/bin/scripts/droplang.c
@@ -199,10 +199,10 @@ main(int argc, char *argv[])
    result = executeQuery(conn, sql.data, progname, echo);
    if (PQntuples(result) == 0)
    {
-       PQfinish(conn);
        fprintf(stderr, _("%s: language \"%s\" is not installed in "
                          "database \"%s\"\n"),
-               progname, langname, dbname);
+               progname, langname, PQdb(conn));
+       PQfinish(conn);
        exit(1);
    }
    PQclear(result);

diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index b6f3dcbca04bfe47a5e4d8309716d94e39a8d2c3..e579c2a75af7c983a749f31512623116cf77d1bc 100644 (file)
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -263,7 +263,7 @@ main(int argc, char *argv[])
         * specified
         */
        if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
-           reindex_one_database(dbname, dbname, "DATABASE", host, port,
+           reindex_one_database(NULL, dbname, "DATABASE", host, port,
                         username, prompt_password, progname, echo, verbose);
    }
 
@@ -280,6 +280,9 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
 
    PGconn     *conn;
 
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
+
    initPQExpBuffer(&sql);
 
    appendPQExpBufferStr(&sql, "REINDEX");
@@ -294,26 +297,23 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
    else if (strcmp(type, "SCHEMA") == 0)
        appendPQExpBuffer(&sql, " SCHEMA %s", name);
    else if (strcmp(type, "DATABASE") == 0)
-       appendPQExpBuffer(&sql, " DATABASE %s", fmtId(name));
+       appendPQExpBuffer(&sql, " DATABASE %s", fmtId(PQdb(conn)));
    appendPQExpBufferChar(&sql, ';');
 
-   conn = connectDatabase(dbname, host, port, username, prompt_password,
-                          progname, false, false);
-
    if (!executeMaintenanceCommand(conn, sql.data, echo))
    {
        if (strcmp(type, "TABLE") == 0)
            fprintf(stderr, _("%s: reindexing of table \"%s\" in database \"%s\" failed: %s"),
-                   progname, name, dbname, PQerrorMessage(conn));
+                   progname, name, PQdb(conn), PQerrorMessage(conn));
        if (strcmp(type, "INDEX") == 0)
            fprintf(stderr, _("%s: reindexing of index \"%s\" in database \"%s\" failed: %s"),
-                   progname, name, dbname, PQerrorMessage(conn));
+                   progname, name, PQdb(conn), PQerrorMessage(conn));
        if (strcmp(type, "SCHEMA") == 0)
            fprintf(stderr, _("%s: reindexing of schema \"%s\" in database \"%s\" failed: %s"),
-                   progname, name, dbname, PQerrorMessage(conn));
+                   progname, name, PQdb(conn), PQerrorMessage(conn));
        else
            fprintf(stderr, _("%s: reindexing of database \"%s\" failed: %s"),
-                   progname, dbname, PQerrorMessage(conn));
+                   progname, PQdb(conn), PQerrorMessage(conn));
        PQfinish(conn);
        exit(1);
    }
@@ -359,9 +359,11 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port,
                        const char *username, enum trivalue prompt_password,
                        const char *progname, bool echo, bool verbose)
 {
+   PGconn     *conn;
    PQExpBufferData sql;
 
-   PGconn     *conn;
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, false);
 
    initPQExpBuffer(&sql);
 
@@ -370,10 +372,8 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port,
    if (verbose)
        appendPQExpBuffer(&sql, " (VERBOSE)");
 
-   appendPQExpBuffer(&sql, " SYSTEM %s;", dbname);
+   appendPQExpBuffer(&sql, " SYSTEM %s;", PQdb(conn));
 
-   conn = connectDatabase(dbname, host, port, username, prompt_password,
-                          progname, false, false);
    if (!executeMaintenanceCommand(conn, sql.data, echo))
    {
        fprintf(stderr, _("%s: reindexing of system catalogs failed: %s"),

diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 0863c64cbc5d42768d8df3320ffb39ae156a0ba5..056e24ddab86a08faf90d7cb4c650064cce346fc 100644 (file)
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -57,14 +57,12 @@ static void prepare_vacuum_command(PQExpBuffer sql, PGconn *conn,
                       vacuumingOptions *vacopts, const char *table);
 
 static void run_vacuum_command(PGconn *conn, const char *sql, bool echo,
-                  const char *dbname, const char *table,
-                  const char *progname, bool async);
+                  const char *table, const char *progname, bool async);
 
 static ParallelSlot *GetIdleSlot(ParallelSlot slots[], int numslots,
-           const char *dbname, const char *progname);
+           const char *progname);
 
-static bool GetQueryResult(PGconn *conn, const char *dbname,
-              const char *progname);
+static bool GetQueryResult(PGconn *conn, const char *progname);
 
 static void DisconnectDatabase(ParallelSlot *slot);
 
@@ -355,19 +353,20 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
    Assert(stage == ANALYZE_NO_STAGE ||
           (stage >= 0 && stage < ANALYZE_NUM_STAGES));
 
+   conn = connectDatabase(dbname, host, port, username, prompt_password,
+                          progname, false, true);
+
    if (!quiet)
    {
        if (stage != ANALYZE_NO_STAGE)
-           printf(_("%s: processing database \"%s\": %s\n"), progname, dbname,
-                  stage_messages[stage]);
+           printf(_("%s: processing database \"%s\": %s\n"),
+                  progname, PQdb(conn), stage_messages[stage]);
        else
-           printf(_("%s: vacuuming database \"%s\"\n"), progname, dbname);
+           printf(_("%s: vacuuming database \"%s\"\n"),
+                  progname, PQdb(conn));
        fflush(stdout);
    }
 
-   conn = connectDatabase(dbname, host, port, username, prompt_password,
-                          progname, false, true);
-
    initPQExpBuffer(&sql);
 
    /*
@@ -473,7 +472,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
             * Get a free slot, waiting until one becomes free if none
             * currently is.
             */
-           free_slot = GetIdleSlot(slots, concurrentCons, dbname, progname);
+           free_slot = GetIdleSlot(slots, concurrentCons, progname);
            if (!free_slot)
            {
                failed = true;
@@ -491,7 +490,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
         * errors in GetQueryResult through GetIdleSlot.)
         */
        run_vacuum_command(free_slot->connection, sql.data,
-                          echo, dbname, tabname, progname, parallel);
+                          echo, tabname, progname, parallel);
 
        if (cell)
            cell = cell->next;
@@ -504,7 +503,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
        for (j = 0; j < concurrentCons; j++)
        {
            /* wait for all connection to return the results */
-           if (!GetQueryResult((slots + j)->connection, dbname, progname))
+           if (!GetQueryResult((slots + j)->connection, progname))
                goto finish;
 
            (slots + j)->isFree = true;
@@ -672,8 +671,7 @@ prepare_vacuum_command(PQExpBuffer sql, PGconn *conn, vacuumingOptions *vacopts,
  */
 static void
 run_vacuum_command(PGconn *conn, const char *sql, bool echo,
-                  const char *dbname, const char *table,
-                  const char *progname, bool async)
+                  const char *table, const char *progname, bool async)
 {
    bool        status;
 
@@ -692,10 +690,10 @@ run_vacuum_command(PGconn *conn, const char *sql, bool echo,
        if (table)
            fprintf(stderr,
            _("%s: vacuuming of table \"%s\" in database \"%s\" failed: %s"),
-                   progname, table, dbname, PQerrorMessage(conn));
+                   progname, table, PQdb(conn), PQerrorMessage(conn));
        else
            fprintf(stderr, _("%s: vacuuming of database \"%s\" failed: %s"),
-                   progname, dbname, PQerrorMessage(conn));
+                   progname, PQdb(conn), PQerrorMessage(conn));
 
        if (!async)
        {
@@ -721,7 +719,7 @@ run_vacuum_command(PGconn *conn, const char *sql, bool echo,
  * If an error occurs, NULL is returned.
  */
 static ParallelSlot *
-GetIdleSlot(ParallelSlot slots[], int numslots, const char *dbname,
+GetIdleSlot(ParallelSlot slots[], int numslots,
            const char *progname)
 {
    int         i;
@@ -761,7 +759,7 @@ GetIdleSlot(ParallelSlot slots[], int numslots, const char *dbname,
             * We set the cancel-receiving connection to the one in the zeroth
             * slot above, so fetch the error from there.
             */
-           GetQueryResult(slots->connection, dbname, progname);
+           GetQueryResult(slots->connection, progname);
            return NULL;
        }
        Assert(i != 0);
@@ -777,7 +775,7 @@ GetIdleSlot(ParallelSlot slots[], int numslots, const char *dbname,
 
            (slots + i)->isFree = true;
 
-           if (!GetQueryResult((slots + i)->connection, dbname, progname))
+           if (!GetQueryResult((slots + i)->connection, progname))
                return NULL;
 
            if (firstFree < 0)
@@ -796,7 +794,7 @@ GetIdleSlot(ParallelSlot slots[], int numslots, const char *dbname,
  * reported and subsequently ignored.
  */
 static bool
-GetQueryResult(PGconn *conn, const char *dbname, const char *progname)
+GetQueryResult(PGconn *conn, const char *progname)
 {
    PGresult   *result;
 
@@ -812,7 +810,7 @@ GetQueryResult(PGconn *conn, const char *dbname, const char *progname)
            char       *sqlState = PQresultErrorField(result, PG_DIAG_SQLSTATE);
 
            fprintf(stderr, _("%s: vacuuming of database \"%s\" failed: %s"),
-                   progname, dbname, PQerrorMessage(conn));
+                   progname, PQdb(conn), PQerrorMessage(conn));
 
            if (sqlState && strcmp(sqlState, ERRCODE_UNDEFINED_TABLE) != 0)
            {