8000 Empty search_path in Autovacuum and non-psql/pgbench clients. · fantomancer/postgres@582edc3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 582edc3

Browse files
nmischnmisch
committed
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
1 parent 3d2aed6 commit 582edc3

File tree

24 files changed

+338
-66
lines changed

24 files changed

+338
-66
lines changed

‎contrib/oid2name/oid2name.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "catalog/pg_class.h"
1313

14+
#include "fe_utils/connect.h"
1415
#include "libpq-fe.h"
1516
#include "pg_getopt.h"
1617

@@ -266,6 +267,7 @@ sql_conn(struct options *my_opts)
266267
bool have_password = false;
267268
char password[100];
268269
bool new_pass;
270+
PGresult *res;
269271

270272
/*
271273
* Start the connection. Loop until we have a password if requested by
@@ -323,6 +325,17 @@ sql_conn(struct options *my_opts)
323325
exit(1);
324326
}
325327

328+
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
329+
if (PQresultStatus(res) != PGRES_TUPLES_OK)
330+
{
331+
fprintf(stderr, "oid2name: could not clear search_path: %s\n",
332+
PQerrorMessage(conn));
333+
PQclear(res);
334+
PQfinish(conn);
335+
exit(-1);
336+
}
337+
PQclear(res);
338+
326339
/* return the conn if good */
327340
return conn;
328341
}

‎contrib/vacuumlo/vacuumlo.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
< A3DB /code>
2323

2424
#include "catalog/pg_class.h"
2525

26+
#include "fe_utils/connect.h"
2627
#include "libpq-fe.h"
2728
#include "pg_getopt.h"
2829

@@ -140,11 +141,8 @@ vacuumlo(const char *database, const struct _param *param)
140141
fprintf(stdout, "Test run: no large objects will be removed!\n");
141142
}
142143

143-
/*
144-
* Don't get fooled by any non-system catalogs
145-
*/
146-
res = PQexec(conn, "SET search_path = pg_catalog");
147-
if (PQresultStatus(res) != PGRES_COMMAND_OK)
144+
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
145+
if (PQresultStatus(res) != PGRES_TUPLES_OK)
148146
{
149147
fprintf(stderr, "Failed to set search_path:\n");
150148
fprintf(stderr, "%s", PQerrorMessage(conn));

‎src/backend/postmaster/autovacuum.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,12 @@ AutoVacLauncherMain(int argc, char *argv[])
574574
/* must unblock signals before calling rebuild_database_list */
575575
PG_SETMASK(&UnBlockSig);
576576

577+
/*
578+
* Set always-secure search path. Launcher doesn't connect to a database,
579+
* so this has no effect.
580+
*/
581+
SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
582+
577583
/*
578584
* Force zero_damaged_pages OFF in the autovac process, even if it is set
579585
* in postgresql.conf. We don't really want such a dangerous option being
@@ -1584,6 +1590,14 @@ AutoVacWorkerMain(int argc, char *argv[])
15841590

15851591
PG_SETMASK(&UnBlockSig);
15861592

1593+
/*
1594+
* Set always-secure search path, so malicious users can't redirect user
1595+
* code (e.g. pg_index.indexprs). (That code runs in a
1596+
* SECURITY_RESTRICTED_OPERATION sandbox, so malicious users could not
1597+
* take control of the entire autovacuum worker in any case.)
1598+
*/
1599+
SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
1600+
15871601
/*
15881602
* Force zero_damaged_pages OFF in the autovac process, even if it is set
15891603
* in postgresql.conf. We don't really want such a dangerous option being

‎src/bin/pg_basebackup/streamutil.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "access/xlog_internal.h"
2525
#include "common/fe_memutils.h"
2626
#include "datatype/timestamp.h"
27+
#include "fe_utils/connect.h"
2728
#include "port/pg_bswap.h"
2829
#include "pqexpbuffer.h"
2930

@@ -208,6 +209,23 @@ GetConnection(void)
208209
if (conn_opts)
209210
PQconninfoFree(conn_opts);
210211

212+
/* Set always-secure search path, so malicious users can't get control. */
213+
if (dbname != NULL)
214+
{
215+
PGresult *res;
216+
217+
res = PQexec(tmpconn, ALWAYS_SECURE_SEARCH_PATH_SQL);
218+
if (PQresultStatus(res) != PGRES_TUPLES_OK)
219+
{
220+
fprintf(stderr, _("%s: could not clear search_path: %s\n"),
221+
progname, PQerrorMessage(tmpconn));
222+
PQclear(res);
223+
PQfinish(tmpconn);
224+
exit(1);
225+
}
226+
PQclear(res);
227+
}
228+
211229
/*
212230
* Ensure we have the same value of integer_datetimes (now always "on") as
213231
* the server we are connecting to.

‎src/bin/pg_dump/pg_backup_db.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "postgres_fe.h"
1313

1414
#include "dumputils.h"
15+
#include "fe_utils/connect.h"
1516
#include "fe_utils/string_utils.h"
1617
#include "parallel.h"
1718
#include "pg_backup_archiver.h"
@@ -102,6 +103,10 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
102103

103104
PQfinish(AH->connection);
104105
AH->connection = newConn;
106+
107+
/* Start strict; later phases may override this. */
108+
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
109+
ALWAYS_SECURE_SEARCH_PATH_SQL));
105110
}
106111

107112
/*
@@ -304,6 +309,10 @@ ConnectDatabase(Archive *AHX,
304309
PQdb(AH->connection) ? PQdb(AH->connection) : "",
305310
PQerrorMessage(AH->connection));
306311

312+
/* Start strict; later phases may override this. */
313+
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
314+
ALWAYS_SECURE_SEARCH_PATH_SQL));
315+
307316
/*
308317
* We want to remember connection's actual password, whether or not we got
309318
* it by prompting. So we don't just store the password variable.

‎src/bin/pg_dump/pg_dump.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
#include "pg_backup_db.h"
6161
#include "pg_backup_utils.h"
6262
#include "pg_dump.h"
63+
#include "fe_utils/connect.h"
6364
#include "fe_utils/string_utils.h"
6465

6566

@@ -1021,6 +1022,8 @@ setup_connection(Archive *AH, const char *dumpencoding,
10211022
PGconn *conn = GetConnection(AH);
10221023
const char *std_strings;
10231024

1025+
PQclear(ExecuteSqlQueryForSingleRow(AH, ALWAYS_SECURE_SEARCH_PATH_SQL));
1026+
10241027
/*
10251028
* Set the client encoding if requested.
10261029
*/
@@ -1311,19 +1314,29 @@ expand_table_name_patterns(Archive *fout,
13111314

13121315
for (cell = patterns->head; cell; cell = cell->next)
13131316
{
1317+
/*
1318+
* Query must remain ABSOLUTELY devoid of unqualified names. This
1319+
* would be unnecessary given a pg_table_is_visible() variant taking a
1320+
* search_path argument.
1321+
*/
13141322
appendPQExpBuffer(query,
13151323
"SELECT c.oid"
13161324
"\nFROM pg_catalog.pg_class c"
1317-
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace"
1318-
"\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c', '%c')\n",
1325+
"\n LEFT JOIN pg_catalog.pg_namespace n"
1326+
"\n ON n.oid OPERATOR(pg_catalog.=) c.relnamespace"
1327+
"\nWHERE c.relkind OPERATOR(pg_catalog.=) ANY"
1328+
"\n (array['%c', '%c', '%c', '%c', '%c', '%c'])\n",
13191329
RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW,
13201330
RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE,
13211331
RELKIND_PARTITIONED_TABLE);
13221332
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
13231333
false, "n.nspname", "c.relname", NULL,
13241334
"pg_catalog.pg_table_is_visible(c.oid)");
13251335

1336+
ExecuteSqlStatement(fout, "RESET search_path");
13261337
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
1338+
PQclear(ExecuteSqlQueryForSingleRow(fout,
1339+
ALWAYS_SECURE_SEARCH_PATH_SQL));
13271340
if (strict_names && PQntuples(res) == 0)
13281341
exit_horribly(NULL, "no matching tables were found for pattern \"%s\"\n", cell->val);
13291342

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "dumputils.h"
2424
#include "pg_backup.h"
2525
#include "common/file_utils.h"
26+
#include "fe_utils/connect.h"
2627
#include "fe_utils/string_utils.h"
2728

2829
/* version string we expect back from pg_dump */
@@ -1717,10 +1718,7 @@ connectDatabase(const char *dbname, const char *connection_string,
17171718
exit_nicely(1);
17181719
}
17191720

1720-
/*
1721-
* Make sure we are not fooled by non-system schemas in the search path.
1722-
*/
1723-
executeCommand(conn, "SET search_path = pg_catalog");
1721+
PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL));
17241722

17251723
return conn;
17261724
}

‎src/bin/pg_rewind/libpq_fetch.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "libpq-fe.h"
2525
#include "catalog/catalog.h"
2626
#include "catalog/pg_type.h"
27+
#include "fe_utils/connect.h"
2728
#include "port/pg_bswap.h"
2829

2930
static PGconn *conn = NULL;
@@ -54,6 +55,12 @@ libpqConnect(const char *connstr)
5455

5556
pg_log(PG_PROGRESS, "connected to server\n");
5657

58+
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
59+
if (PQresultStatus(res) != PGRES_TUPLES_OK)
60+
pg_fatal("could not clear search_path: %s",
61+
PQresultErrorMessage(res));
62+
PQclear(res);
63+
5764
/*
5865
* Check that the server is not in hot standby mode. There is no
5966
* fundamental reason that couldn't be made to work, but it doesn't

‎src/bin/pg_upgrade/server.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "postgres_fe.h"
1111

12+
#include "fe_utils/connect.h"
1213
#include "fe_utils/string_utils.h"
1314
#include "pg_upgrade.h"
1415

@@ -40,6 +41,8 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
4041
exit(1);
4142
}
4243

44+
PQclear(executeQueryOrDie(conn, ALWAYS_SECURE_SEARCH_PATH_SQL));
45+
4346
return conn;
4447
}
4548

‎src/bin/scripts/clusterdb.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,17 +195,21 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
195195

196196
PGconn *conn;
197197

198+
conn = connectDatabase(dbname, host, port, username, prompt_password,
199+
progname, echo, false, false);
200+
198201
initPQExpBuffer(&sql);
199202

200203
appendPQExpBufferStr(&sql, "CLUSTER");
201204
if (verbose)
202205
appendPQExpBufferStr(&sql, " VERBOSE");
203206
if (table)
204-
appendPQExpBuffer(&sql, " %s", table);
207+
{
208+
appendPQExpBufferChar(&sql, ' ');
209+
appendQualifiedRelation(&sql, table, conn, progname, echo);
210+
}
205211
appendPQExpBufferChar(&sql, ';');
206212

207-
conn = connectDatabase(dbname, host, port, username, prompt_password,
208-
progname, false, false);
209213
if (!executeMaintenanceCommand(conn, sql.data, echo))
210214
{
211215
if (table)
@@ -234,7 +238,7 @@ cluster_all_databases(bool verbose, const char *maintenance_db,
234238
int i;
235239

236240
conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
237-
prompt_password, progname);
241+
prompt_password, progname, echo);
238242
result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo);
239243
PQfinish(conn);
240244

0 commit comments

Comments
 (0)
0