Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3d2aed6

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 3bf05e0 commit 3d2aed6

16 files changed

+1166
-1492
lines changed

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

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,17 @@
8686
#define PRETTYINDENT_LIMIT 40 /* wrap limit */
8787

8888
/* Pretty flags */
89-
#define PRETTYFLAG_PAREN 1
90-
#define PRETTYFLAG_INDENT 2
89+
#define PRETTYFLAG_PAREN 0x0001
90+
#define PRETTYFLAG_INDENT 0x0002
91+
#define PRETTYFLAG_SCHEMA 0x0004
9192

9293
/* Default line length for pretty-print wrapping: 0 means wrap always */
9394
#define WRAP_COLUMN_DEFAULT 0
9495

95-
/* macro to test if pretty action needed */
96+
/* macros to test if pretty action needed */
9697
#define PRETTY_PAREN(context) ((context)->prettyFlags & PRETTYFLAG_PAREN)
9798
#define PRETTY_INDENT(context) ((context)->prettyFlags & PRETTYFLAG_INDENT)
99+
#define PRETTY_SCHEMA(context) ((context)->prettyFlags & PRETTYFLAG_SCHEMA)
98100

99101

100102
/* ----------
@@ -499,7 +501,7 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
499501
int prettyFlags;
500502
char *res;
501503

502-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
504+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
503505

504506
res = pg_get_ruledef_worker(ruleoid, prettyFlags);
505507

@@ -620,7 +622,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
620622
int prettyFlags;
621623
char *res;
622624

623-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
625+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
624626

625627
res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
626628

@@ -640,7 +642,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
640642
char *res;
641643

642644
/* calling this implies we want pretty printing */
643-
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT;
645+
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
644646

645647
res = pg_get_viewdef_worker(viewoid, prettyFlags, wrap);
646648

@@ -686,7 +688,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
686688
Oid viewoid;
687689
char *res;
688690

689-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
691+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
690692

691693
/* Look up view name. Can't lock it - we might not have privileges. */
692694
viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
@@ -922,8 +924,15 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
922924
appendStringInfoString(&buf, " TRUNCATE");
923925
findx++;
924926
}
927+
928+
/*
929+
* In non-pretty mode, always schema-qualify the target table name for
930+
* safety. In pretty mode, schema-qualify only if not visible.
931+
*/
925932
appendStringInfo(&buf, " ON %s ",
926-
generate_relation_name(trigrec->tgrelid, NIL));
933+
pretty ?
934+
generate_relation_name(trigrec->tgrelid, NIL) :
935+
generate_qualified_relation_name(trigrec->tgrelid));
927936

928937
if (OidIsValid(trigrec->tgconstraint))
929938
{
@@ -1017,7 +1026,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
10171026
context.windowClause = NIL;
10181027
context.windowTList = NIL;
10191028
context.varprefix = true;
1020-
context.prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1029+
context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
10211030
context.wrapColumn = WRAP_COLUMN_DEFAULT;
10221031
context.indentLevel = PRETTYINDENT_STD;
10231032
context.special_exprkind = EXPR_KIND_NONE;
@@ -1104,7 +1113,7 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
11041113
int prettyFlags;
11051114
char *res;
11061115

1107-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1116+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
11081117

11091118
res = pg_get_indexdef_worker(indexrelid, colno, NULL, colno != 0, false,
11101119
false, prettyFlags, true);
@@ -1132,7 +1141,8 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
11321141
{
11331142
int prettyFlags;
11341143

1135-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1144+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1145+
11361146
return pg_get_indexdef_worker(indexrelid, 0, NULL, true, false, false,
11371147
prettyFlags, false);
11381148
}
@@ -1264,7 +1274,9 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
12641274
quote_identifier(NameStr(idxrelrec->relname)),
12651275
idxrelrec->relkind == RELKIND_PARTITIONED_INDEX
12661276
&& !inherits ? "ONLY " : "",
1267-
generate_relation_name(indrelid, NIL),
1277+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
1278+
generate_relation_name(indrelid, NIL) :
1279+
generate_qualified_relation_name(indrelid),
12681280
quote_identifier(NameStr(amrec->amname)));
12691281
else /* currently, must be EXCLUDE constraint */
12701282
appendStringInfo(&buf, "EXCLUDE USING %s (",
@@ -1575,7 +1587,8 @@ pg_get_partkeydef_columns(Oid relid, bool pretty)
15751587
{
15761588
int prettyFlags;
15771589

1578-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1590+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1591+
15791592
return pg_get_partkeydef_worker(relid, prettyFlags, true, false);
15801593
}
15811594

@@ -1803,7 +1816,7 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
18031816
int prettyFlags;
18041817
char *res;
18051818

1806-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1819+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
18071820

18081821
res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true);
18091822

@@ -2258,7 +2271,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
22582271
int prettyFlags;
22592272
char *relname;
22602273

2261-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
2274+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
22622275

22632276
if (OidIsValid(relid))
22642277
{
@@ -4709,7 +4722,10 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
47094722
}
47104723

47114724
/* The relation the rule is fired on */
4712-
appendStringInfo(buf, " TO %s", generate_relation_name(ev_class, NIL));
4725+
appendStringInfo(buf, " TO %s",
4726+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
4727+
generate_relation_name(ev_class, NIL) :
4728+
generate_qualified_relation_name(ev_class));
47134729

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

‎src/bin/pg_dump/dumputils.c

Lines changed: 71 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
3232
*
3333
* name: the object name, in the form to use in the commands (already quoted)
3434
* subname: the sub-object name, if any (already quoted); NULL if none
35+
* nspname: the namespace the object is in (NULL if none); not pre-quoted
3536
* type: the object type (as seen in GRANT command: must be one of
3637
* TABLE, SEQUENCE, FUNCTION, PROCEDURE, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
3738
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT)
@@ -52,7 +53,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
5253
* since this routine uses fmtId() internally.
5354
*/
5455
bool
55-
buildACLCommands(const char *name, const char *subname,
56+
buildACLCommands(const char *name, const char *subname, const char *nspname,
5657
const char *type, const char *acls, const char *racls,
5758
const char *owner, const char *prefix, int remoteVersion,
5859
PQExpBuffer sql)
@@ -152,7 +153,10 @@ buildACLCommands(const char *name, const char *subname,
152153
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
153154
if (subname)
154155
appendPQExpBuffer(firstsql, "(%s)", subname);
155-
appendPQExpBuffer(firstsql, " ON %s %s FROM PUBLIC;\n", type, name);
156+
appendPQExpBuffer(firstsql, " ON %s ", type);
157+
if (nspname && *nspname)
158+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
159+
appendPQExpBuffer(firstsql, "%s FROM PUBLIC;\n", name);
156160
}
157161
else
158162
{
@@ -170,8 +174,11 @@ buildACLCommands(const char *name, const char *subname,
170174
{
171175
if (privs->len > 0)
172176
{
173-
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s %s FROM ",
174-
prefix, privs->data, type, name);
177+
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
178+
prefix, privs->data, type);
179+
if (nspname && *nspname)
180+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
181+
appendPQExpBuffer(firstsql, "%s FROM ", name);
175182
if (grantee->len == 0)
176183
appendPQExpBufferStr(firstsql, "PUBLIC;\n");
177184
else if (strncmp(grantee->data, "group ",
@@ -185,8 +192,11 @@ buildACLCommands(const char *name, const char *subname,
185192
if (privswgo->len > 0)
186193
{
187194
appendPQExpBuffer(firstsql,
188-
"%sREVOKE GRANT OPTION FOR %s ON %s %s FROM ",
189-
prefix, privswgo->data, type, name);
195+
"%sREVOKE GRANT OPTION FOR %s ON %s ",
196+
prefix, privswgo->data, type);
197+
if (nspname && *nspname)
198+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
199+
appendPQExpBuffer(firstsql, "%s FROM ", name);
190200
if (grantee->len == 0)
191201
appendPQExpBufferStr(firstsql, "PUBLIC");
192202
else if (strncmp(grantee->data, "group ",
@@ -251,18 +261,33 @@ buildACLCommands(const char *name, const char *subname,
251261
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
252262
if (subname)
253263
appendPQExpBuffer(firstsql, "(%s)", subname);
254-
appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
255-
type, name, fmtId(grantee->data));
264+
appendPQExpBuffer(firstsql, " ON %s ", type);
265+
if (nspname && *nspname)
266+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
267+
appendPQExpBuffer(firstsql, "%s FROM %s;\n",
268+
name, fmtId(grantee->data));
256269
if (privs->len > 0)
270+
{
257271
appendPQExpBuffer(firstsql,
258-
"%sGRANT %s ON %s %s TO %s;\n",
259-
prefix, privs->data, type, name,
260-
fmtId(grantee->data));
272+
"%sGRANT %s ON %s ",
273+
prefix, privs->data, type);
274+
if (nspname && *nspname)
275+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
276+
appendPQExpBuffer(firstsql,
277+
"%s TO %s;\n",
278+
name, fmtId(grantee->data));
279+
}
261280
if (privswgo->len > 0)
281+
{
262282
appendPQExpBuffer(firstsql,
263-
"%sGRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
264-
prefix, privswgo->data, type, name,
265-
fmtId(grantee->data));
283+
"%sGRANT %s ON %s ",
284+
prefix, privswgo->data, type);
285+
if (nspname && *nspname)
286+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
287+
appendPQExpBuffer(firstsql,
288+
"%s TO %s WITH GRANT OPTION;\n",
289+
name, fmtId(grantee->data));
290+
}
266291
}
267292
}
268293
else
@@ -284,8 +309,11 @@ buildACLCommands(const char *name, const char *subname,
284309

285310
if (privs->len > 0)
286311
{
287-
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
288-
prefix, privs->data, type, name);
312+
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
313+
prefix, privs->data, type);
314+
if (nspname && *nspname)
315+
appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
316+
appendPQExpBuffer(secondsql, "%s TO ", name);
289317
if (grantee->len == 0)
290318
appendPQExpBufferStr(secondsql, "PUBLIC;\n");
291319
else if (strncmp(grantee->data, "group ",
@@ -297,8 +325,11 @@ buildACLCommands(const char *name, const char *subname,
297325
}
298326
if (privswgo->len > 0)
299327
{
300-
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
301-
prefix, privswgo->data, type, name);
328+
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
329+
prefix, privswgo->data, type);
330+
if (nspname && *nspname)
331+
appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
332+
appendPQExpBuffer(secondsql, "%s TO ", name);
302333
if (grantee->len == 0)
303334
appendPQExpBufferStr(secondsql, "PUBLIC");
304335
else if (strncmp(grantee->data, "group ",
@@ -328,8 +359,11 @@ buildACLCommands(const char *name, const char *subname,
328359
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
329360
if (subname)
330361
appendPQExpBuffer(firstsql, "(%s)", subname);
331-
appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
332-
type, name, fmtId(owner));
362+
appendPQExpBuffer(firstsql, " ON %s ", type);
363+
if (nspname && *nspname)
364+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
365+
appendPQExpBuffer(firstsql, "%s FROM %s;\n",
366+
name, fmtId(owner));
333367
}
334368

335369
destroyPQExpBuffer(grantee);
@@ -388,7 +422,8 @@ buildDefaultACLCommands(const char *type, const char *nspname,
388422
if (strlen(initacls) != 0 || strlen(initracls) != 0)
389423
{
390424
appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
391-
if (!buildACLCommands("", NULL, type, initacls, initracls, owner,
425+
if (!buildACLCommands("", NULL, NULL, type,
426+
initacls, initracls, owner,
392427
prefix->data, remoteVersion, sql))
393428
{
394429
destroyPQExpBuffer(prefix);
@@ -397,7 +432,8 @@ buildDefaultACLCommands(const char *type, const char *nspname,
397432
appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
398433
}
399434

400-
if (!buildACLCommands("", NULL, type, acls, racls, owner,
435+
if (!buildACLCommands("", NULL, NULL, type,
436+
acls, racls, owner,
401437
prefix->data, remoteVersion, sql))
402438
{
403439
destroyPQExpBuffer(prefix);
@@ -641,26 +677,32 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
641677
* buildShSecLabelQuery
642678
*
643679
* Build a query to retrieve security labels for a shared object.
680+
* The object is identified by its OID plus the name of the catalog
681+
* it can be found in (e.g., "pg_database" for database names).
682+
* The query is appended to "sql". (We don't execute it here so as to
683+
* keep this file free of assumptions about how to deal with SQL errors.)
644684
*/
645685
void
646-
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, uint32 objectId,
686+
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, Oid objectId,
647687
PQExpBuffer sql)
648688
{
649689
appendPQExpBuffer(sql,
650690
"SELECT provider, label FROM pg_catalog.pg_shseclabel "
651-
"WHERE classoid = '%s'::pg_catalog.regclass AND "
652-
"objoid = %u", catalog_name, objectId);
691+
"WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
692+
"AND objoid = '%u'", catalog_name, objectId);
653693
}
654694

655695
/*
656696
* emitShSecLabels
657697
*
658-
* Format security label data retrieved by the query generated in
659-
* buildShSecLabelQuery.
698+
* Construct SECURITY LABEL commands using the data retrieved by the query
699+
* generated by buildShSecLabelQuery, and append them to "buffer".
700+
* Here, the target object is identified by its type name (e.g. "DATABASE")
701+
* and its name (not pre-quoted).
660702
*/
661703
void
662704
emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
663-
const char *target, const char *objname)
705+
const char *objtype, const char *objname)
664706
{
665707
int i;
666708

@@ -672,7 +714,7 @@ emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
672714
/* must use fmtId result before calling it again */
673715
appendPQExpBuffer(buffer,
674716
"SECURITY LABEL FOR %s ON %s",
675-
fmtId(provider), target);
717+
fmtId(provider), objtype);
676718
appendPQExpBuffer(buffer,
677719
" %s IS ",
678720
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
@@ -197,6 +197,9 @@ typedef struct Archive
197197
/* info needed for string escaping */
198198
int encoding; /* libpq code for client_encoding */
199199
bool std_strings; /* standard_conforming_strings */
200+
201+
/* other important stuff */
202+
char *searchpath; /* search_path to set during restore */
200203
char *use_role; /* Issue SET ROLE to this */
201204

202205
/* error handling */

0 commit comments

Comments
 (0)