From c400717172d77e5b07e51e04c5e5e13da181572e Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 8 Aug 2016 10:07:46 -0400 Subject: [PATCH] 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 --- src/bin/scripts/clusterdb.c | 4 ++-- src/bin/scripts/createlang.c | 4 ++-- src/bin/scripts/droplang.c | 4 ++-- src/bin/scripts/reindexdb.c | 26 ++++++++++----------- src/bin/scripts/vacuumdb.c | 44 +++++++++++++++++------------------- 5 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c index be34ba1122..9b816d9c66 100644 --- 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 f4eb0797f0..b93eada476 100644 --- 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 8b23fe1379..1f1f7ca15a 100644 --- 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 0c8c90c22a..43f61013ef 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -264,7 +264,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); } @@ -281,6 +281,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"); @@ -295,26 +298,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); } @@ -360,9 +360,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); @@ -371,10 +373,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 dbaae1288b..a6bb135e25 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -58,14 +58,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); @@ -356,19 +354,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); /* @@ -474,7 +473,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; @@ -492,7 +491,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; @@ -505,7 +504,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; @@ -673,8 +672,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; @@ -693,10 +691,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) { @@ -722,7 +720,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; @@ -762,7 +760,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); @@ -778,7 +776,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) @@ -797,7 +795,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; @@ -813,7 +811,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) { -- 2.40.0