8000 Avoid using unsafe search_path settings during dump and restore. · postgres/postgres@815172b · GitHub
[go: up one dir, main page]
Skip to content

Commit 815172b

Browse files
tglsfdctglsfdc
committed
Avoid using unsafe search_path settings during dump and restore.
Historically, pg_dump has "set search_path = foo, pg_catalog" when dumping an object in schema "foo", and has also caused that setting to be used while restoring the object. This is problematic because functions and operators in schema "foo" could capture references meant to refer to pg_catalog entries, both in the queries issued by pg_dump and those issued during the subsequent restore run. That could result in dump/restore misbehavior, or in privilege escalation if a nefarious user installs trojan-horse functions or operators. This patch changes pg_dump so that it does not change the search_path dynamically. The emitted restore script sets the search_path to what was used at dump time, and then leaves it alone thereafter. Created objects are placed in the correct schema, regardless of the active search_path, by dint of schema-qualifying their names in the CREATE commands, as well as in subsequent ALTER and ALTER-like commands. Since this change requires a change in the behavior of pg_restore when processing an archive file made according to this new convention, bump the archive file version number; old versions of pg_restore will therefore refuse to process files made with new versions of pg_dump. Security: CVE-2018-1058
1 parent bcce4c3 commit 815172b

File tree

14 files changed

+1002
-1261
lines changed

14 files changed

+1002
-1261
lines changed

‎src/backend/utils/adt/ruleutils.c

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,17 @@
8282
#define PRETTYINDENT_LIMIT 40 /* wrap limit */
8383

8484
/* Pretty flags */
85-
#define PRETTYFLAG_PAREN 1
86-
#define PRETTYFLAG_INDENT 2
85+
#define PRETTYFLAG_PAREN 0x0001
86+
#define PRETTYFLAG_INDENT 0x0002
87+
#define PRETTYFLAG_SCHEMA 0x0004
8788

8889
/* Default line length for pretty-print wrapping: 0 means wrap always */
8990
#define WRAP_COLUMN_DEFAULT 0
9091

91-
/* macro to test if pretty action needed */
92+
/* macros to test if pretty action needed */
9293
#define PRETTY_PAREN(context) ((context)->prettyFlags & PRETTYFLAG_PAREN)
9394
#define PRETTY_INDENT(context) ((context)->prettyFlags & PRETTYFLAG_INDENT)
95+
#define PRETTY_SCHEMA(context) ((context)->prettyFlags & PRETTYFLAG_SCHEMA)
9496

9597

9698
/* ----------
@@ -489,7 +491,7 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
489491
int prettyFlags;
490492
char *res;
491493

492-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
494+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
493495

494496
res = pg_get_ruledef_worker(ruleoid, prettyFlags);
495497

@@ -610,7 +612,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
610612
int prettyFlags;
611613
char *res;
612614

613-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
615+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
614616

615617
res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
616618

@@ -630,7 +632,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
630632
char *res;
631633

632634
/* calling this implies we want pretty printing */
633-
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT;
635+
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
634636

635637
res = pg_get_viewdef_worker(viewoid, prettyFlags, wrap);
636638

@@ -676,7 +678,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
676678
Oid viewoid;
677679
char *res;
678680

679-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
681+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
680682

681683
/* Look up view name. Can't lock it - we might not have privileges. */
682684
viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
@@ -910,8 +912,15 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
910912
appendStringInfoString(&buf, " TRUNCATE");
911913
findx++;
912914
}
915+
916+
/*
917+
* In non-pretty mode, always schema-qualify the target table name for
918+
* safety. In pretty mode, schema-qualify only if not visible.
919+
*/
913920
appendStringInfo(&buf, " ON %s ",
914-
generate_relation_name(trigrec->tgrelid, NIL));
921+
pretty ?
922+
generate_relation_name(trigrec->tgrelid, NIL) :
923+
generate_qualified_relation_name(trigrec->tgrelid));
915924

916925
if (OidIsValid(trigrec->tgconstraint))
917926
{
@@ -984,7 +993,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
984993
context.windowClause = NIL;
985994
context.windowTList = NIL;
986995
context.varprefix = true;
987-
context.prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
996+
context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
988997
context.wrapColumn = WRAP_COLUMN_DEFAULT;
989998
context.indentLevel = PRETTYINDENT_STD;
990999
context.special_exprkind = EXPR_KIND_NONE;
@@ -1071,7 +1080,7 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
10711080
int prettyFlags;
10721081
char *res;
10731082

1074-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1083+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
10751084

10761085
res = pg_get_indexdef_worker(indexrelid, colno, NULL, colno != 0, false,
10771086
prettyFlags, true);
@@ -1099,7 +1108,8 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
10991108
{
11001109
int prettyFlags;
11011110

1102-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1111+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1112+
11031113
return pg_get_indexdef_worker(indexrelid, 0, NULL, true, false,
11041114
prettyFlags, false);
11051115
}
@@ -1229,7 +1239,9 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
12291239
appendStringInfo(&buf, "CREATE %sINDEX %s ON %s USING %s (",
12301240
idxrec->indisunique ? "UNIQUE " : "",
12311241
quote_identifier(NameStr(idxrelrec->relname)),
1232-
generate_relation_name(indrelid, NIL),
1242+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
1243+
generate_relation_name(indrelid, NIL) :
1244+
generate_qualified_relation_name(indrelid),
12331245
quote_identifier(NameStr(amrec->amname)));
12341246
else /* currently, must be EXCLUDE constraint */
12351247
appendStringInfo(&buf, "EXCLUDE USING %s (",
@@ -1427,7 +1439,7 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
14271439
int prettyFlags;
14281440
char *res;
14291441

1430-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1442+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
14311443

14321444
res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true);
14331445

@@ -1869,7 +1881,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
18691881
int prettyFlags;
18701882
char *relname;
18711883

1872-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1884+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
18731885

18741886
if (OidIsValid(relid))
18751887
{
@@ -4300,7 +4312,10 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
43004312
}
43014313

43024314
/* The relation the rule is fired on */
4303-
appendStringInfo(buf, " TO %s", generate_relation_name(ev_class, NIL));
4315+
appendStringInfo(buf, " TO %s",
4316+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
4317+
generate_relation_name(ev_class, NIL) :
4318+
generate_qualified_relation_name(ev_class));
43044319

43054320
/* If the rule has an event qualification, add it */
43064321
if (ev_qual == NULL)

‎src/bin/pg_dump/dumputils.c

Lines changed: 71 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
3434
*
3535
* name: the object name, in the form to use in the commands (already quoted)
3636
* subname: the sub-object name, if any (already quoted); NULL if none
37+
* nspname: the namespace the object is in (NULL if none); not pre-quoted
3738
* type: the object type (as seen in GRANT command: must be one of
3839
* TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
3940< 1241 /td>
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT)
@@ -54,7 +55,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
5455
* since this routine uses fmtId() internally.
5556
*/
5657
bool
57-
buildACLCommands(const char *name, const char *subname,
58+
buildACLCommands(const char *name, const char *subname, const char *nspname,
5859
const char *type, const char *acls, const char *racls,
5960
const char *owner, const char *prefix, int remoteVersion,
6061
PQExpBuffer sql)
@@ -154,7 +155,10 @@ buildACLCommands(const char *name, const char *subname,
154155
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
155156
if (subname)
156157
appendPQExpBuffer(firstsql, "(%s)", subname);
157-
appendPQExpBuffer(firstsql, " ON %s %s FROM PUBLIC;\n", type, name);
158+
appendPQExpBuffer(firstsql, " ON %s ", type);
159+
if (nspname && *nspname)
160+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
161+
appendPQExpBuffer(firstsql, "%s FROM PUBLIC;\n", name);
158162
}
159163
else
160164
{
@@ -172,8 +176,11 @@ buildACLCommands(const char *name, const char *subname,
172176
{
173177
if (privs->len > 0)
174178
{
175-
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s %s FROM ",
176-
prefix, privs->data, type, name);
179+
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
180+
prefix, privs->data, type);
181+
if (nspname && *nspname)
182+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
183+
appendPQExpBuffer(firstsql, "%s FROM ", name);
177184
if (grantee->len == 0)
178185
appendPQExpBufferStr(firstsql, "PUBLIC;\n");
179186
else if (strncmp(grantee->data, "group ",
@@ -187,8 +194,11 @@ buildACLCommands(const char *name, const char *subname,
187194
if (privswgo->len > 0)
188195
{
189196
appendPQExpBuffer(firstsql,
190-
"%sREVOKE GRANT OPTION FOR %s ON %s %s FROM ",
191-
prefix, privswgo->data, type, name);
197+
"%sREVOKE GRANT OPTION FOR %s ON %s ",
198+
prefix, privswgo->data, type);
199+
if (nspname && *nspname)
200+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
201+
appendPQExpBuffer(firstsql, "%s FROM ", name);
192202
if (grantee->len == 0)
193203
appendPQExpBufferStr(firstsql, "PUBLIC");
194204
else if (strncmp(grantee->data, "group ",
@@ -255,18 +265,33 @@ buildACLCommands(const char *name, const char *subname,
255265
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
256266
if (subname)
257267
appendPQExpBuffer(firstsql, "(%s)", subname);
258-
appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
259-
type, name, fmtId(grantee->data));
268+
appendPQExpBuffer(firstsql, " ON %s ", type);
269+
if (nspname && *nspname)
270+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
271+
appendPQExpBuffer(firstsql, "%s FROM %s;\n",
272+
name, fmtId(grantee->data));
260273
if (privs->len > 0)
274+
{
261275
appendPQExpBuffer(firstsql,
262-
"%sGRANT %s ON %s %s TO %s;\n",
263-
prefix, privs->data, type, name,
264-
fmtId(grantee->data));
276+
"%sGRANT %s ON %s ",
277+
prefix, privs->data, type);
278+
if (nspname && *nspname)
279+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
280+
appendPQExpBuffer(firstsql,
281+
"%s TO %s;\n",
282+
name, fmtId(grantee->data));
283+
}
265284
if (privswgo->len > 0)
285+
{
266286
appendPQExpBuffer(firstsql,
267-
"%sGRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
268-
prefix, privswgo->data, type, name,
269-
fmtId(grantee->data));
287+
"%sGRANT %s ON %s ",
288+
prefix, privswgo->data, type);
289+
if (nspname && *nspname)
290+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
291+
appendPQExpBuffer(firstsql,
292+
"%s TO %s WITH GRANT OPTION;\n",
293+
name, fmtId(grantee->data));
294+
}
270295
}
271296
}
272297
else
@@ -288,8 +313,11 @@ buildACLCommands(const char *name, const char *subname,
288313

289314
if (privs->len > 0)
290315
{
291-
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
292-
prefix, privs->data, type, name);
316+
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
317+
prefix, privs->data, type);
318+
if (nspname && *nspname)
319+
appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
320+
appendPQExpBuffer(secondsql, "%s TO ", name);
293321
if (grantee->len == 0)
294322
appendPQExpBufferStr(secondsql, "PUBLIC;\n");
295323
else if (strncmp(grantee->data, "group ",
@@ -301,8 +329,11 @@ buildACLCommands(const char *name, const char *subname,
301329
}
302330
if (privswgo->len > 0)
303331
{
304-
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
305-
prefix, privswgo->data, type, name);
332+
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
333+
prefix, privswgo->data, type);
334+
if (nspname && *nspname)
335+
appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
336+
appendPQExpBuffer(secondsql, "%s TO ", name);
306337
if (grantee->len == 0)
307338
appendPQExpBufferStr(secondsql, "PUBLIC");
308339
else if (strncmp(grantee->data, "group ",
@@ -332,8 +363,11 @@ buildACLCommands(const char *name, const char *subname,
332363
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
333364
if (subname)
334365
appendPQExpBuffer(firstsql, "(%s)", subname);
335-
appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
336-
type, name, fmtId(owner));
366+
appendPQExpBuffer(firstsql, " ON %s ", type);
367+
if (nspname && *nspname)
368+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
369+
appendPQExpBuffer(firstsql, "%s FROM %s;\n",
370+
name, fmtId(owner));
337371
}
338372

339373
destroyPQExpBuffer(grantee);
@@ -392,7 +426,8 @@ buildDefaultACLCommands(const char *type, const char *nspname,
392426
if (strlen(initacls) != 0 || strlen(initracls) != 0)
393427
{
394428
appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
395-
if (!buildACLCommands("", NULL, type, initacls, initracls, owner,
429+
if (!buildACLCommands("", NULL, NULL, type,
430+
initacls, initracls, owner,
396431
prefix->data, remoteVersion, sql))
397432
{
398433
destroyPQExpBuffer(prefix);
@@ -401,7 +436,8 @@ buildDefaultACLCommands(const char *type, const char *nspname,
401436
appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
402437
}
403438

404-
if (!buildACLCommands("", NULL, type, acls, racls, owner,
439+
if (!buildACLCommands("", NULL, NULL, type,
440+
acls, racls, owner,
405441
prefix->data, remoteVersion, sql))
406442
{
407443
destroyPQExpBuffer(prefix);
@@ -645,26 +681,32 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
645681
* buildShSecLabelQuery
646682
*
647683
* Build a query to retrieve security labels for a shared object.
684+
* The object is identified by its OID plus the name of the catalog
685+
* it can be found in (e.g., "pg_database" for database names).
686+
* The query is appended to "sql". (We don't execute it here so as to
687+
* keep this file free of assumptions about how to deal with SQL errors.)
648688
*/
649689
void
650-
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, uint32 objectId,
690+
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, Oid objectId,
651691
PQExpBuffer sql)
652692
{
653693
appendPQExpBuffer(sql,
654694
"SELECT provider, label FROM pg_catalog.pg_shseclabel "
655-
"WHERE classoid = '%s'::pg_catalog.regclass AND "
656-
"objoid = %u", catalog_name, objectId);
695+
"WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
696+
"AND objoid = '%u'", catalog_name, objectId);
657697
}
658698

659699
/*
660700
* emitShSecLabels
661701
*
662-
* Format security label data retrieved by the query generated in
663-
* buildShSecLabelQuery.
702+
* Construct SECURITY LABEL commands using the data retrieved by the query
703+
* generated by buildShSecLabelQuery, and append them to "buffer".
704+
* Here, the target object is identified by its type name (e.g. "DATABASE")
705+
* and its name (not pre-quoted).
664706
*/
665707
void
666708
emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
667-
const char *target, const char *objname)
709+
const char *objtype, const char *objname)
668710
{
669711
int i;
670712

@@ -676,7 +718,7 @@ emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
676718
/* must use fmtId result before calling it again */
677719
appendPQExpBuffer(buffer,
678720
"SECURITY LABEL FOR %s ON %s",
679-
fmtId(provider), target);
721+
fmtId(provider), objtype);
680722
appendPQExpBuffer(buffer,
681723
" %s IS ",
682724
fmtId(objname));

‎src/bin/pg_dump/dumputils.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
#endif
3737

3838

39-
extern bool buildACLCommands(const char *name, const char *subname,
39+
extern bool buildACLCommands(const char *name, const char *subname, const char *nspname,
4040
const char *type, const char *acls, const char *racls,
4141
const char *owner, const char *prefix, int remoteVersion,
4242
PQExpBuffer sql);
@@ -47,9 +47,9 @@ extern bool buildDefaultACLCommands(const char *type, const char *nspname,
4747
int remoteVersion,
4848
PQExpBuffer sql);
4949
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
50-
uint32 objectId, PQExpBuffer sql);
50+
Oid objectId, PQExpBuffer sql);
5151
extern void emitShSecLabels(PGconn *conn, PGresult *res,
52-
PQExpBuffer buffer, const char *target, const char *objname);
52+
PQExpBuffer buffer, const char *objtype, const char *objname);
5353

5454
extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
5555
PQExpBuffer init_acl_subquery, PQExpBuffer init_racl_subquery,

‎src/bin/pg_dump/pg_backup.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ typedef struct Archive
186186
/* info needed for string escaping */
187187
int encoding; /* libpq code for client_encoding */
188188
bool std_strings; /* standard_conforming_strings */
189+
190+
/* other important stuff */
191+
char *searchpath; /* search_path to set during restore */
189192
char *use_role; /* Issue SET ROLE to this */
190193

191194
/* error handling */

0 commit comments

Comments
 (0)
0