Skip to content

Commit 9f6e529

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 fd090d0 commit 9f6e529

File tree

12 files changed

+842
-1081
lines changed

12 files changed

+842
-1081
lines changed

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

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,17 @@
7777
#define PRETTYINDENT_LIMIT 40 /* wrap limit */
7878

7979
/* Pretty flags */
80-
#define PRETTYFLAG_PAREN 1
81-
#define PRETTYFLAG_INDENT 2
80+
#define PRETTYFLAG_PAREN 0x0001
81+
#define PRETTYFLAG_INDENT 0x0002
82+
#define PRETTYFLAG_SCHEMA 0x0004
8283

8384
/* Default line length for pretty-print wrapping: 0 means wrap always */
8485
#define WRAP_COLUMN_DEFAULT 0
8586

86-
/* macro to test if pretty action needed */
87+
/* macros to test if pretty action needed */
8788
#define PRETTY_PAREN(context) ((context)->prettyFlags & PRETTYFLAG_PAREN)
8889
#define PRETTY_INDENT(context) ((context)->prettyFlags & PRETTYFLAG_INDENT)
90+
#define PRETTY_SCHEMA(context) ((context)->prettyFlags & PRETTYFLAG_SCHEMA)
8991

9092

9193
/* ----------
@@ -458,7 +460,8 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
458460
bool pretty = PG_GETARG_BOOL(1);
459461
int prettyFlags;
460462

461-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
463+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
464+
462465
PG_RETURN_TEXT_P(string_to_text(pg_get_ruledef_worker(ruleoid, prettyFlags)));
463466
}
464467

@@ -557,7 +560,8 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
557560
bool pretty = PG_GETARG_BOOL(1);
558561
int prettyFlags;
559562

560-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
563+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
564+
561565
PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT)));
562566
}
563567

@@ -570,7 +574,8 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
570574
int prettyFlags;
571575

572576
/* calling this implies we want pretty printing */
573-
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT;
577+
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
578+
574579
PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags, wrap)));
575580
}
576581

@@ -603,7 +608,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
603608
RangeVar *viewrel;
604609
Oid viewoid;
605610

606-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
611+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
607612

608613
/* Look up view name. Can't lock it - we might not have privileges. */
609614
viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
@@ -807,8 +812,15 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
807812
appendStringInfoString(&buf, " TRUNCATE");
808813
findx++;
809814
}
815+
816+
/*
817+
* In non-pretty mode, always schema-qualify the target table name for
818+
* safety. In pretty mode, schema-qualify only if not visible.
819+
*/
810820
appendStringInfo(&buf, " ON %s ",
811-
generate_relation_name(trigrec->tgrelid, NIL));
821+
pretty ?
822+
generate_relation_name(trigrec->tgrelid, NIL) :
823+
generate_qualified_relation_name(trigrec->tgrelid));
812824

813825
if (OidIsValid(trigrec->tgconstraint))
814826
{
@@ -881,7 +893,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
881893
context.windowClause = NIL;
882894
context.windowTList = NIL;
883895
context.varprefix = true;
884-
context.prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
896+
context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
885897
context.wrapColumn = WRAP_COLUMN_DEFAULT;
886898
context.indentLevel = PRETTYINDENT_STD;
887899

@@ -961,7 +973,8 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
961973
bool pretty = PG_GETARG_BOOL(2);
962974
int prettyFlags;
963975

964-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
976+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
977+
965978
PG_RETURN_TEXT_P(string_to_text(pg_get_indexdef_worker(indexrelid, colno,
966979
NULL,
967980
colno != 0,
@@ -986,7 +999,8 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
986999
{
9871000
int prettyFlags;
9881001

989-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1002+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1003+
9901004
return pg_get_indexdef_worker(indexrelid, 0, NULL, true, false, prettyFlags);
9911005
}
9921006

@@ -1107,7 +1121,9 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
11071121
appendStringInfo(&buf, "CREATE %sINDEX %s ON %s USING %s (",
11081122
idxrec->indisunique ? "UNIQUE " : "",
11091123
quote_identifier(NameStr(idxrelrec->relname)),
1110-
generate_relation_name(indrelid, NIL),
1124+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
1125+
generate_relation_name(indrelid, NIL) :
1126+
generate_qualified_relation_name(indrelid),
11111127
quote_identifier(NameStr(amrec->amname)));
11121128
else /* currently, must be EXCLUDE constraint */
11131129
appendStringInfo(&buf, "EXCLUDE USING %s (",
@@ -1299,7 +1315,8 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
12991315
bool pretty = PG_GETARG_BOOL(1);
13001316
int prettyFlags;
13011317

1302-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1318+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1319+
13031320
PG_RETURN_TEXT_P(string_to_text(pg_get_constraintdef_worker(constraintId,
13041321
false,
13051322
prettyFlags)));
@@ -1728,7 +1745,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
17281745
int prettyFlags;
17291746
char *relname;
17301747

1731-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1748+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
17321749

17331750
if (OidIsValid(relid))
17341751
{
@@ -4113,7 +4130,10 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
41134130
}
41144131

41154132
/* The relation the rule is fired on */
4116-
appendStringInfo(buf, " TO %s", generate_relation_name(ev_class, NIL));
4133+
appendStringInfo(buf, " TO %s",
4134+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
4135+
generate_relation_name(ev_class, NIL) :
4136+
generate_qualified_relation_name(ev_class));
41174137

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

‎src/bin/pg_dump/dumputils.c

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,7 @@ parsePGArray(const char *atext, char ***itemarray, int *nitems)
718718
*
719719
* name: the object name, in the form to use in the commands (already quoted)
720720
* subname: the sub-object name, if any (already quoted); NULL if none
721+
* nspname: the namespace the object is in (NULL if none); not pre-quoted
721722
* type: the object type (as seen in GRANT command: must be one of
722723
* TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
723724
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT)
@@ -737,7 +738,7 @@ parsePGArray(const char *atext, char ***itemarray, int *nitems)
737738
* since this routine uses fmtId() internally.
738739
*/
739740
bool
740-
buildACLCommands(const char *name, const char *subname,
741+
buildACLCommands(const char *name, const char *subname, const char *nspname,
741742
const char *type, const char *acls, const char *owner,
742743
const char *prefix, int remoteVersion,
743744
PQExpBuffer sql)
@@ -791,7 +792,10 @@ buildACLCommands(const char *name, const char *subname,
791792
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
792793
if (subname)
793794
appendPQExpBuffer(firstsql, "(%s)", subname);
794-
appendPQExpBuffer(firstsql, " ON %s %s FROM PUBLIC;\n", type, name);
795+
appendPQExpBuffer(firstsql, " ON %s ", type);
796+
if (nspname && *nspname)
797+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
798+
appendPQExpBuffer(firstsql, "%s FROM PUBLIC;\n", name);
795799

796800
/*
797801
* We still need some hacking though to cover the case where new default
@@ -839,18 +843,33 @@ buildACLCommands(const char *name, const char *subname,
839843
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
840844
if (subname)
841845
appendPQExpBuffer(firstsql, "(%s)", subname);
842-
appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
843-
type, name, fmtId(grantee->data));
846+
appendPQExpBuffer(firstsql, " ON %s ", type);
847+
if (nspname && *nspname)
848+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
849+
appendPQExpBuffer(firstsql, "%s FROM %s;\n",
850+
name, fmtId(grantee->data));
844851
if (privs->len > 0)
852+
{
853+
appendPQExpBuffer(firstsql,
854+
"%sGRANT %s ON %s ",
855+
prefix, privs->data, type);
856+
if (nspname && *nspname)
857+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
845858
appendPQExpBuffer(firstsql,
846-
"%sGRANT %s ON %s %s TO %s;\n",
847-
prefix, privs->data, type, name,
848-
fmtId(grantee->data));
859+
"%s TO %s;\n",
860+
name, fmtId(grantee->data));
861+
}
849862
if (privswgo->len > 0)
863+
{
864+
appendPQExpBuffer(firstsql,
865+
"%sGRANT %s ON %s ",
866+
prefix, privswgo->data, type);
867+
if (nspname && *nspname)
868+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
850869
appendPQExpBuffer(firstsql,
851-
"%sGRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
852-
prefix, privswgo->data, type, name,
853-
fmtId(grantee->data));
870+
"%s TO %s WITH GRANT OPTION;\n",
871+
name, fmtId(grantee->data));
872+
}
854873
}
855874
}
856875
else
@@ -865,8 +884,11 @@ buildACLCommands(const char *name, const char *subname,
865884

866885
if (privs->len > 0)
867886
{
868-
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
869-
prefix, privs->data, type, name);
887+
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
888+
prefix, privs->data, type);
889+
if (nspname && *nspname)
890+
appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
891+
appendPQExpBuffer(secondsql, "%s TO ", name);
870892
if (grantee->len == 0)
871893
appendPQExpBufferStr(secondsql, "PUBLIC;\n");
872894
else if (strncmp(grantee->data, "group ",
@@ -878,8 +900,11 @@ buildACLCommands(const char *name, const char *subname,
878900
}
879901
if (privswgo->len > 0)
880902
{
881-
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
882-
prefix, privswgo->data, type, name);
903+
appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
904+
prefix, privswgo->data, type);
905+
if (nspname && *nspname)
906+
appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
907+
appendPQExpBuffer(secondsql, "%s TO ", name);
883908
if (grantee->len == 0)
884909
appendPQExpBufferStr(secondsql, "PUBLIC");
885910
else if (strncmp(grantee->data, "group ",
@@ -906,8 +931,11 @@ buildACLCommands(const char *name, const char *subname,
906931
appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
907932
if (subname)
908933
appendPQExpBuffer(firstsql, "(%s)", subname);
909-
appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
910-
type, name, fmtId(owner));
934+
appendPQExpBuffer(firstsql, " ON %s ", type);
935+
if (nspname && *nspname)
936+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
937+
appendPQExpBuffer(firstsql, "%s FROM %s;\n",
938+
name, fmtId(owner));
911939
}
912940

913941
destroyPQExpBuffer(grantee);
@@ -958,7 +986,7 @@ buildDefaultACLCommands(const char *type, const char *nspname,
958986
if (nspname)
959987
appendPQExpBuffer(prefix, "IN SCHEMA %s ", fmtId(nspname));
960988

961-
result = buildACLCommands("", NULL,
989+
result = buildACLCommands("", NULL, NULL,
962990
type, acls, owner,
963991
prefix->data, remoteVersion,
964992
sql);
@@ -1412,26 +1440,32 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern,
14121440
* buildShSecLabelQuery
14131441
*
14141442
* Build a query to retrieve security labels for a shared object.
1443+
* The object is identified by its OID plus the name of the catalog
1444+
* it can be found in (e.g., "pg_database" for database names).
1445+
* The query is appended to "sql". (We don't execute it here so as to
1446+
* keep this file free of assumptions about how to deal with SQL errors.)
14151447
*/
14161448
void
1417-
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, uint32 objectId,
1449+
buildShSecLabelQuery(PGconn *conn, const char *catalog_name, Oid objectId,
14181450
PQExpBuffer sql)
14191451
{
14201452
appendPQExpBuffer(sql,
14211453
"SELECT provider, label FROM pg_catalog.pg_shseclabel "
1422-
"WHERE classoid = '%s'::pg_catalog.regclass AND "
1423-
"objoid = %u", catalog_name, objectId);
1454+
"WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
1455+
"AND objoid = '%u'", catalog_name, objectId);
14241456
}
14251457

14261458
/*
14271459
* emitShSecLabels
14281460
*
1429-
* Format security label data retrieved by the query generated in
1430-
* buildShSecLabelQuery.
1461+
* Construct SECURITY LABEL commands using the data retrieved by the query
1462+
* generated by buildShSecLabelQuery, and append them to "buffer".
1463+
* Here, the target object is identified by its type name (e.g. "DATABASE")
1464+
* and its name (not pre-quoted).
14311465
*/
14321466
void
14331467
emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
1434-
const char *target, const char *objname)
1468+
const char *objtype, const char *objname)
14351469
{
14361470
int i;
14371471

@@ -1443,7 +1477,7 @@ emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
14431477
/* must use fmtId result before calling it again */
14441478
appendPQExpBuffer(buffer,
14451479
"SECURITY LABEL FOR %s ON %s",
1446-
fmtId(provider), target);
1480+
fmtId(provider), objtype);
14471481
appendPQExpBuffer(buffer,
14481482
" %s IS ",
14491483
fmtId(objname));

‎src/bin/pg_dump/dumputils.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ extern void appendShellString(PQExpBuffer buf, const char *str);
5353
extern void appendConnStrVal(PQExpBuffer buf, const char *str);
5454
extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname);
5555
extern bool parsePGArray(const char *atext, char ***itemarray, int *nitems);
56-
extern bool buildACLCommands(const char *name, const char *subname,
56+
extern bool buildACLCommands(const char *name, const char *subname, const char *nspname,
5757
const char *type, const char *acls, const char *owner,
5858
const char *prefix, int remoteVersion,
5959
PQExpBuffer sql);
@@ -67,9 +67,9 @@ extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
6767
const char *schemavar, const char *namevar,
6868
const char *altnamevar, const char *visibilityrule);
6969
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
70-
uint32 objectId, PQExpBuffer sql);
70+
Oid objectId, PQExpBuffer sql);
7171
extern void emitShSecLabels(PGconn *conn, PGresult *res,
72-
PQExpBuffer buffer, const char *target, const char *objname);
72+
PQExpBuffer buffer, const char *objtype, const char *objname);
7373
extern void set_dump_section(const char *arg, int *dumpSections);
7474

7575
extern void simple_string_list_append(SimpleStringList *list, const char *val);

‎src/bin/pg_dump/pg_backup.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ struct Archive
8989
/* info needed for string escaping */
9090
int encoding; /* libpq code for client_encoding */
9191
bool std_strings; /* standard_conforming_strings */
92+
93+
/* other important stuff */
94+
char *searchpath; /* search_path to set during restore */
9295
char *use_role; /* Issue SET ROLE to this */
9396

9497
/* error handling */

0 commit comments

Comments
 (0)