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

Commit b8a2908

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 8c97abb commit b8a2908

File tree

15 files changed

+1113
-1415
lines changed

15 files changed

+1113
-1415
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
/* ----------
@@ -498,7 +500,7 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
498500
int prettyFlags;
499501
char *res;
500502

501-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
503+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
502504

503505
res = pg_get_ruledef_worker(ruleoid, prettyFlags);
504506

@@ -619,7 +621,7 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
619621
int prettyFlags;
620622
char *res;
621623

622-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
624+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
623625

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

@@ -639,7 +641,7 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
639641
char *res;
640642

641643
/* calling this implies we want pretty printing */
642-
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT;
644+
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
643645

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

@@ -685,7 +687,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
685687
Oid viewoid;
686688
char *res;
687689

688-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
690+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
689691

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

927936
if (OidIsValid(trigrec->tgconstraint))
928937
{
@@ -1016,7 +1025,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
10161025
context.windowClause = NIL;
10171026
context.windowTList = NIL;
10181027
context.varprefix = true;
1019-
context.prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1028+
context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
10201029
context.wrapColumn = WRAP_COLUMN_DEFAULT;
10211030
context.indentLevel = PRETTYINDENT_STD;
10221031
context.special_exprkind = EXPR_KIND_NONE;
@@ -1103,7 +1112,7 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
11031112
int prettyFlags;
11041113
char *res;
11051114

1106-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1115+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
11071116

11081117
res = pg_get_indexdef_worker(indexrelid, colno, NULL, colno != 0, false,
11091118
prettyFlags, true);
@@ -1131,7 +1140,8 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
11311140
{
11321141
int prettyFlags;
11331142

1134-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1143+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1144+
11351145
return pg_get_indexdef_worker(indexrelid, 0, NULL, true, false,
11361146
prettyFlags, false);
11371147
}
@@ -1261,7 +1271,9 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
12611271
appendStringInfo(&buf, "CREATE %sINDEX %s ON %s USING %s (",
12621272
idxrec->indisunique ? "UNIQUE " : "",
12631273
quote_identifier(NameStr(idxrelrec->relname)),
1264-
generate_relation_name(indrelid, NIL),
1274+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
1275+
generate_relation_name(indrelid, NIL) :
1276+
generate_qualified_relation_name(indrelid),
12651277
quote_identifier(NameStr(amrec->amname)));
12661278
else /* currently, must be EXCLUDE constraint */
12671279
appendStringInfo(&buf, "EXCLUDE USING %s (",
@@ -1572,7 +1584,8 @@ pg_get_partkeydef_columns(Oid relid, bool pretty)
15721584
{
15731585
int prettyFlags;
15741586

1575-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1587+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1588+
15761589
return pg_get_partkeydef_worker(relid, prettyFlags, true, false);
15771590
}
15781591

@@ -1796,7 +1809,7 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
17961809
int prettyFlags;
17971810
char *res;
17981811

1799-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1812+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
18001813

18011814
res = pg_get_constraintdef_worker(constraintId, false, prettyFlags, true);
18021815

@@ -2238,7 +2251,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
22382251
int prettyFlags;
22392252
char *relname;
22402253

2241-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
2254+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
22422255

22432256
if (OidIsValid(relid))
22442257
{
@@ -4671,7 +4684,10 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
46714684
}
46724685

46734686
/* The relation the rule is fired on */
4674-
appendStringInfo(buf, " TO %s", generate_relation_name(ev_class, NIL));
4687+
appendStringInfo(buf, " TO %s",
4688+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
4689+
generate_relation_name(ev_class, NIL) :
4690+
generate_qualified_relation_name(ev_class));
46754691

46764692
/* If the rule has an event qualification, add it */
46774693
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, 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);
@@ -638,26 +674,32 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
638674
* buildShSecLabelQuery
639675
*
640676
* Build a query to retrieve security labels for a shared object.
677+
* The object is identified by its OID plus the name of the catalog
678+
* it can be found in (e.g., "pg_database" for database names).
679+
* The query is appended to "sql". (We don't execute it here so as to
680+
* keep this file free of assumptions about how to deal with SQL errors.)
641681
*/
642682
void
643-
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, uint32 objectId,
683+
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, Oid objectId,
644684
PQExpBuffer sql)
645685
{
646686
appendPQExpBuffer(sql,
647687
"SELECT provider, label FROM pg_catalog.pg_shseclabel "
648-
"WHERE classoid = '%s'::pg_catalog.regclass AND "
649-
"objoid = %u", catalog_name, objectId);
688+
"WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
689+
"AND objoid = '%u'", catalog_name, objectId);
650690
}
651691

652692
/*
653693
* emitShSecLabels
654694
*
655-
* Format security label data retrieved by the query generated in
656-
* buildShSecLabelQuery.
695+
* Construct SECURITY LABEL commands using the data retrieved by the query
696+
* generated by buildShSecLabelQuery, and append them to "buffer".
697+
* Here, the target object is identified by its type name (e.g. "DATABASE")
698+
* and its name (not pre-quoted).
657699
*/
658700
void
659701
emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
660-
const char *target, const char *objname)
702+
const char *objtype, const char *objname)
661703
{
662704
int i;
663705

@@ -669,7 +711,7 @@ emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
669711
/* must use fmtId result before calling it again */
670712
appendPQExpBuffer(buffer,
671713
"SECURITY LABEL FOR %s ON %s",
672-
fmtId(provider), target);
714+
fmtId(provider), objtype);
673715
appendPQExpBuffer(buffer,
674716
" %s IS ",
675717
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
@@ -194,6 +194,9 @@ typedef struct Archive
194194
/* info needed for string escaping */
195195
int encoding; /* libpq code for client_encoding */
196196
bool std_strings; /* standard_conforming_strings */
197+
198+
/* other important stuff */
199+
char *searchpath; /* search_path to set during restore */
197200
char *use_role; /* Issue SET ROLE to this */
198201

199202
/* error handling */

0 commit comments

Comments
 (0)