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 files changed, 71 insertions, 29 deletions

diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index c1719780604..abd0579d5d6 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -34,6 +34,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
  *
  *	name: the object name, in the form to use in the commands (already quoted)
  *	subname: the sub-object name, if any (already quoted); NULL if none
+ *	nspname: the namespace the object is in (NULL if none); not pre-quoted
  *	type: the object type (as seen in GRANT command: must be one of
  *		TABLE, SEQUENCE, FUNCTION, LANGUAGE, SCHEMA, DATABASE, TABLESPACE,
  *		FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT)
@@ -54,7 +55,7 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
  * since this routine uses fmtId() internally.
  */
 bool
-buildACLCommands(const char *name, const char *subname,
+buildACLCommands(const char *name, const char *subname, const char *nspname,
 				 const char *type, const char *acls, const char *racls,
 				 const char *owner, const char *prefix, int remoteVersion,
 				 PQExpBuffer sql)
@@ -154,7 +155,10 @@ buildACLCommands(const char *name, const char *subname,
 		appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
 		if (subname)
 			appendPQExpBuffer(firstsql, "(%s)", subname);
-		appendPQExpBuffer(firstsql, " ON %s %s FROM PUBLIC;\n", type, name);
+		appendPQExpBuffer(firstsql, " ON %s ", type);
+		if (nspname && *nspname)
+			appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+		appendPQExpBuffer(firstsql, "%s FROM PUBLIC;\n", name);
 	}
 	else
 	{
@@ -172,8 +176,11 @@ buildACLCommands(const char *name, const char *subname,
 			{
 				if (privs->len > 0)
 				{
-					appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s %s FROM ",
-									  prefix, privs->data, type, name);
+					appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
+									  prefix, privs->data, type);
+					if (nspname && *nspname)
+						appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+					appendPQExpBuffer(firstsql, "%s FROM ", name);
 					if (grantee->len == 0)
 						appendPQExpBufferStr(firstsql, "PUBLIC;\n");
 					else if (strncmp(grantee->data, "group ",
@@ -187,8 +194,11 @@ buildACLCommands(const char *name, const char *subname,
 				if (privswgo->len > 0)
 				{
 					appendPQExpBuffer(firstsql,
-							   "%sREVOKE GRANT OPTION FOR %s ON %s %s FROM ",
-									  prefix, privswgo->data, type, name);
+									  "%sREVOKE GRANT OPTION FOR %s ON %s ",
+									  prefix, privswgo->data, type);
+					if (nspname && *nspname)
+						appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+					appendPQExpBuffer(firstsql, "%s FROM ", name);
 					if (grantee->len == 0)
 						appendPQExpBufferStr(firstsql, "PUBLIC");
 					else if (strncmp(grantee->data, "group ",
@@ -255,18 +265,33 @@ buildACLCommands(const char *name, const char *subname,
 					appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
 					if (subname)
 						appendPQExpBuffer(firstsql, "(%s)", subname);
-					appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
-									  type, name, fmtId(grantee->data));
+					appendPQExpBuffer(firstsql, " ON %s ", type);
+					if (nspname && *nspname)
+						appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+					appendPQExpBuffer(firstsql, "%s FROM %s;\n",
+									  name, fmtId(grantee->data));
 					if (privs->len > 0)
+					{
 						appendPQExpBuffer(firstsql,
-										  "%sGRANT %s ON %s %s TO %s;\n",
-										  prefix, privs->data, type, name,
-										  fmtId(grantee->data));
+										  "%sGRANT %s ON %s ",
+										  prefix, privs->data, type);
+						if (nspname && *nspname)
+							appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+						appendPQExpBuffer(firstsql,
+										  "%s TO %s;\n",
+										  name, fmtId(grantee->data));
+					}
 					if (privswgo->len > 0)
+					{
 						appendPQExpBuffer(firstsql,
-							"%sGRANT %s ON %s %s TO %s WITH GRANT OPTION;\n",
-										  prefix, privswgo->data, type, name,
-										  fmtId(grantee->data));
+										  "%sGRANT %s ON %s ",
+										  prefix, privswgo->data, type);
+						if (nspname && *nspname)
+							appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+						appendPQExpBuffer(firstsql,
+										  "%s TO %s WITH GRANT OPTION;\n",
+										  name, fmtId(grantee->data));
+					}
 				}
 			}
 			else
@@ -288,8 +313,11 @@ buildACLCommands(const char *name, const char *subname,
 
 				if (privs->len > 0)
 				{
-					appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
-									  prefix, privs->data, type, name);
+					appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
+									  prefix, privs->data, type);
+					if (nspname && *nspname)
+						appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
+					appendPQExpBuffer(secondsql, "%s TO ", name);
 					if (grantee->len == 0)
 						appendPQExpBufferStr(secondsql, "PUBLIC;\n");
 					else if (strncmp(grantee->data, "group ",
@@ -301,8 +329,11 @@ buildACLCommands(const char *name, const char *subname,
 				}
 				if (privswgo->len > 0)
 				{
-					appendPQExpBuffer(secondsql, "%sGRANT %s ON %s %s TO ",
-									  prefix, privswgo->data, type, name);
+					appendPQExpBuffer(secondsql, "%sGRANT %s ON %s ",
+									  prefix, privswgo->data, type);
+					if (nspname && *nspname)
+						appendPQExpBuffer(secondsql, "%s.", fmtId(nspname));
+					appendPQExpBuffer(secondsql, "%s TO ", name);
 					if (grantee->len == 0)
 						appendPQExpBufferStr(secondsql, "PUBLIC");
 					else if (strncmp(grantee->data, "group ",
@@ -332,8 +363,11 @@ buildACLCommands(const char *name, const char *subname,
 		appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix);
 		if (subname)
 			appendPQExpBuffer(firstsql, "(%s)", subname);
-		appendPQExpBuffer(firstsql, " ON %s %s FROM %s;\n",
-						  type, name, fmtId(owner));
+		appendPQExpBuffer(firstsql, " ON %s ", type);
+		if (nspname && *nspname)
+			appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
+		appendPQExpBuffer(firstsql, "%s FROM %s;\n",
+						  name, fmtId(owner));
 	}
 
 	destroyPQExpBuffer(grantee);
@@ -392,7 +426,8 @@ buildDefaultACLCommands(const char *type, const char *nspname,
 	if (strlen(initacls) != 0 || strlen(initracls) != 0)
 	{
 		appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
-		if (!buildACLCommands("", NULL, type, initacls, initracls, owner,
+		if (!buildACLCommands("", NULL, NULL, type,
+							  initacls, initracls, owner,
 							  prefix->data, remoteVersion, sql))
 		{
 			destroyPQExpBuffer(prefix);
@@ -401,7 +436,8 @@ buildDefaultACLCommands(const char *type, const char *nspname,
 		appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
 	}
 
-	if (!buildACLCommands("", NULL, type, acls, racls, owner,
+	if (!buildACLCommands("", NULL, NULL, type,
+						  acls, racls, owner,
 						  prefix->data, remoteVersion, sql))
 	{
 		destroyPQExpBuffer(prefix);
@@ -645,26 +681,32 @@ AddAcl(PQExpBuffer aclbuf, const char *keyword, const char *subname)
  * buildShSecLabelQuery
  *
  * Build a query to retrieve security labels for a shared object.
+ * The object is identified by its OID plus the name of the catalog
+ * it can be found in (e.g., "pg_database" for database names).
+ * The query is appended to "sql".  (We don't execute it here so as to
+ * keep this file free of assumptions about how to deal with SQL errors.)
  */
 void
-buildShSecLabelQuery(PGconn *conn, const char *catalog_name, uint32 objectId,
+buildShSecLabelQuery(PGconn *conn, const char *catalog_name, Oid objectId,
 					 PQExpBuffer sql)
 {
 	appendPQExpBuffer(sql,
 					  "SELECT provider, label FROM pg_catalog.pg_shseclabel "
-					  "WHERE classoid = '%s'::pg_catalog.regclass AND "
-					  "objoid = %u", catalog_name, objectId);
+					  "WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
+					  "AND objoid = '%u'", catalog_name, objectId);
 }
 
 /*
  * emitShSecLabels
  *
- * Format security label data retrieved by the query generated in
- * buildShSecLabelQuery.
+ * Construct SECURITY LABEL commands using the data retrieved by the query
+ * generated by buildShSecLabelQuery, and append them to "buffer".
+ * Here, the target object is identified by its type name (e.g. "DATABASE")
+ * and its name (not pre-quoted).
  */
 void
 emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
-				const char *target, const char *objname)
+				const char *objtype, const char *objname)
 {
 	int			i;
 
@@ -676,7 +718,7 @@ emitShSecLabels(PGconn *conn, PGresult *res, PQExpBuffer buffer,
 		/* must use fmtId result before calling it again */
 		appendPQExpBuffer(buffer,
 						  "SECURITY LABEL FOR %s ON %s",
-						  fmtId(provider), target);
+						  fmtId(provider), objtype);
 		appendPQExpBuffer(buffer,
 						  " %s IS ",
 						  fmtId(objname));