The attached patch implements a new SEARCH clause for CREATE FUNCTION.
The SEARCH clause controls the search_path used when executing
functions that were created without a SET clause.
Background:
Controlling search_path is critical for the correctness and security of
functions. Right now, the author of a function without a SET clause has
little ability to control the function's behavior, because even basic
operators like "+" involve search_path. This is a big problem for, e.g.
functions used in expression indexes which are called by any user with
write privileges on the table.
Motivation:
I'd like to (eventually) get to safe-by-default behavior. In other
words, the simplest function declaration should be safe for the most
common use cases.
To get there, we need some way to explicitly specify the less common
cases. Right now there's no way for the function author to indicate
that a function intends to use the session's search path. We also need
an easier way to specify that the user wants a safe search_path ("SET
search_path = pg_catalog, pg_temp" is arcane).
And when we know more about the user's actual intent, then it will be
easier to either form a transition plan to push users into the safer
behavior, or at least warn strongly when the user is doing something
dangerous (i.e. using a function that depends on the session's search
path as part of an expression index).
Today, the only information we have about the user's intent is the
presence or absence of a "SET search_path" clause, which is not a
strong signal.
Proposal:
Add SEARCH { DEFAULT | SYSTEM | SESSION } clause to CREATE/ALTER
function.
* SEARCH DEFAULT is the same as no SEARCH clause at all, and ends up
stored in the catalog as prosearch='d'.
* SEARCH SYSTEM means that we switch to the safe search path of
"pg_catalog, pg_temp" when executing the function. Stored as
prosearch='y'.
* SEARCH SESSION means that we don't switch the search_path when
executing the function, and it's inherited from the session. Stored as
prosearch='e'.
Regardless of the SEARCH clause, a "SET search_path" clause will
override it. The SEARCH clause only matters when "SET search_path" is
not there.
Additionally provide a GUC, defaulting to false for compatibility, that
can interpret prosearch='d' as if it were prosearch='y'. It could help
provide a transition path. I know there's a strong reluctance to adding
these kinds of GUCs; I can remove it and I think the patch will still
be worthwhile. Perhaps there are alternatives that could help with
migration at pg_dump time instead?
Benefits:
1. The user can be more explicit about their actual intent. Do they
want safety and consistency? Or the flexibility of using the session's
search_path?
2. We can more accurately serve the user's intent. For instance, the
safe search_path of "pg_catalog, pg_temp" is arcane and seems to be
there just because we don't have a way to specify that pg_temp be
excluded entirely. But perhaps in the future we *do* want to exclude
pg_temp entirely. Knowing that the user just wants "SEARCH SYSTEM"
allows us some freedom to do that.
3. Users can be forward-compatible by specifying the functions that
really do need to use the session's search path as SEARCH SESSION, so
that they will never be broken in the future. That gives us a cleaner
path toward making the default behavior safe.
--
Jeff Davis
PostgreSQL Contributor Team - AWS
From 0218b472bd1ae8152b8bc8d92de4b7cc13dbbbad Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Fri, 11 Aug 2023 14:33:36 -0700
Subject: [PATCH] CREATE FUNCTION ... SEARCH { DEFAULT | SYSTEM | SESSION }.
Control the search_path while executing functions created without an
explicit SET clause.
Allows users to be forward compatible by explicitly declaring that a
function depends on the session's search_path, or that it only depends
on the safe search_path of "pg_catalog, pg_temp".
Also adds a GUC that allows users to choose how to treat functions
that have not specified a SEARCH clause or a SET clause. The GUC is
false by default to preserve existing semantics, but eventually it
will default to true for greater safety.
---
doc/src/sgml/config.sgml | 26 +++++++
doc/src/sgml/ref/alter_function.sgml | 15 ++++
doc/src/sgml/ref/create_function.sgml | 34 +++++++++
src/backend/catalog/pg_aggregate.c | 1 +
src/backend/catalog/pg_proc.c | 12 ++++
src/backend/commands/functioncmds.c | 44 +++++++++++-
src/backend/commands/typecmds.c | 4 ++
src/backend/optimizer/util/clauses.c | 1 +
src/backend/parser/gram.y | 15 ++++
src/backend/utils/fmgr/fmgr.c | 24 +++++--
src/backend/utils/misc/guc_tables.c | 10 +++
src/backend/utils/misc/postgresql.conf.sample | 2 +
src/include/catalog/namespace.h | 6 ++
src/include/catalog/pg_proc.h | 15 ++++
src/include/utils/guc.h | 1 +
.../regress/expected/create_function_sql.out | 69 +++++++++++++++++++
src/test/regress/sql/create_function_sql.sql | 35 ++++++++++
17 files changed, 305 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 11251fa05e..f366cc274c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10271,6 +10271,32 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
+ <varlistentry id="guc-safe-function-search-path" xreflabel="safe_function_search_path">
+ <term><varname>safe_function_search_path</varname> (<type>boolean</type>)
+ <indexterm>
+ <primary><varname>safe_function_search_path</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ If set to <literal>true</literal>, functions declared with
+ <literal>SEARCH DEFAULT</literal> (or no <literal>SEARCH</literal>
+ clause) behave as if <literal>SEARCH SYSTEM</literal> was
+ specified. This is the safest option because functions will have
+ consistent behavior regardless of the session's current setting of
+ <xref linkend="guc-search-path"/>.
+ </para>
+ <para>
+ If set to <literal>false</literal> (the default), functions declared
+ with <literal>SEARCH DEFAULT</literal> (or no
+ <literal>SEARCH</literal> clause) behave as if <literal>SEARCH
+ SESSION</literal> was specified.
+ </para>
+ <para>
+ See <xref linkend="sql-createfunction"/> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry id="guc-backslash-quote" xreflabel="backslash_quote">
<term><varname>backslash_quote</varname> (<type>enum</type>)
<indexterm><primary>strings</primary><secondary>backslash quotes</secondary></indexterm>
diff --git a/doc/src/sgml/ref/alter_function.sgml b/doc/src/sgml/ref/alter_function.sgml
index 8193b17f25..61d601b68f 100644
--- a/doc/src/sgml/ref/alter_function.sgml
+++ b/doc/src/sgml/ref/alter_function.sgml
@@ -37,6 +37,7 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT
IMMUTABLE | STABLE | VOLATILE
[ NOT ] LEAKPROOF
+ SEARCH { DEFAULT | SYSTEM | SESSION }
[ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER
PARALLEL { UNSAFE | RESTRICTED | SAFE }
COST <replaceable class="parameter">execution_cost</replaceable>
@@ -198,6 +199,20 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SEARCH DEFAULT</literal></term>
+ <term><literal>SEARCH SYSTEM</literal></term>
+ <term><literal>SEARCH SESSION</literal></term>
+
+ <listitem>
+ <para>
+ Change the <literal>SEARCH</literal> behavior of the function to the
+ specified setting. See <xref linkend="sql-createfunction"/> for
+ details.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal><optional> EXTERNAL </optional> SECURITY INVOKER</literal></term>
<term><literal><optional> EXTERNAL </optional> SECURITY DEFINER</literal></term>
diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index 863d99d1fc..82ca2955f4 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -31,6 +31,7 @@ CREATE [ OR REPLACE ] FUNCTION
| { IMMUTABLE | STABLE | VOLATILE }
| [ NOT ] LEAKPROOF
| { CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT }
+ | SEARCH { DEFAULT | SYSTEM | SESSION }
| { [ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER }
| PARALLEL { UNSAFE | RESTRICTED | SAFE }
| COST <replaceable class="parameter">execution_cost</replaceable>
@@ -402,6 +403,39 @@ CREATE [ OR REPLACE ] FUNCTION
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SEARCH DEFAULT</literal></term>
+ <term><literal>SEARCH SYSTEM</literal></term>
+ <term><literal>SEARCH SESSION</literal></term>
+
+ <listitem>
+ <para>
+ These settings control the function's <xref linkend="guc-search-path"/>
+ while executing if not set explicitly with a <literal>SET</literal>
+ clause in the <command>CREATE FUNCTION</command> command.
+ </para>
+ <para>
+ <literal>SEARCH SYSTEM</literal> indicates that the function will use
+ <literal>pg_catalog, pg_temp</literal>. This is the safest option
+ because the function will have consistent behavior regardless of the
+ session's setting.
+ </para>
+ <para>
+ <literal>SEARCH SESSION</literal> indicates that the function will
+ inherit session's current <literal>search_path</literal>
+ setting. Behavior of the function may change when the session's
+ <literal>search_path</literal> changes.
+ </para>
+ <para>
+ <literal>SEARCH DEFAULT</literal> is the same as not specifying any
+ <literal>SEARCH</literal> clause at all. If <xref
+ linkend="guc-safe-function-search-path"/> is set to
+ <literal>true</literal>, then the function will use
+ <literal>pg_catalog, pg_temp</literal>; otherwise it will inherit the
+ session's current <literal>search_path</literal> setting.
+ </para>
+ </listitem>
+ </varlistentry>
<varlistentry>
<term><literal><optional>EXTERNAL</optional> SECURITY INVOKER</literal></term>
<term><literal><optional>EXTERNAL</optional> SECURITY DEFINER</literal></term>
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index ebc4454743..e4ec68ec43 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -624,6 +624,7 @@ AggregateCreate(const char *aggName,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_AGGREGATE,
+ PROSEARCH_DEFAULT, /* no SEARCH clause for aggregates */
false, /* security invoker (currently not
* definable for agg) */
false, /* isLeakProof */
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b5fd364003..05917fd3df 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -81,6 +81,7 @@ ProcedureCreate(const char *procedureName,
const char *probin,
Node *prosqlbody,
char prokind,
+ char prosearch,
bool security_definer,
bool isLeakProof,
bool isStrict,
@@ -308,6 +309,7 @@ ProcedureCreate(const char *procedureName,
values[Anum_pg_proc_provariadic - 1] = ObjectIdGetDatum(variadicType);
values[Anum_pg_proc_prosupport - 1] = ObjectIdGetDatum(prosupport);
values[Anum_pg_proc_prokind - 1] = CharGetDatum(prokind);
+ values[Anum_pg_proc_prosearch - 1] = CharGetDatum(prosearch);
values[Anum_pg_proc_prosecdef - 1] = BoolGetDatum(security_definer);
values[Anum_pg_proc_proleakproof - 1] = BoolGetDatum(isLeakProof);
values[Anum_pg_proc_proisstrict - 1] = BoolGetDatum(isStrict);
@@ -988,6 +990,16 @@ sql_function_parse_error_callback(void *arg)
}
}
+/*
+ * Check whether the search_path must be set while executing this function.
+ */
+bool
+prosearch_is_system(char prosearch)
+{
+ return (prosearch == PROSEARCH_SYSTEM ||
+ (prosearch == PROSEARCH_DEFAULT && safe_function_search_path));
+}
+
/*
* Adjust a syntax error occurring inside the function body of a CREATE
* FUNCTION or DO command. This can be used by any function validator or
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 7ba6a86ebe..7c2dfda01e 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -507,6 +507,7 @@ compute_common_attribute(ParseState *pstate,
DefElem *defel,
DefElem **volatility_item,
DefElem **strict_item,
+ DefElem **search_item,
DefElem **security_item,
DefElem **leakproof_item,
List **set_items,
@@ -533,6 +534,13 @@ compute_common_attribute(ParseState *pstate,
*strict_item = defel;
}
+ else if (strcmp(defel->defname, "search") == 0)
+ {
+ if (*search_item)
+ errorConflictingDefElem(defel, pstate);
+
+ *search_item = defel;
+ }
else if (strcmp(defel->defname, "security") == 0)
{
if (*security_item)
@@ -603,6 +611,24 @@ procedure_error:
return false;
}
+static char
+interpret_func_search(DefElem *defel)
+{
+ char *str = strVal(defel->arg);
+
+ if (strcmp(str, "default") == 0)
+ return PROSEARCH_DEFAULT;
+ else if (strcmp(str, "system") == 0)
+ return PROSEARCH_SYSTEM;
+ else if (strcmp(str, "session") == 0)
+ return PROSEARCH_SESSION;
+ else
+ {
+ elog(ERROR, "invalid volatility \"%s\"", str);
+ return 0; /* keep compiler quiet */
+ }
+}
+
static char
interpret_func_volatility(DefElem *defel)
{
@@ -725,6 +751,7 @@ compute_function_attributes(ParseState *pstate,
bool *windowfunc_p,
char *volatility_p,
bool *strict_p,
+ char *search_p,
bool *security_definer,
bool *leakproof_p,
ArrayType **proconfig,
@@ -740,6 +767,7 @@ compute_function_attributes(ParseState *pstate,
DefElem *windowfunc_item = NULL;
DefElem *volatility_item = NULL;
DefElem *strict_item = NULL;
+ DefElem *search_item = NULL;
DefElem *security_item = NULL;
DefElem *leakproof_item = NULL;
List *set_items = NIL;
@@ -786,6 +814,7 @@ compute_function_attributes(ParseState *pstate,
defel,
&volatility_item,
&strict_item,
+ &search_item,
&security_item,
&leakproof_item,
&set_items,
@@ -814,6 +843,8 @@ compute_function_attributes(ParseState *pstate,
*volatility_p = interpret_func_volatility(volatility_item);
if (strict_item)
*strict_p = boolVal(strict_item->arg);
+ if (search_item)
+ *search_p = interpret_func_search(search_item);
if (security_item)
*security_definer = boolVal(security_item->arg);
if (leakproof_item)
@@ -1042,7 +1073,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
isStrict,
security,
isLeakProof;
- char volatility;
+ char prosearch,
+ volatility;
ArrayType *proconfig;
float4 procost;
float4 prorows;
@@ -1067,6 +1099,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
language = NULL;
isWindowFunc = false;
isStrict = false;
+ prosearch = PROSEARCH_DEFAULT;
security = false;
isLeakProof = false;
volatility = PROVOLATILE_VOLATILE;
@@ -1082,8 +1115,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
stmt->options,
&as_clause, &language, &transformDefElem,
&isWindowFunc, &volatility,
- &isStrict, &security, &isLeakProof,
- &proconfig, &procost, &prorows,
+ &isStrict, &prosearch, &security,
+ &isLeakProof, &proconfig, &procost, &prorows,
&prosupport, ¶llel);
if (!language)
@@ -1271,6 +1304,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
probin_str, /* converted to text later */
prosqlbody,
stmt->is_procedure ? PROKIND_PROCEDURE : (isWindowFunc ? PROKIND_WINDOW : PROKIND_FUNCTION),
+ prosearch,
security,
isLeakProof,
isStrict,
@@ -1355,6 +1389,7 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
ListCell *l;
DefElem *volatility_item = NULL;
DefElem *strict_item = NULL;
+ DefElem *search_item = NULL;
DefElem *security_def_item = NULL;
DefElem *leakproof_item = NULL;
List *set_items = NIL;
@@ -1399,6 +1434,7 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
defel,
&volatility_item,
&strict_item,
+ &search_item,
&security_def_item,
&leakproof_item,
&set_items,
@@ -1413,6 +1449,8 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
procForm->provolatile = interpret_func_volatility(volatility_item);
if (strict_item)
procForm->proisstrict = boolVal(strict_item->arg);
+ if (search_item)
+ procForm->prosearch = interpret_func_search(search_item);
if (security_def_item)
procForm->prosecdef = boolVal(security_def_item->arg);
if (leakproof_item)
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 5e97606793..baa8dd706e 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1763,6 +1763,7 @@ makeRangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
false, /* isStrict */
@@ -1828,6 +1829,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
true, /* isStrict */
@@ -1872,6 +1874,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
true, /* isStrict */
@@ -1910,6 +1913,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
true, /* isStrict */
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index da258968b8..ad93e0a28f 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4474,6 +4474,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
*/
if (funcform->prolang != SQLlanguageId ||
funcform->prokind != PROKIND_FUNCTION ||
+ prosearch_is_system(funcform->prosearch) ||
funcform->prosecdef ||
funcform->proretset ||
funcform->prorettype == RECORDOID ||
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 15ece871a0..e264fa536f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8528,6 +8528,21 @@ common_func_opt_item:
{
$$ = makeDefElem("support", (Node *) $2, @1);
}
+ | SEARCH DEFAULT
+ {
+ /* we abuse the normal content of a DefElem here */
+ $$ = makeDefElem("search", (Node *) makeString("default"), @1);
+ }
+ | SEARCH SYSTEM_P
+ {
+ /* we abuse the normal content of a DefElem here */
+ $$ = makeDefElem("search", (Node *) makeString("system"), @1);
+ }
+ | SEARCH SESSION
+ {
+ /* we abuse the normal content of a DefElem here */
+ $$ = makeDefElem("search", (Node *) makeString("session"), @1);
+ }
| FunctionSetResetClause
{
/* we abuse the normal content of a DefElem here */
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9dfdf890c5..2dc7224940 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -16,6 +16,7 @@
#include "postgres.h"
#include "access/detoast.h"
+#include "catalog/namespace.h"
#include "catalog/pg_language.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
@@ -110,6 +111,7 @@ fmgr_lookupByName(const char *name)
return NULL;
}
+
/*
* This routine fills a FmgrInfo struct, given the OID
* of the function to be called.
@@ -203,6 +205,7 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
*/
if (!ignore_security &&
(procedureStruct->prosecdef ||
+ prosearch_is_system(procedureStruct->prosearch) ||
!heap_attisnull(procedureTuple, Anum_pg_proc_proconfig, NULL) ||
FmgrHookIsNeeded(functionId)))
{
@@ -612,6 +615,7 @@ struct fmgr_security_definer_cache
{
FmgrInfo flinfo; /* lookup info for target function */
Oid userid; /* userid to set, or InvalidOid */
+ char *searchPath; /* from SEARCH clause, if specified */
List *configNames; /* GUC names to set, or NIL */
List *configValues; /* GUC values to set, or NIL */
Datum arg; /* passthrough argument for plugin modules */
@@ -630,6 +634,9 @@ struct fmgr_security_definer_cache
extern Datum
fmgr_security_definer(PG_FUNCTION_ARGS)
{
+ GucContext context = superuser() ? PGC_SUSET : PGC_USERSET;
+ GucSource source = PGC_S_SESSION;
+ GucAction action = GUC_ACTION_SAVE;
Datum result;
struct fmgr_security_definer_cache *volatile fcache;
FmgrInfo *save_flinfo;
@@ -662,6 +669,9 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
fcinfo->flinfo->fn_oid);
procedureStruct = (Form_pg_proc) GETSTRUCT(tuple);
+ if (prosearch_is_system(procedureStruct->prosearch))
+ fcache->searchPath = NAMESPACE_SAFE_SEARCH_PATH;
+
if (procedureStruct->prosecdef)
fcache->userid = procedureStruct->proowner;
@@ -687,20 +697,22 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
/* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */
GetUserIdAndSecContext(&save_userid, &save_sec_context);
- if (fcache->configNames != NIL) /* Need a new GUC nesting level */
+ if (fcache->searchPath != NULL || fcache->configNames != NIL) /* Need a new GUC nesting level */
save_nestlevel = NewGUCNestLevel();
else
- save_nestlevel = 0; /* keep compiler quiet */
+ save_nestlevel = 0;
if (OidIsValid(fcache->userid))
SetUserIdAndSecContext(fcache->userid,
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
+ if (fcache->searchPath != NULL)
+ (void) set_config_option("search_path", fcache->searchPath,
+ context, source,
+ action, true, 0, false);
+
forboth(lc1, fcache->configNames, lc2, fcache->configValues)
{
- GucContext context = superuser() ? PGC_SUSET : PGC_USERSET;
- GucSource source = PGC_S_SESSION;
- GucAction action = GUC_ACTION_SAVE;
char *name = lfirst(lc1);
char *value = lfirst(lc2);
@@ -749,7 +761,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
fcinfo->flinfo = save_flinfo;
- if (fcache->configNames != NIL)
+ if (save_nestlevel > 0)
AtEOXact_GUC(true, save_nestlevel);
if (OidIsValid(fcache->userid))
SetUserIdAndSecContext(save_userid, save_sec_context);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index f9dba43b8c..7b32235d52 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -504,6 +504,7 @@ bool log_btree_build_stats = false;
char *event_source;
bool row_security;
+bool safe_function_search_path = false;
bool check_function_bodies = true;
/*
@@ -1593,6 +1594,15 @@ struct config_bool ConfigureNamesBool[] =
true,
NULL, NULL, NULL
},
+ {
+ {"safe_function_search_path", PGC_SUSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Use safe search path for all functions, unless otherwise specified."),
+ NULL
+ },
+ &safe_function_search_path,
+ false,
+ NULL, NULL, NULL
+ },
{
{"check_function_bodies", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Check routine bodies during CREATE FUNCTION and CREATE PROCEDURE."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index c768af9a73..accbfb5609 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -688,6 +688,8 @@
#default_toast_compression = 'pglz' # 'pglz' or 'lz4'
#temp_tablespaces = '' # a list of tablespace names, '' uses
# only default tablespace
+#safe_function_search_path = off # change to safe search_path while
+ # executing functions
#check_function_bodies = on
#default_transaction_isolation = 'read committed'
#default_transaction_read_only = off
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index 49ef619e4b..5567c1268f 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -76,6 +76,12 @@ typedef enum RVROption
typedef void (*RangeVarGetRelidCallback) (const RangeVar *relation, Oid relId,
Oid oldRelId, void *callback_arg);
+/*
+ * Safe search path for cases where the session search path can't be trusted
+ * (e.g. for SECURITY DEFINER functions).
+ */
+#define NAMESPACE_SAFE_SEARCH_PATH "pg_catalog, pg_temp"
+
#define RangeVarGetRelid(relation, lockmode, missing_ok) \
RangeVarGetRelidExtended(relation, lockmode, \
(missing_ok) ? RVR_MISSING_OK : 0, NULL, NULL)
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index e7abe0b497..b9627ad493 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -58,6 +58,9 @@ CATALOG(pg_proc,1255,ProcedureRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(81,Proce
/* see PROKIND_ categories below */
char prokind BKI_DEFAULT(f);
+ /* security definer */
+ char prosearch BKI_DEFAULT(d);
+
/* security definer */
bool prosecdef BKI_DEFAULT(f);
@@ -150,6 +153,15 @@ DECLARE_UNIQUE_INDEX(pg_proc_proname_args_nsp_index, 2691, ProcedureNameArgsNspI
#define PROKIND_WINDOW 'w'
#define PROKIND_PROCEDURE 'p'
+/*
+ * Symbolic values for prosearch column: these determine the search_path under
+ * which a function executes if not specified in a SET clause
+ * (proconfig). PROSEARCH_DEFAULT means that no SEARCH clause was specified.
+ */
+#define PROSEARCH_DEFAULT 'd'
+#define PROSEARCH_SYSTEM 'y'
+#define PROSEARCH_SESSION 'e'
+
/*
* Symbolic values for provolatile column: these indicate whether the result
* of a function is dependent *only* on the values of its explicit arguments,
@@ -197,6 +209,7 @@ extern ObjectAddress ProcedureCreate(const char *procedureName,
const char *probin,
Node *prosqlbody,
char prokind,
+ char prosearch,
bool security_definer,
bool isLeakProof,
bool isStrict,
@@ -213,6 +226,8 @@ extern ObjectAddress ProcedureCreate(const char *procedureName,
float4 procost,
float4 prorows);
+extern bool prosearch_is_system(char prosearch);
+
extern bool function_parse_error_transpose(const char *prosrc);
extern List *oid_array_to_list(Datum datum);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e89083ee0e..1241d095ba 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -249,6 +249,7 @@ extern PGDLLIMPORT bool log_executor_stats;
extern PGDLLIMPORT bool log_statement_stats;
extern PGDLLIMPORT bool log_btree_build_stats;
+extern PGDLLIMPORT bool safe_function_search_path;
extern PGDLLIMPORT bool check_function_bodies;
extern PGDLLIMPORT bool current_role_is_superuser;
diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out
index 50aca5940f..72bbc02ee9 100644
--- a/src/test/regress/expected/create_function_sql.out
+++ b/src/test/regress/expected/create_function_sql.out
@@ -88,6 +88,75 @@ SELECT proname, provolatile FROM pg_proc
functest_b_4 | v
(4 rows)
+--
+-- SEARCH DEFAULT | SYSTEM | SESSION
+--
+CREATE FUNCTION show_search_path() RETURNS TEXT
+ LANGUAGE plpgsql AS
+$$
+ BEGIN
+ RETURN current_setting('search_path');
+ END;
+$$;
+SET safe_function_search_path = true;
+ALTER FUNCTION show_search_path() SEARCH DEFAULT;
+SELECT show_search_path();
+ show_search_path
+---------------------
+ pg_catalog, pg_temp
+(1 row)
+
+ALTER FUNCTION show_search_path() SEARCH SYSTEM;
+SELECT show_search_path();
+ show_search_path
+---------------------
+ pg_catalog, pg_temp
+(1 row)
+
+ALTER FUNCTION show_search_path() SEARCH SESSION;
+SELECT show_search_path();
+ show_search_path
+------------------------
+ temp_func_test, public
+(1 row)
+
+RESET safe_function_search_path;
+ALTER FUNCTION show_search_path() SEARCH DEFAULT;
+SELECT show_search_path();
+ show_search_path
+------------------------
+ temp_func_test, public
+(1 row)
+
+ALTER FUNCTION show_search_path() SEARCH SYSTEM;
+SELECT show_search_path();
+ show_search_path
+---------------------
+ pg_catalog, pg_temp
+(1 row)
+
+ALTER FUNCTION show_search_path() SEARCH SESSION;
+SELECT show_search_path();
+ show_search_path
+------------------------
+ temp_func_test, public
+(1 row)
+
+ALTER FUNCTION show_search_path() SEARCH SYSTEM SET search_path = test1;
+SELECT show_search_path();
+ show_search_path
+------------------
+ test1
+(1 row)
+
+ALTER FUNCTION show_search_path() SEARCH SESSION SET search_path = test1;
+SELECT show_search_path();
+ show_search_path
+------------------
+ test1
+(1 row)
+
+DROP FUNCTION show_search_path();
--
-- SECURITY DEFINER | INVOKER
--
diff --git a/src/test/regress/sql/create_function_sql.sql b/src/test/regress/sql/create_function_sql.sql
index 89e9af3a49..b0756e1668 100644
--- a/src/test/regress/sql/create_function_sql.sql
+++ b/src/test/regress/sql/create_function_sql.sql
@@ -60,6 +60,41 @@ SELECT proname, provolatile FROM pg_proc
'functest_B_3'::regproc,
'functest_B_4'::regproc) ORDER BY proname;
+--
+-- SEARCH DEFAULT | SYSTEM | SESSION
+--
+
+CREATE FUNCTION show_search_path() RETURNS TEXT
+ LANGUAGE plpgsql AS
+$$
+ BEGIN
+ RETURN current_setting('search_path');
+ END;
+$$;
+
+SET safe_function_search_path = true;
+ALTER FUNCTION show_search_path() SEARCH DEFAULT;
+SELECT show_search_path();
+ALTER FUNCTION show_search_path() SEARCH SYSTEM;
+SELECT show_search_path();
+ALTER FUNCTION show_search_path() SEARCH SESSION;
+SELECT show_search_path();
+
+RESET safe_function_search_path;
+ALTER FUNCTION show_search_path() SEARCH DEFAULT;
+SELECT show_search_path();
+ALTER FUNCTION show_search_path() SEARCH SYSTEM;
+SELECT show_search_path();
+ALTER FUNCTION show_search_path() SEARCH SESSION;
+SELECT show_search_path();
+
+ALTER FUNCTION show_search_path() SEARCH SYSTEM SET search_path = test1;
+SELECT show_search_path();
+ALTER FUNCTION show_search_path() SEARCH SESSION SET search_path = test1;
+SELECT show_search_path();
+
+DROP FUNCTION show_search_path();
+
--
-- SECURITY DEFINER | INVOKER
--
--
2.34.1
