From 91f3ffc5249eff99c311fb27e7b29a44d9c62be1 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Mon, 26 Feb 2018 07:39:44 -0800
Subject: [PATCH] Empty search_path in Autovacuum and non-psql/pgbench clients.
This makes the client programs behave as documented regardless of the
connect-time search_path and regardless of user-created objects. Today,
a malicious user with CREATE permission on a search_path schema can take
control of certain of these clients' queries and invoke arbitrary SQL
functions under the client identity, often a superuser. This is
exploitable in the default configuration, where all users have CREATE
privilege on schema "public".
This changes behavior of user-defined code stored in the database, like
pg_index.indexprs and pg_extension_config_dump(). If they reach code
bearing unqualified names, "does not exist" or "no schema has been
selected to create in" errors might appear. Users may fix such errors
by schema-qualifying affected names. After upgrading, consider watching
server logs for these errors.
The --table arguments of src/bin/scripts clients have been lax; for
example, "vacuumdb -Zt pg_am\;CHECKPOINT" performed a checkpoint. That
now fails, but for now, "vacuumdb -Zt 'pg_am(amname);CHECKPOINT'" still
performs a checkpoint.
Back-patch to 9.3 (all supported versions).
Reviewed by Tom Lane, though this fix strategy was not his first choice.
Reported by Arseniy Sharoglazov.
Security: CVE-2018-1058
---
contrib/oid2name/oid2name.c | 13 +++
contrib/vacuumlo/vacuumlo.c | 8 +-
src/backend/postmaster/autovacuum.c | 14 +++
src/bin/pg_basebackup/streamutil.c | 18 ++++
src/bin/pg_dump/dumputils.c | 16 +--
src/bin/pg_dump/pg_backup_db.c | 11 +++
src/bin/pg_dump/pg_dump.c | 17 +++-
src/bin/pg_dump/pg_dumpall.c | 7 +-
src/bin/pg_rewind/libpq_fetch.c | 7 ++
src/bin/pg_upgrade/server.c | 3 +
src/bin/scripts/Makefile | 23 ++---
src/bin/scripts/clusterdb.c | 12 ++-
src/bin/scripts/common.c | 137 ++++++++++++++++++++++++--
src/bin/scripts/common.h | 10 +-
src/bin/scripts/createdb.c | 2 +-
src/bin/scripts/createlang.c | 4 +-
src/bin/scripts/createuser.c | 2 +-
src/bin/scripts/dropdb.c | 3 +-
src/bin/scripts/droplang.c | 10 +-
src/bin/scripts/dropuser.c | 2 +-
src/bin/scripts/reindexdb.c | 23 ++---
src/bin/scripts/t/010_clusterdb.pl | 2 +-
src/bin/scripts/t/090_reindexdb.pl | 6 +-
src/bin/scripts/t/100_vacuumdb.pl | 30 +++++-
src/bin/scripts/vacuumdb.c | 29 ++++--
src/include/Makefile | 3 +-
src/include/fe_utils/connect.h | 28 ++++++
src/tools/findoidjoins/findoidjoins.c | 9 ++
28 files changed, 360 insertions(+), 89 deletions(-)
create mode 100644 src/include/fe_utils/connect.h
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index e5eeec21c15..91da40352b4 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -9,6 +9,7 @@
*/
#include "postgres_fe.h"
+#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
@@ -263,6 +264,7 @@ sql_conn(struct options * my_opts)
PGconn *conn;
char *password = NULL;
bool new_pass;
+ PGresult *res;
/*
* Start the connection. Loop until we have a password if requested by
@@ -322,6 +324,17 @@ sql_conn(struct options * my_opts)
exit(1);
}
+ res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ fprintf(stderr, "oid2name: could not clear search_path: %s\n",
+ PQerrorMessage(conn));
+ PQclear(res);
+ PQfinish(conn);
+ exit(-1);
+ }
+ PQclear(res);
+
/* return the conn if good */
return conn;
}
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index ca0d3048b82..fa51e33f5d4 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -21,6 +21,7 @@
#include <termios.h>
#endif
+#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pg_getopt.h"
@@ -135,11 +136,8 @@ vacuumlo(const char *database, const struct _param * param)
fprintf(stdout, "Test run: no large objects will be removed!\n");
}
- /*
- * Don't get fooled by any non-system catalogs
- */
- res = PQexec(conn, "SET search_path = pg_catalog");
- if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
fprintf(stderr, "Failed to set search_path:\n");
fprintf(stderr, "%s", PQerrorMessage(conn));
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 9963c4c9281..fe886de6ffe 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -530,6 +530,12 @@ AutoVacLauncherMain(int argc, char *argv[])
/* must unblock signals before calling rebuild_database_list */
PG_SETMASK(&UnBlockSig);
+ /*
+ * Set always-secure search path. Launcher doesn't connect to a database,
+ * so this has no effect.
+ */
+ SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
+
/*
* Force zero_damaged_pages OFF in the autovac process, even if it is set
* in postgresql.conf. We don't really want such a dangerous option being
@@ -1543,6 +1549,14 @@ AutoVacWorkerMain(int argc, char *argv[])
PG_SETMASK(&UnBlockSig);
+ /*
+ * Set always-secure search path, so malicious users can't redirect user
+ * code (e.g. pg_index.indexprs). (That code runs in a
+ * SECURITY_RESTRICTED_OPERATION sandbox, so malicious users could not
+ * take control of the entire autovacuum worker in any case.)
+ */
+ SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
+
/*
* Force zero_damaged_pages OFF in the autovac process, even if it is set
* in postgresql.conf. We don't really want such a dangerous option being
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index ac3eb48cd6e..8e87099b4ba 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -30,6 +30,7 @@
#include "pqexpbuffer.h"
#include "common/fe_memutils.h"
#include "datatype/timestamp.h"
+#include "fe_utils/connect.h"
#define ERRCODE_DUPLICATE_OBJECT "42710"
@@ -208,6 +209,23 @@ GetConnection(void)
if (conn_opts)
PQconninfoFree(conn_opts);
+ /* Set always-secure search path, so malicious users can't get control. */
+ if (dbname != NULL)
+ {
+ PGresult *res;
+
+ res = PQexec(tmpconn, ALWAYS_SECURE_SEARCH_PATH_SQL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ fprintf(stderr, _("%s: could not clear search_path: %s\n"),
+ progname, PQerrorMessage(tmpconn));
+ PQclear(res);
+ PQfinish(tmpconn);
+ exit(1);
+ }
+ PQclear(res);
+ }
+
/*
* Ensure we have the same value of integer timestamps as the server we
* are connecting to.
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 2d9c0de853d..a4c91e63a0b 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -1376,8 +1376,9 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
}
/*
- * Now decide what we need to emit. Note there will be a leading "^(" in
- * the patterns in any case.
+ * Now decide what we need to emit. We may run under a hostile
+ * search_path, so qualify EVERY name. Note there will be a leading "^("
+ * in the patterns in any case.
*/
if (namebuf.len > 2)
{
@@ -1390,15 +1391,18 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
WHEREAND();
if (altnamevar)
{
- appendPQExpBuffer(buf, "(%s ~ ", namevar);
+ appendPQExpBuffer(buf,
+ "(%s OPERATOR(pg_catalog.~) ", namevar);
appendStringLiteralConn(buf, namebuf.data, conn);
- appendPQExpBuffer(buf, "\n OR %s ~ ", altnamevar);
+ appendPQExpBuffer(buf,
+ "\n OR %s OPERATOR(pg_catalog.~) ",
+ altnamevar);
appendStringLiteralConn(buf, namebuf.data, conn);
appendPQExpBufferStr(buf, ")\n");
}
else
{
- appendPQExpBuffer(buf, "%s ~ ", namevar);
+ appendPQExpBuffer(buf, "%s OPERATOR(pg_catalog.~) ", namevar);
appendStringLiteralConn(buf, namebuf.data, conn);
appendPQExpBufferChar(buf, '\n');
}
@@ -1414,7 +1418,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
if (strcmp(schemabuf.data, "^(.*)$") != 0 && schemavar)
{
WHEREAND();
- appendPQExpBuffer(buf, "%s ~ ", schemavar);
+ appendPQExpBuffer(buf, "%s OPERATOR(pg_catalog.~) ", schemavar);
appendStringLiteralConn(buf, schemabuf.data, conn);
appendPQExpBufferChar(buf, '\n');
}
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index ce60b4db8aa..c4c059cfb1b 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -12,6 +12,7 @@
#include "postgres_fe.h"
#include "dumputils.h"
+#include "fe_utils/connect.h"
#include "parallel.h"
#include "pg_backup_archiver.h"
#include "pg_backup_db.h"
@@ -113,6 +114,11 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
PQfinish(AH->connection);
AH->connection = newConn;
+ /* Start strict; later phases may override this. */
+ if (PQserverVersion(AH->connection) >= 70300)
+ PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
+ ALWAYS_SECURE_SEARCH_PATH_SQL));
+
return 1;
}
@@ -321,6 +327,11 @@ ConnectDatabase(Archive *AHX,
PQdb(AH->connection) ? PQdb(AH->connection) : "",
PQerrorMessage(AH->connection));
+ /* Start strict; later phases may override this. */
+ if (PQserverVersion(AH->connection) >= 70300)
+ PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
+ ALWAYS_SECURE_SEARCH_PATH_SQL));
+
/*
* We want to remember connection's actual password, whether or not we got
* it by prompting. So we don't just store the password variable.
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 12ef354b408..dbe2b6560a4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -60,6 +60,7 @@
#include "pg_backup_db.h"
#include "pg_backup_utils.h"
#include "pg_dump.h"
+#include "fe_utils/connect.h"
typedef struct
@@ -965,6 +966,9 @@ setup_connection(Archive *AH, const char *dumpencoding,
PGconn *conn = GetConnection(AH);
const char *std_strings;
+ if (AH->remoteVersion >= 70300)
+ PQclear(ExecuteSqlQueryForSingleRow(AH, ALWAYS_SECURE_SEARCH_PATH_SQL));
+
/*
* Set the client encoding if requested.
*/
@@ -1257,13 +1261,20 @@ expand_table_name_patterns(Archive *fout,
for (cell = patterns->head; cell; cell = cell->next)
{
+ /*
+ * Query must remain ABSOLUTELY devoid of unqualified names. This
+ * would be unnecessary given a pg_table_is_visible() variant taking a
+ * search_path argument.
+ */
if (cell != patterns->head)
appendPQExpBufferStr(query, "UNION ALL\n");
appendPQExpBuffer(query,
"SELECT c.oid"
"\nFROM pg_catalog.pg_class c"
- "\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace"
- "\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c')\n",
+ "\n LEFT JOIN pg_catalog.pg_namespace n"
+ "\n ON n.oid OPERATOR(pg_catalog.=) c.relnamespace"
+ "\nWHERE c.relkind OPERATOR(pg_catalog.=) ANY"
+ "\n (array['%c', '%c', '%c', '%c', '%c'])\n",
RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW,
RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE);
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
@@ -1271,7 +1282,9 @@ expand_table_name_patterns(Archive *fout,
"pg_catalog.pg_table_is_visible(c.oid)");
}
+ ExecuteSqlStatement(fout, "RESET search_path");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ PQclear(ExecuteSqlQueryForSingleRow(fout, ALWAYS_SECURE_SEARCH_PATH_SQL));
for (i = 0; i < PQntuples(res); i++)
{
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index fb6da30b175..bf2937e82db 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -26,6 +26,7 @@
#include "dumputils.h"
#include "pg_backup.h"
+#include "fe_utils/connect.h"
/* version string we expect back from pg_dump */
#define PGDUMP_VERSIONSTR "pg_dump (PostgreSQL) " PG_VERSION "\n"
@@ -1983,12 +1984,8 @@ connectDatabase(const char *dbname, const char *connection_string,
exit_nicely(1);
}
- /*
- * On 7.3 and later, make sure we are not fooled by non-system schemas in
- * the search path.
- */
if (server_version >= 70300)
- executeCommand(conn, "SET search_path = pg_catalog");
+ PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL));
return conn;
}
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 6a12957a216..22b99642794 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -29,6 +29,7 @@
#include "libpq-fe.h"
#include "catalog/catalog.h"
#include "catalog/pg_type.h"
+#include "fe_utils/connect.h"
static PGconn *conn = NULL;
@@ -58,6 +59,12 @@ libpqConnect(const char *connstr)
pg_log(PG_PROGRESS, "connected to server\n");
+ res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pg_fatal("could not clear search_path: %s",
+ PQresultErrorMessage(res));
+ PQclear(res);
+
/*
* Check that the server is not in hot standby mode. There is no
* fundamental reason that couldn't be made to work, but it doesn't
diff --git a/src/bin/pg_upgrade/server.c b/src/bin/pg_upgrade/server.c
index 5e7cc6171cc..da3aefca829 100644
--- a/src/bin/pg_upgrade/server.c
+++ b/src/bin/pg_upgrade/server.c
@@ -9,6 +9,7 @@
#include "postgres_fe.h"
+#include "fe_utils/connect.h"
#include "pg_upgrade.h"
@@ -39,6 +40,8 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
exit(1);
}
+ PQclear(executeQueryOrDie(conn, ALWAYS_SECURE_SEARCH_PATH_SQL));
+
return conn;
}
diff --git a/src/bin/scripts/Makefile b/src/bin/scripts/Makefile
index c8317164f8a..2961c182eb6 100644
--- a/src/bin/scripts/Makefile
+++ b/src/bin/scripts/Makefile
@@ -25,16 +25,17 @@ all: $(PROGRAMS)
%: %.o $(WIN32RES)
$(CC) $(CFLAGS) $^ $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
-createdb: createdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-createlang: createlang.o common.o print.o mbprint.o | submake-libpq submake-libpgport
-createuser: createuser.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-dropdb: dropdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-droplang: droplang.o common.o print.o mbprint.o | submake-libpq submake-libpgport
-dropuser: dropuser.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-clusterdb: clusterdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-vacuumdb: vacuumdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-reindexdb: reindexdb.o common.o dumputils.o kwlookup.o keywords.o | submake-libpq submake-libpgport
-pg_isready: pg_isready.o common.o | submake-libpq submake-libpgport
+SCRIPTS_COMMON = common.o dumputils.o kwlookup.o keywords.o
+createdb: createdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+createlang: createlang.o $(SCRIPTS_COMMON) print.o mbprint.o | submake-libpq submake-libpgport
+createuser: createuser.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+dropdb: dropdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+droplang: droplang.o $(SCRIPTS_COMMON) print.o mbprint.o | submake-libpq submake-libpgport
+dropuser: dropuser.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+clusterdb: clusterdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+vacuumdb: vacuumdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+reindexdb: reindexdb.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
+pg_isready: pg_isready.o $(SCRIPTS_COMMON) | submake-libpq submake-libpgport
dumputils.c keywords.c: % : $(top_srcdir)/src/bin/pg_dump/%
rm -f $@ && $(LN_S) $< .
@@ -65,7 +66,7 @@ uninstall:
clean distclean maintainer-clean:
rm -f $(addsuffix $(X), $(PROGRAMS)) $(addsuffix .o, $(PROGRAMS))
- rm -f common.o dumputils.o kwlookup.o keywords.o print.o mbprint.o $(WIN32RES)
+ rm -f $(SCRIPTS_COMMON) print.o mbprint.o $(WIN32RES)
rm -f dumputils.c print.c mbprint.c kwlookup.c keywords.c
rm -rf tmp_check
diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 75dc051ef13..a6fc0449978 100644
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -194,17 +194,21 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
PGconn *conn;
+ conn = connectDatabase(dbname, host, port, username, prompt_password,
+ progname, echo, false, false);
+
initPQExpBuffer(&sql);
appendPQExpBufferStr(&sql, "CLUSTER");
if (verbose)
appendPQExpBufferStr(&sql, " VERBOSE");
if (table)
- appendPQExpBuffer(&sql, " %s", table);
+ {
+ appendPQExpBufferChar(&sql, ' ');
+ appendQualifiedRelation(&sql, table, conn, progname, echo);
+ }
appendPQExpBufferChar(&sql, ';');
- conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, false);
if (!executeMaintenanceCommand(conn, sql.data, echo))
{
if (table)
@@ -233,7 +237,7 @@ cluster_all_databases(bool verbose, const char *maintenance_db,
int i;
conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname);
+ prompt_password, progname, echo);
result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo);
PQfinish(conn);
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index 362e748b9a7..0f76d171f89 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -18,6 +18,8 @@
#include <unistd.h>
#include "common.h"
+#include "dumputils.h"
+#include "fe_utils/connect.h"
static PGcancel *volatile cancelConn = NULL;
@@ -63,9 +65,10 @@ handle_help_version_opts(int argc, char *argv[],
* as before, else we might create password exposure hazards.)
*/
PGconn *
-connectDatabase(const char *dbname, const char *pghost, const char *pgport,
- const char *pguser, enum trivalue prompt_password,
- const char *progname, bool fail_ok, bool allow_password_reuse)
+connectDatabase(const char *dbname, const char *pghost,
+ const char *pgport, const char *pguser,
+ enum trivalue prompt_password, const char *progname,
+ bool echo, bool fail_ok, bool allow_password_reuse)
{
PGconn *conn;
static char *password = NULL;
@@ -143,6 +146,10 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
exit(1);
}
+ if (PQserverVersion(conn) >= 70300)
+ PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL,
+ progname, echo));
+
return conn;
}
@@ -150,24 +157,24 @@ connectDatabase(const char *dbname, const char *pghost, const char *pgport,
* Try to connect to the appropriate maintenance database.
*/
PGconn *
-connectMaintenanceDatabase(const char *maintenance_db, const char *pghost,
- const char *pgport, const char *pguser,
- enum trivalue prompt_password,
- const char *progname)
+connectMaintenanceDatabase(const char *maintenance_db,
+ const char *pghost, const char *pgport,
+ const char *pguser, enum trivalue prompt_password,
+ const char *progname, bool echo)
{
PGconn *conn;
/* If a maintenance database name was specified, just connect to it. */
if (maintenance_db)
return connectDatabase(maintenance_db, pghost, pgport, pguser,
- prompt_password, progname, false, false);
+ prompt_password, progname, echo, false, false);
/* Otherwise, try postgres first and then template1. */
conn = connectDatabase("postgres", pghost, pgport, pguser, prompt_password,
- progname, true, false);
+ progname, echo, true, false);
if (!conn)
conn = connectDatabase("template1", pghost, pgport, pguser,
- prompt_password, progname, false, false);
+ prompt_password, progname, echo, false, false);
return conn;
}
@@ -253,6 +260,116 @@ executeMaintenanceCommand(PGconn *conn, const char *query, bool echo)
return r;
}
+
+/*
+ * Split TABLE[(COLUMNS)] into TABLE and [(COLUMNS)] portions. When you
+ * finish using them, pg_free(*table). *columns is a pointer into "spec",
+ * possibly to its NUL terminator.
+ */
+static void
+split_table_columns_spec(const char *spec, int encoding,
+ char **table, const char **columns)
+{
+ bool inquotes = false;
+ const char *cp = spec;
+
+ /*
+ * Find the first '(' not identifier-quoted. Based on
+ * dequote_downcase_identifier().
+ */
+ while (*cp && (*cp != '(' || inquotes))
+ {
+ if (*cp == '"')
+ {
+ if (inquotes && cp[1] == '"')
+ cp++; /* pair does not affect quoting */
+ else
+ inquotes = !inquotes;
+ cp++;
+ }
+ else
+ cp += PQmblen(cp, encoding);
+ }
+ *table = pg_strdup(spec);
+ (*table)[cp - spec] = '\0'; /* no strndup */
+ *columns = cp;
+}
+
+/*
+ * Break apart TABLE[(COLUMNS)] of "spec". With the reset_val of search_path
+ * in effect, have regclassin() interpret the TABLE portion. Append to "buf"
+ * the qualified name of TABLE, followed by any (COLUMNS). Exit on failure.
+ * We use this to interpret --table=foo under the search path psql would get,
+ * in advance of "ANALYZE public.foo" under the always-secure search path.
+ */
+void
+appendQualifiedRelation(PQExpBuffer buf, const char *spec,
+ PGconn *conn, const char *progname, bool echo)
+{
+ char *table;
+ const char *columns;
+ PQExpBufferData sql;
+ PGresult *res;
+ int ntups;
+
+ /* Before 7.3, the concept of qualifying a name did not exist. */
+ if (PQserverVersion(conn) < 70300)
+ {
+ appendPQExpBufferStr(&sql, spec);
+ return;
+ }
+
+ split_table_columns_spec(spec, PQclientEncoding(conn), &table, &columns);
+
+ /*
+ * Query must remain ABSOLUTELY devoid of unqualified names. This would
+ * be unnecessary given a regclassin() variant taking a search_path
+ * argument.
+ */
+ initPQExpBuffer(&sql);
+ appendPQExpBufferStr(&sql,
+ "SELECT c.relname, ns.nspname\n"
+ " FROM pg_catalog.pg_class c,"
+ " pg_catalog.pg_namespace ns\n"
+ " WHERE c.relnamespace OPERATOR(pg_catalog.=) ns.oid\n"
+ " AND c.oid OPERATOR(pg_catalog.=) ");
+ appendStringLiteralConn(&sql, table, conn);
+ appendPQExpBufferStr(&sql, "::pg_catalog.regclass;");
+
+ executeCommand(conn, "RESET search_path", progname, echo);
+
+ /*
+ * One row is a typical result, as is a nonexistent relation ERROR.
+ * regclassin() unconditionally accepts all-digits input as an OID; if no
+ * relation has that OID; this query returns no rows. Catalog corruption
+ * might elicit other row counts.
+ */
+ res = executeQuery(conn, sql.data, progname, echo);
+ ntups = PQntuples(res);
+ if (ntups != 1)
+ {
+ fprintf(stderr,
+ ngettext("%s: query returned %d row instead of one: %s\n",
+ "%s: query returned %d rows instead of one: %s\n",
+ ntups),
+ progname, ntups, sql.data);
+ PQfinish(conn);
+ exit(1);
+ }
+ appendPQExpBufferStr(buf,
+ fmtQualifiedId(PQserverVersion(conn),
+ PQgetvalue(res, 0, 1),
+ PQgetvalue(res, 0, 0)));
+ appendPQExpBufferStr(buf, columns);
+ PQclear(res);
+ termPQExpBuffer(&sql);
+ pg_free(table);
+
+ PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL,
+ progname, echo));
+}
+
+
/*
* Check yes/no answer in a localized way. 1=yes, 0=no, -1=neither.
*/
diff --git a/src/bin/scripts/common.h b/src/bin/scripts/common.h
index 90382959184..c74ca4e2b5a 100644
--- a/src/bin/scripts/common.h
+++ b/src/bin/scripts/common.h
@@ -32,11 +32,12 @@ extern void handle_help_version_opts(int argc, char *argv[],
extern PGconn *connectDatabase(const char *dbname, const char *pghost,
const char *pgport, const char *pguser,
enum trivalue prompt_password, const char *progname,
- bool fail_ok, bool allow_password_reuse);
+ bool echo, bool fail_ok, bool allow_password_reuse);
extern PGconn *connectMaintenanceDatabase(const char *maintenance_db,
- const char *pghost, const char *pgport, const char *pguser,
- enum trivalue prompt_password, const char *progname);
+ const char *pghost, const char *pgport,
+ const char *pguser, enum trivalue prompt_password,
+ const char *progname, bool echo);
extern PGresult *executeQuery(PGconn *conn, const char *query,
const char *progname, bool echo);
@@ -47,6 +48,9 @@ extern void executeCommand(PGconn *conn, const char *query,
extern bool executeMaintenanceCommand(PGconn *conn, const char *query,
bool echo);
+extern void appendQualifiedRelation(PQExpBuffer buf, const char *name,
+ PGconn *conn, const char *progname, bool echo);
+
extern bool yesno_prompt(const char *question);
extern void setup_cancel_handler(void);
diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index 4d3fb22622a..9049ccc7cce 100644
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -202,7 +202,7 @@ main(int argc, char *argv[])
maintenance_db = "template1";
conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname);
+ prompt_password, progname, echo);
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/createlang.c b/src/bin/scripts/createlang.c
index 74402c3b795..34bb9e1f355 100644
--- a/src/bin/scripts/createlang.c
+++ b/src/bin/scripts/createlang.c
@@ -141,7 +141,7 @@ main(int argc, char *argv[])
static const bool translate_columns[] = {false, true};
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, false);
+ progname, echo, false, false);
printfPQExpBuffer(&sql, "SELECT lanname as \"%s\", "
"(CASE WHEN lanpltrusted THEN '%s' ELSE '%s' END) as \"%s\" "
@@ -181,7 +181,7 @@ main(int argc, char *argv[])
*p += ('a' - 'A');
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, false);
+ progname, echo, false, false);
/*
* Make sure the language isn't already installed
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index dc358e40ce3..971bf0b1955 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -251,7 +251,7 @@ main(int argc, char *argv[])
login = TRI_YES;
conn = connectDatabase("postgres", host, port, username, prompt_password,
- progname, false, false);
+ progname, echo, false, false);
initPQExpBuffer(&sql);
diff --git a/src/bin/scripts/dropdb.c b/src/bin/scripts/dropdb.c
index c6dc72ef6f6..8809aacf998 100644
--- a/src/bin/scripts/dropdb.c
+++ b/src/bin/scripts/dropdb.c
@@ -129,7 +129,8 @@ main(int argc, char *argv[])
maintenance_db = "template1";
conn = connectMaintenanceDatabase(maintenance_db,
- host, port, username, prompt_password, progname);
+ host, port, username, prompt_password,
+ progname, echo);
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/droplang.c b/src/bin/scripts/droplang.c
index d281ab691c4..a029813e53b 100644
--- a/src/bin/scripts/droplang.c
+++ b/src/bin/scripts/droplang.c
@@ -140,7 +140,7 @@ main(int argc, char *argv[])
static const bool translate_columns[] = {false, true};
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, false);
+ progname, echo, false, false);
printfPQExpBuffer(&sql, "SELECT lanname as \"%s\", "
"(CASE WHEN lanpltrusted THEN '%s' ELSE '%s' END) as \"%s\" "
@@ -182,13 +182,7 @@ main(int argc, char *argv[])
*p += ('a' - 'A');
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, false);
-
- /*
- * Force schema search path to be just pg_catalog, so that we don't have
- * to be paranoid about search paths below.
- */
- executeCommand(conn, "SET search_path = pg_catalog;", progname, echo);
+ progname, echo, false, false);
/*
* Make sure the language is installed
diff --git a/src/bin/scripts/dropuser.c b/src/bin/scripts/dropuser.c
index 1b0cf78f2a4..a90fcb2aabb 100644
--- a/src/bin/scripts/dropuser.c
+++ b/src/bin/scripts/dropuser.c
@@ -129,7 +129,7 @@ main(int argc, char *argv[])
(if_exists ? "IF EXISTS " : ""), fmtId(dropuser));
conn = connectDatabase("postgres", host, port, username, prompt_password,
- progname, false, false);
+ progname, echo, false, false);
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index a2aaed1faf8..4151f1f4402 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -281,23 +281,24 @@ 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);
+ progname, echo, false, false);
initPQExpBuffer(&sql);
- appendPQExpBufferStr(&sql, "REINDEX");
+ appendPQExpBufferStr(&sql, "REINDEX ");
if (verbose)
- appendPQExpBufferStr(&sql, " (VERBOSE)");
+ appendPQExpBufferStr(&sql, "(VERBOSE) ");
- if (strcmp(type, "TABLE") == 0)
- appendPQExpBuffer(&sql, " TABLE %s", name);
- else if (strcmp(type, "INDEX") == 0)
- appendPQExpBuffer(&sql, " INDEX %s", name);
+ appendPQExpBufferStr(&sql, type);
+ appendPQExpBufferChar(&sql, ' ');
+ if (strcmp(type, "TABLE") == 0 ||
+ strcmp(type, "INDEX") == 0)
+ appendQualifiedRelation(&sql, name, conn, progname, echo);
else if (strcmp(type, "SCHEMA") == 0)
- appendPQExpBuffer(&sql, " SCHEMA %s", name);
+ appendPQExpBufferStr(&sql, name);
else if (strcmp(type, "DATABASE") == 0)
- appendPQExpBuffer(&sql, " DATABASE %s", fmtId(PQdb(conn)));
+ appendPQExpBufferStr(&sql, fmtId(PQdb(conn)));
appendPQExpBufferChar(&sql, ';');
if (!executeMaintenanceCommand(conn, sql.data, echo))
@@ -334,7 +335,7 @@ reindex_all_databases(const char *maintenance_db,
int i;
conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname);
+ prompt_password, progname, echo);
result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo);
PQfinish(conn);
@@ -371,7 +372,7 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port,
PQExpBufferData sql;
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, false);
+ progname, echo, false, false);
initPQExpBuffer(&sql);
diff --git a/src/bin/scripts/t/010_clusterdb.pl b/src/bin/scripts/t/010_clusterdb.pl
index dc0d78a27d3..bdb32734510 100644
--- a/src/bin/scripts/t/010_clusterdb.pl
+++ b/src/bin/scripts/t/010_clusterdb.pl
@@ -22,5 +22,5 @@ psql 'postgres',
'CREATE TABLE test1 (a int); CREATE INDEX test1x ON test1 (a); CLUSTER test1 USING test1x';
issues_sql_like(
[ 'clusterdb', '-t', 'test1', 'postgres' ],
- qr/statement: CLUSTER test1;/,
+ qr/statement: CLUSTER public\.test1;/,
'cluster specific table');
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 42628c25e2b..18f40516791 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -21,11 +21,11 @@ psql 'postgres',
'CREATE TABLE test1 (a int); CREATE INDEX test1x ON test1 (a);';
issues_sql_like(
[ 'reindexdb', '-t', 'test1', 'postgres' ],
- qr/statement: REINDEX TABLE test1;/,
+ qr/statement: REINDEX TABLE public\.test1;/,
'reindex specific table');
issues_sql_like(
[ 'reindexdb', '-i', 'test1x', 'postgres' ],
- qr/statement: REINDEX INDEX test1x;/,
+ qr/statement: REINDEX INDEX public\.test1x;/,
'reindex specific index');
issues_sql_like(
[ 'reindexdb', '-S', 'pg_catalog', 'postgres' ],
@@ -37,5 +37,5 @@ issues_sql_like(
'reindex system tables');
issues_sql_like(
[ 'reindexdb', '-v', '-t', 'test1', 'postgres' ],
- qr/statement: REINDEX \(VERBOSE\) TABLE test1;/,
+ qr/statement: REINDEX \(VERBOSE\) TABLE public\.test1;/,
'reindex with verbose output');
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index ac160ba8374..47bbd7ea36f 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 18;
+use Test::More tests => 23;
program_help_ok('vacuumdb');
program_version_ok('vacuumdb');
@@ -23,10 +23,32 @@ issues_sql_like(
qr/statement: VACUUM \(FREEZE\);/,
'vacuumdb -F');
issues_sql_like(
- [ 'vacuumdb', '-z', 'postgres' ],
- qr/statement: VACUUM \(ANALYZE\);/,
- 'vacuumdb -z');
+ [ 'vacuumdb', '-zj2', 'postgres' ],
+ qr/statement: VACUUM \(ANALYZE\) pg_catalog\./,
+ 'vacuumdb -zj2');
issues_sql_like(
[ 'vacuumdb', '-Z', 'postgres' ],
qr/statement: ANALYZE;/,
'vacuumdb -Z');
+command_ok([qw(vacuumdb -Z --table=pg_am dbname=template1)],
+ 'vacuumdb with connection string');
+
+command_fails([qw(vacuumdb -Zt pg_am;ABORT postgres)],
+ 'trailing command in "-t", without COLUMNS');
+# Unwanted; better if it failed.
+command_ok([qw(vacuumdb -Zt pg_am(amname);ABORT postgres)],
+ 'trailing command in "-t", with COLUMNS');
+
+psql('postgres', q|
+ CREATE TABLE "need""q(uot" (")x" text);
+
+ CREATE FUNCTION f0(int) RETURNS int LANGUAGE SQL AS 'SELECT $1 * $1';
+ CREATE FUNCTION f1(int) RETURNS int LANGUAGE SQL AS 'SELECT f0($1)';
+ CREATE TABLE funcidx (x int);
+ INSERT INTO funcidx VALUES (0),(1),(2),(3);
+ CREATE INDEX i0 ON funcidx ((f1(x)));
+|);
+command_ok([qw|vacuumdb -Z --table="need""q(uot"(")x") postgres|],
+ 'column list');
+command_fails([qw|vacuumdb -Zt funcidx postgres|],
+ 'unqualifed name via functional index');
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 3bb3f1bf1c1..437f610cfa6 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -58,7 +58,9 @@ static void vacuum_all_databases(vacuumingOptions *vacopts,
const char *progname, bool echo, bool quiet);
static void prepare_vacuum_command(PQExpBuffer sql, PGconn *conn,
- vacuumingOptions *vacopts, const char *table);
+ vacuumingOptions *vacopts, const char *table,
+ bool table_pre_qualified,
+ const char *progname, bool echo);
static void run_vacuum_command(PGconn *conn, const char *sql, bool echo,
const char *table, const char *progname, bool async);
@@ -358,7 +360,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
(stage >= 0 && stage < ANALYZE_NUM_STAGES));
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, true);
+ progname, echo, false, true);
if (!quiet)
{
@@ -430,7 +432,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
for (i = 1; i < concurrentCons; i++)
{
conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, false, true);
+ progname, echo, false, true);
init_slot(slots + i, conn);
}
}
@@ -456,7 +458,8 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
ParallelSlot *free_slot;
const char *tabname = cell ? cell->val : NULL;
- prepare_vacuum_command(&sql, conn, vacopts, tabname);
+ prepare_vacuum_command(&sql, conn, vacopts, tabname,
+ tables == &dbtables, progname, echo);
if (CancelRequested)
{
@@ -547,8 +550,8 @@ vacuum_all_databases(vacuumingOptions *vacopts,
int stage;
int i;
- conn = connectMaintenanceDatabase(maintenance_db, host, port,
- username, prompt_password, progname);
+ conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
+ prompt_password, progname, echo);
result = executeQuery(conn,
"SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;",
progname, echo);
@@ -611,8 +614,10 @@ vacuum_all_databases(vacuumingOptions *vacopts,
* quoted. The command is semicolon-terminated.
*/
static void
-prepare_vacuum_command(PQExpBuffer sql, PGconn *conn, vacuumingOptions *vacopts,
- const char *table)
+prepare_vacuum_command(PQExpBuffer sql, PGconn *conn,
+ vacuumingOptions *vacopts, const char *table,
+ bool table_pre_qualified,
+ const char *progname, bool echo)
{
resetPQExpBuffer(sql);
@@ -668,7 +673,13 @@ prepare_vacuum_command(PQExpBuffer sql, PGconn *conn, vacuumingOptions *vacopts,
}
if (table)
- appendPQExpBuffer(sql, " %s", table);
+ {
+ appendPQExpBufferChar(sql, ' ');
+ if (table_pre_qualified)
+ appendPQExpBufferStr(sql, table);
+ else
+ appendQualifiedRelation(sql, table, conn, progname, echo);
+ }
appendPQExpBufferChar(sql, ';');
}
diff --git a/src/include/Makefile b/src/include/Makefile
index e486fd917d5..d338645d005 100644
--- a/src/include/Makefile
+++ b/src/include/Makefile
@@ -17,7 +17,8 @@ all: pg_config.h pg_config_ext.h pg_config_os.h
# Subdirectories containing headers for server-side dev
-SUBDIRS = access bootstrap catalog commands common datatype executor foreign \
+SUBDIRS = access bootstrap catalog commands common datatype \
+ executor fe_utils foreign \
lib libpq mb nodes optimizer parser postmaster regex replication \
rewrite storage tcop snowball snowball/libstemmer tsearch \
tsearch/dicts utils port port/atomics port/win32 port/win32_msvc \
diff --git a/src/include/fe_utils/connect.h b/src/include/fe_utils/connect.h
new file mode 100644
index 00000000000..fa293d2458d
--- /dev/null
+++ b/src/include/fe_utils/connect.h
@@ -0,0 +1,28 @@
+/*-------------------------------------------------------------------------
+ *
+ * Interfaces in support of FE/BE connections.
+ *
+ *
+ * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/fe_utils/connect.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef CONNECT_H
+#define CONNECT_H
+
+/*
+ * This SQL statement installs an always-secure search path, so malicious
+ * users can't take control. CREATE of an unqualified name will fail, because
+ * this selects no creation schema. This does not demote pg_temp, so it is
+ * suitable where we control the entire FE/BE connection but not suitable in
+ * SECURITY DEFINER functions. This is portable to PostgreSQL 7.3, which
+ * introduced schemas. When connected to an older version from code that
+ * might work with the old server, skip this.
+ */
+#define ALWAYS_SECURE_SEARCH_PATH_SQL \
+ "SELECT pg_catalog.set_config('search_path', '', false)"
+
+#endif /* CONNECT_H */
diff --git a/src/tools/findoidjoins/findoidjoins.c b/src/tools/findoidjoins/findoidjoins.c
index 236122a1e4c..b644b65f880 100644
--- a/src/tools/findoidjoins/findoidjoins.c
+++ b/src/tools/findoidjoins/findoidjoins.c
@@ -7,6 +7,7 @@
*/
#include "postgres_fe.h"
+#include "fe_utils/connect.h"
#include "libpq-fe.h"
#include "pqexpbuffer.h"
@@ -44,6 +45,14 @@ main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
+ if (!res || PQresultStatus(res) != PGRES_TUPLES_OK)
+ {
+ fprintf(stderr, "sql error: %s\n", PQerrorMessage(conn));
+ exit(EXIT_FAILURE);
+ }
+ PQclear(res);
+
/* Get a list of relations that have OIDs */
printfPQExpBuffer(&sql, "%s",
--
GitLab