The attached patch adds some special names to prevent pg_temp and/or
pg_catalog from being included implicitly.
This is a useful safety feature for functions that don't have any need
to search pg_temp.
The current (v16) recommendation is to include pg_temp last, which does
add to the safety, but it's confusing to *include* a namespace when
your intention is actually to *exclude* it, and it's also not
completely excluding pg_temp.
Although the syntax in the attached patch is not much friendlier, at
least it's clear that the intent is to exclude pg_temp. Furthermore, it
will be friendlier if we adopt the SEARCH SYSTEM syntax proposed in
another thread[1].
Additionally, this patch adds a WARNING when creating a schema that
uses one of these special names. Previously, there was no warning when
creating a schema with the name "$user", which could cause confusion.
[1]
https://www.postgresql.org/message-id/flat/2710f56add351a1ed553efb677408e51b060e67c.ca...@j-davis.com
--
Jeff Davis
PostgreSQL Contributor Team - AWS
From 79cb94b857f51dec5cbc24b3d46d2e58de92b5c0 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Fri, 18 Aug 2023 14:21:16 -0700
Subject: [PATCH] Special names "!pg_temp" and "!pg_catalog" for search_path.
These names act as markers preventing the implicit namespaces from
being added to the search path.
Functions often don't need to look in pg_temp, and in those cases it's
safer to exclude pg_temp entirely.
---
doc/src/sgml/config.sgml | 25 ++++++++++--------
doc/src/sgml/ref/create_function.sgml | 8 +++---
src/backend/catalog/namespace.c | 23 ++++++++++++----
src/backend/commands/schemacmds.c | 13 +++++++++
src/test/regress/expected/namespace.out | 35 +++++++++++++++++++++++++
src/test/regress/sql/namespace.sql | 18 +++++++++++++
6 files changed, 101 insertions(+), 21 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 11251fa05e..451f9c7e94 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8631,10 +8631,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
<para>
The system catalog schema, <literal>pg_catalog</literal>, is always
- searched, whether it is mentioned in the path or not. If it is
- mentioned in the path then it will be searched in the specified
- order. If <literal>pg_catalog</literal> is not in the path then it will
- be searched <emphasis>before</emphasis> searching any of the path items.
+ searched, unless the special schema name
+ <literal>!pg_catalog</literal> is specified in the path. If it is
+ mentioned in the path then it will be searched in the specified order.
+ If <literal>pg_catalog</literal> is not in the path then it will be
+ searched <emphasis>before</emphasis> searching any of the path items.
</para>
<!-- To further split hairs, funcname('foo') does not use the temporary
@@ -8643,13 +8644,15 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
func_name grammar production". -->
<para>
Likewise, the current session's temporary-table schema,
- <literal>pg_temp_<replaceable>nnn</replaceable></literal>, is always searched if it
- exists. It can be explicitly listed in the path by using the
- alias <literal>pg_temp</literal><indexterm><primary>pg_temp</primary></indexterm>. If it is not listed in the path then
- it is searched first (even before <literal>pg_catalog</literal>). However,
- the temporary schema is only searched for relation (table, view,
- sequence, etc.) and data type names. It is never searched for
- function or operator names.
+ <literal>pg_temp_<replaceable>nnn</replaceable></literal>, is always
+ searched if it exists, unless the special schema name
+ <literal>!pg_temp</literal> is specified. It can be explicitly
+ listed in the path by using the alias
+ <literal>pg_temp</literal><indexterm><primary>pg_temp</primary></indexterm>.
+ If it is not listed in the path then it is searched first (even before
+ <literal>pg_catalog</literal>). However, the temporary schema is only
+ searched for relation (table, view, sequence, etc.) and data type
+ names. It is never searched for function or operator names.
</para>
<para>
diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index 863d99d1fc..93aebdeb26 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -794,9 +794,8 @@ SELECT * FROM dup(42);
Particularly important in this regard is the
temporary-table schema, which is searched first by default, and
is normally writable by anyone. A secure arrangement can be obtained
- by forcing the temporary schema to be searched last. To do this,
- write <literal>pg_temp</literal><indexterm><primary>pg_temp</primary><secondary>securing functions</secondary></indexterm> as the last entry in <varname>search_path</varname>.
- This function illustrates safe usage:
+ by specifying <literal>!pg_temp</literal>. This function illustrates
+ safe usage:
<programlisting>
CREATE FUNCTION check_password(uname TEXT, pass TEXT)
@@ -811,8 +810,7 @@ BEGIN
END;
$$ LANGUAGE plpgsql
SECURITY DEFINER
- -- Set a secure search_path: trusted schema(s), then 'pg_temp'.
- SET search_path = admin, pg_temp;
+ SET search_path = admin, "!pg_temp";
</programlisting>
This function's intention is to access a table <literal>admin.pwds</literal>.
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 0c679fbf94..6d8a9cad02 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3645,6 +3645,8 @@ recomputeNamespacePath(void)
List *newpath;
ListCell *l;
bool temp_missing;
+ bool implicit_pg_temp = true;
+ bool implicit_pg_catalog = true;
Oid firstNS;
bool pathChanged;
MemoryContext oldcxt;
@@ -3714,6 +3716,14 @@ recomputeNamespacePath(void)
temp_missing = true;
}
}
+ else if (strcmp(curname, "!pg_temp") == 0)
+ {
+ implicit_pg_temp = false;
+ }
+ else if (strcmp(curname, "!pg_catalog") == 0)
+ {
+ implicit_pg_catalog = false;
+ }
else
{
/* normal namespace reference */
@@ -3738,14 +3748,17 @@ recomputeNamespacePath(void)
firstNS = linitial_oid(oidlist);
/*
- * Add any implicitly-searched namespaces to the list. Note these go on
- * the front, not the back; also notice that we do not check USAGE
- * permissions for these.
+ * Add any implicitly-searched namespaces to the list unless the markers
+ * "!pg_catalog" or "!pg_temp" are present. Note these go on the front,
+ * not the back; also notice that we do not check USAGE permissions for
+ * these.
*/
- if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
+ if (implicit_pg_catalog &&
+ !list_member_oid(oidlist, PG_CATALOG_NAMESPACE))
oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist);
- if (OidIsValid(myTempNamespace) &&
+ if (implicit_pg_temp &&
+ OidIsValid(myTempNamespace) &&
!list_member_oid(oidlist, myTempNamespace))
oidlist = lcons_oid(myTempNamespace, oidlist);
diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c
index 6eb3dc6bab..ca4cc9c472 100644
--- a/src/backend/commands/schemacmds.c
+++ b/src/backend/commands/schemacmds.c
@@ -109,6 +109,19 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString,
errmsg("unacceptable schema name \"%s\"", schemaName),
errdetail("The prefix \"pg_\" is reserved for system schemas.")));
+ /*
+ * These names have special meaning for search_path. Emit only a WARNING
+ * (rather than ERROR) to avoid compatibility problems.
+ */
+ if (strcmp(schemaName, "$user") == 0 ||
+ strncmp(schemaName, "!pg_", 4) == 0)
+ {
+ ereport(WARNING,
+ (errcode(ERRCODE_RESERVED_NAME),
+ errmsg("schema name \"%s\" should not be used", schemaName),
+ errdetail("The name \"$user\" and prefix \"!pg_\" have special meaning for search_path.")));
+ }
+
/*
* If if_not_exists was given and the schema already exists, bail out.
* (Note: we needn't check this when not if_not_exists, because
diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.out
index a62fd8ded0..1cd2c64786 100644
--- a/src/test/regress/expected/namespace.out
+++ b/src/test/regress/expected/namespace.out
@@ -114,3 +114,38 @@ SELECT COUNT(*) FROM pg_class WHERE relnamespace =
0
(1 row)
+-- test special names
+CREATE SCHEMA "$user";
+WARNING: schema name "$user" should not be used
+DETAIL: The name "$user" and prefix "!pg_" have special meaning for search_path.
+CREATE SCHEMA "!pg_foo";
+WARNING: schema name "!pg_foo" should not be used
+DETAIL: The name "$user" and prefix "!pg_" have special meaning for search_path.
+-- test "!pg_temp"
+CREATE TEMPORARY TABLE tmp(i INT);
+SELECT * FROM tmp;
+ i
+---
+(0 rows)
+
+SET search_path = public, "!pg_temp";
+SELECT * FROM tmp;
+ERROR: relation "tmp" does not exist
+LINE 1: SELECT * FROM tmp;
+ ^
+RESET search_path;
+DROP TABLE tmp;
+-- test "!pg_catalog"
+SELECT 1+1;
+ ?column?
+----------
+ 2
+(1 row)
+
+SET search_path = public, "!pg_catalog";
+SELECT 1+1;
+ERROR: operator does not exist: integer + integer
+LINE 1: SELECT 1+1;
+ ^
+HINT: No operator matches the given name and argument types. You might need to add explicit type casts.
+RESET search_path;
diff --git a/src/test/regress/sql/namespace.sql b/src/test/regress/sql/namespace.sql
index 3474f5ecf4..77ce7c4c31 100644
--- a/src/test/regress/sql/namespace.sql
+++ b/src/test/regress/sql/namespace.sql
@@ -66,3 +66,21 @@ DROP SCHEMA test_ns_schema_renamed CASCADE;
-- verify that the objects were dropped
SELECT COUNT(*) FROM pg_class WHERE relnamespace =
(SELECT oid FROM pg_namespace WHERE nspname = 'test_ns_schema_renamed');
+
+-- test special names
+CREATE SCHEMA "$user";
+CREATE SCHEMA "!pg_foo";
+
+-- test "!pg_temp"
+CREATE TEMPORARY TABLE tmp(i INT);
+SELECT * FROM tmp;
+SET search_path = public, "!pg_temp";
+SELECT * FROM tmp;
+RESET search_path;
+DROP TABLE tmp;
+
+-- test "!pg_catalog"
+SELECT 1+1;
+SET search_path = public, "!pg_catalog";
+SELECT 1+1;
+RESET search_path;
--
2.34.1
