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

Commit a8fc37a

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 7dd49bd commit a8fc37a

File tree

12 files changed

+882
-1126
lines changed

12 files changed

+882
-1126
lines changed

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

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

8282
/* Pretty flags */
83-
#define PRETTYFLAG_PAREN 1
84-
#define PRETTYFLAG_INDENT 2
83+
#define PRETTYFLAG_PAREN 0x0001
84+
#define PRETTYFLAG_INDENT 0x0002
85+
#define PRETTYFLAG_SCHEMA 0x0004
8586

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

89-
/* macro to test if pretty action needed */
90+
/* macros to test if pretty action needed */
9091
#define PRETTY_PAREN(context) ((context)->prettyFlags & PRETTYFLAG_PAREN)
9192
#define PRETTY_INDENT(context) ((context)->prettyFlags & PRETTYFLAG_INDENT)
93+
#define PRETTY_SCHEMA(context) ((context)->prettyFlags & PRETTYFLAG_SCHEMA)
9294

9395

9496
/* ----------
@@ -472,7 +474,8 @@ pg_get_ruledef_ext(PG_FUNCTION_ARGS)
472474
bool pretty = PG_GETARG_BOOL(1);
473475
int prettyFlags;
474476

475-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
477+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
478+
476479
PG_RETURN_TEXT_P(string_to_text(pg_get_ruledef_worker(ruleoid, prettyFlags)));
477480
}
478481

@@ -571,7 +574,8 @@ pg_get_viewdef_ext(PG_FUNCTION_ARGS)
571574
bool pretty = PG_GETARG_BOOL(1);
572575
int prettyFlags;
573576

574-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
577+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
578+
575579
PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT)));
576580
}
577581

@@ -584,7 +588,8 @@ pg_get_viewdef_wrap(PG_FUNCTION_ARGS)
584588
int prettyFlags;
585589

586590
/* calling this implies we want pretty printing */
587-
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT;
591+
prettyFlags = PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA;
592+
588593
PG_RETURN_TEXT_P(string_to_text(pg_get_viewdef_worker(viewoid, prettyFlags, wrap)));
589594
}
590595

@@ -617,7 +622,7 @@ pg_get_viewdef_name_ext(PG_FUNCTION_ARGS)
617622
RangeVar *viewrel;
618623
Oid viewoid;
619624

620-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
625+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
621626

622627
/* Look up view name. Can't lock it - we might not have privileges. */
623628
viewrel = makeRangeVarFromNameList(textToQualifiedNameList(viewname));
@@ -822,8 +827,15 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
822827
appendStringInfoString(&buf, " TRUNCATE");
823828
findx++;
824829
}
830+
831+
/*
832+
* In non-pretty mode, always schema-qualify the target table name for
833+
* safety. In pretty mode, schema-qualify only if not visible.
834+
*/
825835
appendStringInfo(&buf, " ON %s ",
826-
generate_relation_name(trigrec->tgrelid, NIL));
836+
pretty ?
837+
generate_relation_name(trigrec->tgrelid, NIL) :
838+
generate_qualified_relation_name(trigrec->tgrelid));
827839

828840
if (OidIsValid(trigrec->tgconstraint))
829841
{
@@ -896,7 +908,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty)
896908
context.windowClause = NIL;
897909
context.windowTList = NIL;
898910
context.varprefix = true;
899-
context.prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
911+
context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
900912
context.wrapColumn = WRAP_COLUMN_DEFAULT;
901913
context.indentLevel = PRETTYINDENT_STD;
902914
context.special_exprkind = EXPR_KIND_NONE;
@@ -977,7 +989,8 @@ pg_get_indexdef_ext(PG_FUNCTION_ARGS)
977989
bool pretty = PG_GETARG_BOOL(2);
978990
int prettyFlags;
979991

980-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
992+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
993+
981994
PG_RETURN_TEXT_P(string_to_text(pg_get_indexdef_worker(indexrelid, colno,
982995
NULL,
983996
colno != 0,
@@ -1002,7 +1015,8 @@ pg_get_indexdef_columns(Oid indexrelid, bool pretty)
10021015
{
10031016
int prettyFlags;
10041017

1005-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1018+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1019+
10061020
return pg_get_indexdef_worker(indexrelid, 0, NULL, true, false, prettyFlags);
10071021
}
10081022

@@ -1123,7 +1137,9 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
11231137
appendStringInfo(&buf, "CREATE %sINDEX %s ON %s USING %s (",
11241138
idxrec->indisunique ? "UNIQUE " : "",
11251139
quote_identifier(NameStr(idxrelrec->relname)),
1126-
generate_relation_name(indrelid, NIL),
1140+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
1141+
generate_relation_name(indrelid, NIL) :
1142+
generate_qualified_relation_name(indrelid),
11271143
quote_identifier(NameStr(amrec->amname)));
11281144
else /* currently, must be EXCLUDE constraint */
11291145
appendStringInfo(&buf, "EXCLUDE USING %s (",
@@ -1315,7 +1331,8 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
13151331
bool pretty = PG_GETARG_BOOL(1);
13161332
int prettyFlags;
13171333

1318-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1334+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
1335+
13191336
PG_RETURN_TEXT_P(string_to_text(pg_get_constraintdef_worker(constraintId,
13201337
false,
13211338
prettyFlags)));
@@ -1744,7 +1761,7 @@ pg_get_expr_ext(PG_FUNCTION_ARGS)
17441761
int prettyFlags;
17451762
char *relname;
17461763

1747-
prettyFlags = pretty ? PRETTYFLAG_PAREN | PRETTYFLAG_INDENT : PRETTYFLAG_INDENT;
1764+
prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT;
17481765

17491766
if (OidIsValid(relid))
17501767
{
@@ -4161,7 +4178,10 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
41614178
}
41624179

41634180
/* The relation the rule is fired on */
4164-
appendStringInfo(buf, " TO %s", generate_relation_name(ev_class, NIL));
4181+
appendStringInfo(buf, " TO %s",
4182+
(prettyFlags & PRETTYFLAG_SCHEMA) ?
4183+
generate_relation_name(ev_class, NIL) :
4184+
generate_qualified_relation_name(ev_class));
41654185

41664186
/* If the rule has an event qualification, add it */
41674187
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
@@ -87,7 +87,7 @@ extern void appendShellString(PQExpBuffer buf, const char *str);
8787
extern void appendConnStrVal(PQExpBuffer buf, const char *str);
8888
extern void appendPsqlMetaConnect(PQExpBuffer buf, const char *dbname);
8989
extern bool parsePGArray(const char *atext, char ***itemarray, int *nitems);
90-
extern bool buildACLCommands(const char *name, const char *subname,
90+
extern bool buildACLCommands(const char *name, const char *subname, const char *nspname,
9191
const char *type, const char *acls, const char *owner,
9292
const char *prefix, int remoteVersion,
9393
PQExpBuffer sql);
@@ -101,9 +101,9 @@ extern bool processSQLNamePattern(PGconn *conn, PQExpBuffer buf,
101101
const char *schemavar, const char *namevar,
102102
const char *altnamevar, const char *visibilityrule);
103103
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
104-
uint32 objectId, PQExpBuffer sql);
104+
Oid objectId, PQExpBuffer sql);
105105
extern void emitShSecLabels(PGconn *conn, PGresult *res,
106-
PQExpBuffer buffer, const char *target, const char *objname);
106+
PQExpBuffer buffer, const char *objtype, const char *objname);
107107
extern void set_dump_section(const char *arg, int *dumpSections);
108108

109109
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
@@ -185,6 +185,9 @@ typedef struct Archive
185185
/* info needed for string escaping */
186186
int encoding; /* libpq code for client_encoding */
187187
bool std_strings; /* standard_conforming_strings */
188+
189+
/* other important stuff */
190+
char *searchpath; /* search_path to set during restore */
188191
char *use_role; /* Issue SET ROLE to this */
189192

190193
/* error handling */

0 commit comments

Comments
 (0)
0