Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e170b8c

Browse filesBrowse 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 815172b commit e170b8c
Copy full SHA for e170b8c

File tree

Expand file treeCollapse file tree

26 files changed

+347
-77
lines changed
Filter options
Expand file treeCollapse file tree

26 files changed

+347
-77
lines changed

‎contrib/oid2name/oid2name.c

Copy file name to clipboardExpand all lines: contrib/oid2name/oid2name.c
+13Lines changed: 13 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 "libpq-fe.h"
1314
#include "pg_getopt.h"
1415

@@ -263,6 +264,7 @@ sql_conn(struct options * my_opts)
263264
PGconn *conn;
264265
char *password = NULL;
265266
bool new_pass;
267+
PGresult *res;
266268

267269
/*
268270
* Start the connection. Loop until we have a password if requested by
@@ -322,6 +324,17 @@ sql_conn(struct options * my_opts)
322324
exit(1);
323325
}
324326

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

‎contrib/vacuumlo/vacuumlo.c

Copy file name to clipboardExpand all lines: contrib/vacuumlo/vacuumlo.c
+3-5Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <termios.h>
2222
#endif
2323

24+
#include "fe_utils/connect.h"
2425
#include "libpq-fe.h"
2526
#include "pg_getopt.h"
2627

@@ -135,11 +136,8 @@ vacuumlo(const char *database, const struct _param * param)
135136
fprintf(stdout, "Test run: no large objects will be removed!\n");
136137
}
137138

138-
/*
139-
* Don't get fooled by any non-system catalogs
140-
*/
141-
res = PQexec(conn, "SET search_path = pg_catalog");
142-
if (PQresultStatus(res) != PGRES_COMMAND_OK)
139+
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
140+
if (PQresultStatus(res) != PGRES_TUPLES_OK)
143141
{
144142
fprintf(stderr, "Failed to set search_path:\n");
145143
fprintf(stderr, "%s", PQerrorMessage(conn));

‎src/backend/postmaster/autovacuum.c

Copy file name to clipboardExpand all lines: src/backend/postmaster/autovacuum.c
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,12 @@ AutoVacLauncherMain(int argc, char *argv[])
528528
/* must unblock signals before calling rebuild_database_list */
529529
PG_SETMASK(&UnBlockSig);
530530

531+
/*
532+
* Set always-secure search path. Launcher doesn't connect to a database,
533+
* so this has no effect.
534+
*/
535+
SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
536+
531537
/*
532538
* Force zero_damaged_pages OFF in the autovac process, even if it is set
533539
* in postgresql.conf. We don't really want such a dangerous option being
@@ -1537,6 +1543,14 @@ AutoVacWorkerMain(int argc, char *argv[])
15371543

15381544
PG_SETMASK(&UnBlockSig);
15391545

1546+
/*
1547+
* Set always-secure search path, so malicious users can't redirect user
1548+
* code (e.g. pg_index.indexprs). (That code runs in a
1549+
* SECURITY_RESTRICTED_OPERATION sandbox, so malicious users could not
1550+
* take control of the entire autovacuum worker in any case.)
1551+
*/
1552+
SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
1553+
15401554
/*
15411555
* Force zero_damaged_pages OFF in the autovac process, even if it is set
15421556
* in postgresql.conf. We don't really want such a dangerous option being

‎src/bin/pg_basebackup/streamutil.c

Copy file name to clipboardExpand all lines: src/bin/pg_basebackup/streamutil.c
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "pqexpbuffer.h"
3131
#include "common/fe_memutils.h"
3232
#include "datatype/timestamp.h"
33+
#include "fe_utils/connect.h"
3334

3435
#define ERRCODE_DUPLICATE_OBJECT "42710"
3536

@@ -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 timestamps as the server we
213231
* are connecting to.

‎src/bin/pg_dump/pg_backup_db.c

Copy file name to clipboardExpand all lines: src/bin/pg_dump/pg_backup_db.c
+11Lines changed: 11 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"
@@ -114,6 +115,11 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
114115
PQfinish(AH->connection);
115116
AH->connection = newConn;
116117

118+
/* Start strict; later phases may override this. */
119+
if (PQserverVersion(AH->connection) >= 70300)
120+
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
121+
ALWAYS_SECURE_SEARCH_PATH_SQL));
122+
117123
return 1;
118124
}
119125

@@ -322,6 +328,11 @@ ConnectDatabase(Archive *AHX,
322328
PQdb(AH->connection) ? PQdb(AH->connection) : "",
323329
PQerrorMessage(AH->connection));
324330

331+
/* Start strict; later phases may override this. */
332+
if (PQserverVersion(AH->connection) >= 70300)
333+
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
334+
ALWAYS_SECURE_SEARCH_PATH_SQL));
335+
325336
/*
326337
* We want to remember connection's actual password, whether or not we got
327338
* it by prompting. So we don't just store the password variable.

‎src/bin/pg_dump/pg_dump.c

Copy file name to clipboardExpand all lines: src/bin/pg_dump/pg_dump.c
+16-2Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "pg_backup_db.h"
6262
#include "pg_backup_utils.h"
6363
#include "pg_dump.h"
64+
#include "fe_utils/connect.h"
6465
#include "fe_utils/string_utils.h"
6566

6667

@@ -982,6 +983,9 @@ setup_connection(Archive *AH, const char *dumpencoding,
982983
PGconn *conn = GetConnection(AH);
983984
const char *std_strings;
984985

986+
if (AH->remoteVersion >= 70300)
987+
PQclear(ExecuteSqlQueryForSingleRow(AH, ALWAYS_SECURE_SEARCH_PATH_SQL));
988+
985989
/*
986990
* Set the client encoding if requested.
987991
*/
@@ -1280,18 +1284,28 @@ expand_table_name_patterns(Archive *fout,
12801284

12811285
for (cell = patterns->head; cell; cell = cell->next)
12821286
{
1287+
/*
1288+
* Query must remain ABSOLUTELY devoid of unqualified names. This
1289+
* would be unnecessary given a pg_table_is_visible() variant taking a
1290+
* search_path argument.
1291+
*/
12831292
appendPQExpBuffer(query,
12841293
"SELECT c.oid"
12851294
"\nFROM pg_catalog.pg_class c"
1286-
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace"
1287-
"\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c')\n",
1295+
"\n LEFT JOIN pg_catalog.pg_namespace n"
1296+
"\n ON n.oid OPERATOR(pg_catalog.=) c.relnamespace"
1297+
"\nWHERE c.relkind OPERATOR(pg_catalog.=) ANY"
1298+
"\n (array['%c', '%c', '%c', '%c', '%c'])\n",
12881299
RELKIND_RELATION, RELKIND_SEQUENCE, RELKIND_VIEW,
12891300
RELKIND_MATVIEW, RELKIND_FOREIGN_TABLE);
12901301
processSQLNamePattern(GetConnection(fout), query, cell->val, true,
12911302
false, "n.nspname", "c.relname", NULL,
12921303
"pg_catalog.pg_table_is_visible(c.oid)");
12931304

1305+
ExecuteSqlStatement(fout, "RESET search_path");
12941306
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
1307+
PQclear(ExecuteSqlQueryForSingleRow(fout,
1308+
ALWAYS_SECURE_SEARCH_PATH_SQL));
12951309
if (strict_names && PQntuples(res) == 0)
12961310
exit_horribly(NULL, "no matching tables were found for pattern \"%s\"\n", cell->val);
12971311

‎src/bin/pg_dump/pg_dumpall.c

Copy file name to clipboardExpand all lines: src/bin/pg_dump/pg_dumpall.c
+2-5Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include "dumputils.h"
2828
#include "pg_backup.h"
29+
#include "fe_utils/connect.h"
2930
#include "fe_utils/string_utils.h"
3031

3132
/* version string we expect back from pg_dump */
@@ -2079,12 +2080,8 @@ connectDatabase(const char *dbname, const char *connection_string,
20792080
exit_nicely(1);
20802081
}
20812082

2082-
/*
2083-
* On 7.3 and later, make sure we are not fooled by non-system schemas in
2084-
* the search path.
2085-
*/
20862083
if (server_version >= 70300)
2087-
executeCommand(conn, "SET search_path = pg_catalog");
2084+
PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL));
20882085

20892086
return conn;
20902087
}

‎src/bin/pg_rewind/libpq_fetch.c

Copy file name to clipboardExpand all lines: src/bin/pg_rewind/libpq_fetch.c
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "libpq-fe.h"
3030
#include "catalog/catalog.h"
3131
#include "catalog/pg_type.h"
32+
#include "fe_utils/connect.h"
3233

3334
static PGconn *conn = NULL;
3435

@@ -58,6 +59,12 @@ libpqConnect(const char *connstr)
5859

5960
pg_log(PG_PROGRESS, "connected to server\n");
6061

62+
res = PQexec(conn, ALWAYS_SECURE_SEARCH_PATH_SQL);
63+
if (PQresultStatus(res) != PGRES_TUPLES_OK)
64+
pg_fatal("could not clear search_path: %s",
65+
PQresultErrorMessage(res));
66+
PQclear(res);
67+
6168
/*
6269
* Check that the server is not in hot standby mode. There is no
6370
* fundamental reason that couldn't be made to work, but it doesn't

‎src/bin/pg_upgrade/server.c

Copy file name to clipboardExpand all lines: src/bin/pg_upgrade/server.c
+3Lines 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

Copy file name to clipboardExpand all lines: src/bin/scripts/clusterdb.c
+8-4Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.