Re: Request for assistance to backport CVE-2022-1552 fixes to 9.6 and 9.4

Hello pgsql-hackers,

Is there anyone willing to review the patches that I prepared?  I'd have
substatntially more confidence in the patches with a review from an
upstream developer who is familiar with the code.

Regards,

-Roberto

On Mon, Jul 04, 2022 at 06:06:58PM -0400, Roberto C. Sánchez wrote:
> On Wed, Jun 08, 2022 at 05:31:22PM -0400, Roberto C. Sánchez wrote:
> > On Wed, Jun 08, 2022 at 04:15:47PM -0400, Tom Lane wrote:
> > > Roberto =?iso-8859-1?Q?C=2E_S=E1nchez?= <roberto@debian.org> writes:
> > > > I am investigating backporting the fixes for CVE-2022-1552 to 9.6 and
> > > > 9.4 as part of Debian LTS and Extended LTS.  I am aware that these
> > > > releases are no longer supported upstream, but I have made an attempt at
> > > > adapting commits ef792f7856dea2576dcd9cab92b2b05fe955696b and
> > > > f26d5702857a9c027f84850af48b0eea0f3aa15c from the REL_10_STABLE branch.
> > > > I would appreciate a review of the attached patches and any comments on
> > > > things that may have been missed and/or adapted improperly.
> > > 
> > > FWIW, I would not recommend being in a huge hurry to back-port those
> > > changes, pending the outcome of this discussion:
> > > 
> > > https://www.postgresql.org/message-id/flat/f8a4105f076544c180a87ef0c4822352%40stmuk.bayern.de
> > > 
> > Thanks for the pointer.
> > 
> > > We're going to have to tweak that code somehow, and it's not yet
> > > entirely clear how.
> > > 
> > I will monitor the discussion to see what comes of it.
> > 
> Based on the discussion in the other thread, I have made an attempt to
> backport commit 88b39e61486a8925a3861d50c306a51eaa1af8d6 to 9.6 and 9.4.
> The only significant change that I had to make was to add the full
> function signatures for the REVOKE/GRANT in the citext test.
> 
> One question that I had about the change as committed is whether a
> REVOKE is needed on s.citext_ne, like so:
> 
> REVOKE ALL ON FUNCTION s.citext_ne FROM PUBLIC;
> 
> Or (for pre-10):
> 
> REVOKE ALL ON FUNCTION s.citext_ne(s.citext, s.citext) FROM PUBLIC;
> 
> I ask because the comment immediately preceding the sequence of REVOKEs
> includes the comment "Revoke all conceivably-relevant ACLs within the
> extension."  I am not especially knowledgable about deep internals, but
> that function seems like it would belong in the same group with the
> others.
> 
> In any event, would someone be willing to review the attached patches
> for correctness?  I would like to shortly publish updates to 9.6 and 9.4
> in Debian and a review would be most appreciated.
> 
> Regards,
> 
> -Roberto
> 
> -- 
> Roberto C. Sánchez

> From ef792f7856dea2576dcd9cab92b2b05fe955696b Mon Sep 17 00:00:00 2001
> From: Noah Misch <noah@leadboat.com>
> Date: Mon, 9 May 2022 08:35:08 -0700
> Subject: [PATCH] Make relation-enumerating operations be security-restricted
>  operations.
> 
> When a feature enumerates relations and runs functions associated with
> all found relations, the feature's user shall not need to trust every
> user having permission to create objects.  BRIN-specific functionality
> in autovacuum neglected to account for this, as did pg_amcheck and
> CLUSTER.  An attacker having permission to create non-temp objects in at
> least one schema could execute arbitrary SQL functions under the
> identity of the bootstrap superuser.  CREATE INDEX (not a
> relation-enumerating operation) and REINDEX protected themselves too
> late.  This change extends to the non-enumerating amcheck interface.
> Back-patch to v10 (all supported versions).
> 
> Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin.
> Reported by Alexander Lakhin.
> 
> Security: CVE-2022-1552
> ---
>  src/backend/access/brin/brin.c           |   30 ++++++++++++++++-
>  src/backend/catalog/index.c              |   41 +++++++++++++++++------
>  src/backend/commands/cluster.c           |   35 ++++++++++++++++----
>  src/backend/commands/indexcmds.c         |   53 +++++++++++++++++++++++++++++--
>  src/backend/utils/init/miscinit.c        |   24 ++++++++------
>  src/test/regress/expected/privileges.out |   42 ++++++++++++++++++++++++
>  src/test/regress/sql/privileges.sql      |   36 +++++++++++++++++++++
>  7 files changed, 231 insertions(+), 30 deletions(-)
> 
> --- a/src/backend/access/brin/brin.c
> +++ b/src/backend/access/brin/brin.c
> @@ -28,6 +28,7 @@
>  #include "pgstat.h"
>  #include "storage/bufmgr.h"
>  #include "storage/freespace.h"
> +#include "utils/guc.h"
>  #include "utils/index_selfuncs.h"
>  #include "utils/memutils.h"
>  #include "utils/rel.h"
> @@ -786,6 +787,9 @@
>      Oid            heapoid;
>      Relation    indexRel;
>      Relation    heapRel;
> +    Oid            save_userid;
> +    int            save_sec_context;
> +    int            save_nestlevel;
>      double        numSummarized = 0;
>  
>      if (RecoveryInProgress())
> @@ -799,10 +803,28 @@
>       * passed indexoid isn't an index then IndexGetRelation() will fail.
>       * Rather than emitting a not-very-helpful error message, postpone
>       * complaining, expecting that the is-it-an-index test below will fail.
> +     *
> +     * unlike brin_summarize_range(), autovacuum never calls this.  hence, we
> +     * don't switch userid.
>       */
>      heapoid = IndexGetRelation(indexoid, true);
>      if (OidIsValid(heapoid))
> +    {
>          heapRel = heap_open(heapoid, ShareUpdateExclusiveLock);
> +
> +        /*
> +         * Autovacuum calls us.  For its benefit, switch to the table owner's
> +         * userid, so that any index functions are run as that user.  Also
> +         * lock down security-restricted operations and arrange to make GUC
> +         * variable changes local to this command.  This is harmless, albeit
> +         * unnecessary, when called from SQL, because we fail shortly if the
> +         * user does not own the index.
> +         */
> +        GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +        SetUserIdAndSecContext(heapRel->rd_rel->relowner,
> +                               save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +        save_nestlevel = NewGUCNestLevel();
> +    }
>      else
>          heapRel = NULL;
>  
> @@ -817,7 +839,7 @@
>                          RelationGetRelationName(indexRel))));
>  
>      /* User must own the index (comparable to privileges needed for VACUUM) */
> -    if (!pg_class_ownercheck(indexoid, GetUserId()))
> +    if (heapRel != NULL && !pg_class_ownercheck(indexoid, save_userid))
>          aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
>                         RelationGetRelationName(indexRel));
>  
> @@ -835,6 +857,12 @@
>      /* OK, do it */
>      brinsummarize(indexRel, heapRel, &numSummarized, NULL);
>  
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(save_userid, save_sec_context);
> +
>      relation_close(indexRel, ShareUpdateExclusiveLock);
>      relation_close(heapRel, ShareUpdateExclusiveLock);
>  
> --- a/src/backend/catalog/index.c
> +++ b/src/backend/catalog/index.c
> @@ -2908,7 +2908,17 @@
>  
>      /* Open and lock the parent heap relation */
>      heapRelation = heap_open(heapId, ShareUpdateExclusiveLock);
> -    /* And the target index relation */
> +
> +    /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
> +
>      indexRelation = index_open(indexId, RowExclusiveLock);
>  
>      /*
> @@ -2922,16 +2932,6 @@
>      indexInfo->ii_Concurrent = true;
>  
>      /*
> -     * Switch to the table owner's userid, so that any index functions are run
> -     * as that user.  Also lock down security-restricted operations and
> -     * arrange to make GUC variable changes local to this command.
> -     */
> -    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> -    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
> -                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> -    save_nestlevel = NewGUCNestLevel();
> -
> -    /*
>       * Scan the index and gather up all the TIDs into a tuplesort object.
>       */
>      ivinfo.index = indexRelation;
> @@ -3395,6 +3395,9 @@
>      Relation    iRel,
>                  heapRelation;
>      Oid            heapId;
> +    Oid            save_userid;
> +    int            save_sec_context;
> +    int            save_nestlevel;
>      IndexInfo  *indexInfo;
>      volatile bool skipped_constraint = false;
>      PGRUsage    ru0;
> @@ -3409,6 +3412,16 @@
>      heapRelation = heap_open(heapId, ShareLock);
>  
>      /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
> +
> +    /*
>       * Open the target index relation and get an exclusive lock on it, to
>       * ensure that no one else is touching this particular index.
>       */
> @@ -3550,6 +3563,12 @@
>                   errdetail_internal("%s",
>                             pg_rusage_show(&ru0))));
>  
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(save_userid, save_sec_context);
> +
>      /* Close rels, but keep locks */
>      index_close(iRel, NoLock);
>      heap_close(heapRelation, NoLock);
> --- a/src/backend/commands/cluster.c
> +++ b/src/backend/commands/cluster.c
> @@ -44,6 +44,7 @@
>  #include "storage/smgr.h"
>  #include "utils/acl.h"
>  #include "utils/fmgroids.h"
> +#include "utils/guc.h"
>  #include "utils/inval.h"
>  #include "utils/lsyscache.h"
>  #include "utils/memutils.h"
> @@ -260,6 +261,9 @@
>  cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
>  {
>      Relation    OldHeap;
> +    Oid            save_userid;
> +    int            save_sec_context;
> +    int            save_nestlevel;
>  
>      /* Check for user-requested abort. */
>      CHECK_FOR_INTERRUPTS();
> @@ -277,6 +281,16 @@
>          return;
>  
>      /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(OldHeap->rd_rel->relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
> +
> +    /*
>       * Since we may open a new transaction for each relation, we have to check
>       * that the relation still is what we think it is.
>       *
> @@ -290,10 +304,10 @@
>          Form_pg_index indexForm;
>  
>          /* Check that the user still owns the relation */
> -        if (!pg_class_ownercheck(tableOid, GetUserId()))
> +        if (!pg_class_ownercheck(tableOid, save_userid))
>          {
>              relation_close(OldHeap, AccessExclusiveLock);
> -            return;
> +            goto out;
>          }
>  
>          /*
> @@ -307,7 +321,7 @@
>          if (RELATION_IS_OTHER_TEMP(OldHeap))
>          {
>              relation_close(OldHeap, AccessExclusiveLock);
> -            return;
> +            goto out;
>          }
>  
>          if (OidIsValid(indexOid))
> @@ -318,7 +332,7 @@
>              if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid)))
>              {
>                  relation_close(OldHeap, AccessExclusiveLock);
> -                return;
> +                goto out;
>              }
>  
>              /*
> @@ -328,14 +342,14 @@
>              if (!HeapTupleIsValid(tuple))        /* probably can't happen */
>              {
>                  relation_close(OldHeap, AccessExclusiveLock);
> -                return;
> +                goto out;
>              }
>              indexForm = (Form_pg_index) GETSTRUCT(tuple);
>              if (!indexForm->indisclustered)
>              {
>                  ReleaseSysCache(tuple);
>                  relation_close(OldHeap, AccessExclusiveLock);
> -                return;
> +                goto out;
>              }
>              ReleaseSysCache(tuple);
>          }
> @@ -389,7 +403,7 @@
>          !RelationIsPopulated(OldHeap))
>      {
>          relation_close(OldHeap, AccessExclusiveLock);
> -        return;
> +        goto out;
>      }
>  
>      /*
> @@ -404,6 +418,13 @@
>      rebuild_relation(OldHeap, indexOid, verbose);
>  
>      /* NB: rebuild_relation does heap_close() on OldHeap */
> +
> +out:
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(save_userid, save_sec_context);
>  }
>  
>  /*
> --- a/src/backend/commands/indexcmds.c
> +++ b/src/backend/commands/indexcmds.c
> @@ -49,6 +49,7 @@
>  #include "utils/acl.h"
>  #include "utils/builtins.h"
>  #include "utils/fmgroids.h"
> +#include "utils/guc.h"
>  #include "utils/inval.h"
>  #include "utils/lsyscache.h"
>  #include "utils/memutils.h"
> @@ -339,8 +340,13 @@
>      LOCKTAG        heaplocktag;
>      LOCKMODE    lockmode;
>      Snapshot    snapshot;
> +    Oid            root_save_userid;
> +    int            root_save_sec_context;
> +    int            root_save_nestlevel;
>      int            i;
>  
> +    root_save_nestlevel = NewGUCNestLevel();
> +
>      /*
>       * Force non-concurrent build on temporary relations, even if CONCURRENTLY
>       * was requested.  Other backends can't access a temporary relation, so
> @@ -381,6 +387,15 @@
>      lockmode = concurrent ? ShareUpdateExclusiveLock : ShareLock;
>      rel = heap_open(relationId, lockmode);
>  
> +    /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations.  We
> +     * already arranged to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context);
> +    SetUserIdAndSecContext(rel->rd_rel->relowner,
> +                           root_save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +
>      relationId = RelationGetRelid(rel);
>      namespaceId = RelationGetNamespace(rel);
>  
> @@ -422,7 +437,7 @@
>      {
>          AclResult    aclresult;
>  
> -        aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
> +        aclresult = pg_namespace_aclcheck(namespaceId, root_save_userid,
>                                            ACL_CREATE);
>          if (aclresult != ACLCHECK_OK)
>              aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
> @@ -449,7 +464,7 @@
>      {
>          AclResult    aclresult;
>  
> -        aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(),
> +        aclresult = pg_tablespace_aclcheck(tablespaceId, root_save_userid,
>                                             ACL_CREATE);
>          if (aclresult != ACLCHECK_OK)
>              aclcheck_error(aclresult, ACL_KIND_TABLESPACE,
> @@ -679,15 +694,33 @@
>  
>      if (!OidIsValid(indexRelationId))
>      {
> +        /* Roll back any GUC changes executed by index functions. */
> +        AtEOXact_GUC(false, root_save_nestlevel);
> +
> +        /* Restore userid and security context */
> +        SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
> +
>          heap_close(rel, NoLock);
>          return address;
>      }
>  
> +    /*
> +     * Roll back any GUC changes executed by index functions, and keep
> +     * subsequent changes local to this command.  It's barely possible that
> +     * some index function changed a behavior-affecting GUC, e.g. xmloption,
> +     * that affects subsequent steps.
> +     */
> +    AtEOXact_GUC(false, root_save_nestlevel);
> +    root_save_nestlevel = NewGUCNestLevel();
> +
>      /* Add any requested comment */
>      if (stmt->idxcomment != NULL)
>          CreateComments(indexRelationId, RelationRelationId, 0,
>                         stmt->idxcomment);
>  
> +    AtEOXact_GUC(false, root_save_nestlevel);
> +    SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
> +
>      if (!concurrent)
>      {
>          /* Close the heap and we're done, in the non-concurrent case */
> @@ -766,6 +799,16 @@
>      /* Open and lock the parent heap relation */
>      rel = heap_openrv(stmt->relation, ShareUpdateExclusiveLock);
>  
> +    /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context);
> +    SetUserIdAndSecContext(rel->rd_rel->relowner,
> +                           root_save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    root_save_nestlevel = NewGUCNestLevel();
> +
>      /* And the target index relation */
>      indexRelation = index_open(indexRelationId, RowExclusiveLock);
>  
> @@ -781,6 +824,12 @@
>      /* Now build the index */
>      index_build(rel, indexRelation, indexInfo, stmt->primary, false);
>  
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, root_save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
> +
>      /* Close both the relations, but keep the locks */
>      heap_close(rel, NoLock);
>      index_close(indexRelation, NoLock);
> --- a/src/backend/utils/init/miscinit.c
> +++ b/src/backend/utils/init/miscinit.c
> @@ -365,15 +365,21 @@
>   * with guc.c's internal state, so SET ROLE has to be disallowed.
>   *
>   * SECURITY_RESTRICTED_OPERATION indicates that we are inside an operation
> - * that does not wish to trust called user-defined functions at all.  This
> - * bit prevents not only SET ROLE, but various other changes of session state
> - * that normally is unprotected but might possibly be used to subvert the
> - * calling session later.  An example is replacing an existing prepared
> - * statement with new code, which will then be executed with the outer
> - * session's permissions when the prepared statement is next used.  Since
> - * these restrictions are fairly draconian, we apply them only in contexts
> - * where the called functions are really supposed to be side-effect-free
> - * anyway, such as VACUUM/ANALYZE/REINDEX.
> + * that does not wish to trust called user-defined functions at all.  The
> + * policy is to use this before operations, e.g. autovacuum and REINDEX, that
> + * enumerate relations of a database or schema and run functions associated
> + * with each found relation.  The relation owner is the new user ID.  Set this
> + * as soon as possible after locking the relation.  Restore the old user ID as
> + * late as possible before closing the relation; restoring it shortly after
> + * close is also tolerable.  If a command has both relation-enumerating and
> + * non-enumerating modes, e.g. ANALYZE, both modes set this bit.  This bit
> + * prevents not only SET ROLE, but various other changes of session state that
> + * normally is unprotected but might possibly be used to subvert the calling
> + * session later.  An example is replacing an existing prepared statement with
> + * new code, which will then be executed with the outer session's permissions
> + * when the prepared statement is next used.  These restrictions are fairly
> + * draconian, but the functions called in relation-enumerating operations are
> + * really supposed to be side-effect-free anyway.
>   *
>   * SECURITY_NOFORCE_RLS indicates that we are inside an operation which should
>   * ignore the FORCE ROW LEVEL SECURITY per-table indication.  This is used to
> --- a/src/test/regress/expected/privileges.out
> +++ b/src/test/regress/expected/privileges.out
> @@ -1244,6 +1244,48 @@
>  -- security-restricted operations
>  \c -
>  CREATE ROLE regress_sro_user;
> +-- Check that index expressions and predicates are run as the table's owner
> +-- A dummy index function checking current_user
> +CREATE FUNCTION sro_ifun(int) RETURNS int AS $$
> +BEGIN
> +    -- Below we set the table's owner to regress_sro_user
> +    ASSERT current_user = 'regress_sro_user',
> +        format('sro_ifun(%s) called by %s', $1, current_user);
> +    RETURN $1;
> +END;
> +$$ LANGUAGE plpgsql IMMUTABLE;
> +-- Create a table owned by regress_sro_user
> +CREATE TABLE sro_tab (a int);
> +ALTER TABLE sro_tab OWNER TO regress_sro_user;
> +INSERT INTO sro_tab VALUES (1), (2), (3);
> +-- Create an expression index with a predicate
> +CREATE INDEX sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +DROP INDEX sro_idx;
> +-- Do the same concurrently
> +CREATE INDEX CONCURRENTLY sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +-- REINDEX
> +REINDEX TABLE sro_tab;
> +REINDEX INDEX sro_idx;
> +REINDEX TABLE CONCURRENTLY sro_tab;  -- v12+ feature
> +ERROR:  syntax error at or near "CONCURRENTLY"
> +LINE 1: REINDEX TABLE CONCURRENTLY sro_tab;
> +                      ^
> +DROP INDEX sro_idx;
> +-- CLUSTER
> +CREATE INDEX sro_cluster_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)));
> +CLUSTER sro_tab USING sro_cluster_idx;
> +DROP INDEX sro_cluster_idx;
> +-- BRIN index
> +CREATE INDEX sro_brin ON sro_tab USING brin ((sro_ifun(a) + sro_ifun(0)));
> +SELECT brin_summarize_new_values('sro_brin');
> + brin_summarize_new_values 
> +---------------------------
> +                         0
> +(1 row)
> +
> +DROP TABLE sro_tab;
>  SET SESSION AUTHORIZATION regress_sro_user;
>  CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
>      'GRANT regress_group2 TO regress_sro_user';
> --- a/src/test/regress/sql/privileges.sql
> +++ b/src/test/regress/sql/privileges.sql
> @@ -762,6 +762,42 @@
>  \c -
>  CREATE ROLE regress_sro_user;
>  
> +-- Check that index expressions and predicates are run as the table's owner
> +
> +-- A dummy index function checking current_user
> +CREATE FUNCTION sro_ifun(int) RETURNS int AS $$
> +BEGIN
> +    -- Below we set the table's owner to regress_sro_user
> +    ASSERT current_user = 'regress_sro_user',
> +        format('sro_ifun(%s) called by %s', $1, current_user);
> +    RETURN $1;
> +END;
> +$$ LANGUAGE plpgsql IMMUTABLE;
> +-- Create a table owned by regress_sro_user
> +CREATE TABLE sro_tab (a int);
> +ALTER TABLE sro_tab OWNER TO regress_sro_user;
> +INSERT INTO sro_tab VALUES (1), (2), (3);
> +-- Create an expression index with a predicate
> +CREATE INDEX sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +DROP INDEX sro_idx;
> +-- Do the same concurrently
> +CREATE INDEX CONCURRENTLY sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +-- REINDEX
> +REINDEX TABLE sro_tab;
> +REINDEX INDEX sro_idx;
> +REINDEX TABLE CONCURRENTLY sro_tab;  -- v12+ feature
> +DROP INDEX sro_idx;
> +-- CLUSTER
> +CREATE INDEX sro_cluster_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)));
> +CLUSTER sro_tab USING sro_cluster_idx;
> +DROP INDEX sro_cluster_idx;
> +-- BRIN index
> +CREATE INDEX sro_brin ON sro_tab USING brin ((sro_ifun(a) + sro_ifun(0)));
> +SELECT brin_summarize_new_values('sro_brin');
> +DROP TABLE sro_tab;
> +
>  SET SESSION AUTHORIZATION regress_sro_user;
>  CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
>      'GRANT regress_group2 TO regress_sro_user';

> From f26d5702857a9c027f84850af48b0eea0f3aa15c Mon Sep 17 00:00:00 2001
> From: Noah Misch <noah@leadboat.com>
> Date: Mon, 9 May 2022 08:35:08 -0700
> Subject: [PATCH] In REFRESH MATERIALIZED VIEW, set user ID before running user
>  code.
> 
> It intended to, but did not, achieve this.  Adopt the new standard of
> setting user ID just after locking the relation.  Back-patch to v10 (all
> supported versions).
> 
> Reviewed by Simon Riggs.  Reported by Alvaro Herrera.
> 
> Security: CVE-2022-1552
> ---
>  src/backend/commands/matview.c           |   30 +++++++++++-------------------
>  src/test/regress/expected/privileges.out |   15 +++++++++++++++
>  src/test/regress/sql/privileges.sql      |   16 ++++++++++++++++
>  3 files changed, 42 insertions(+), 19 deletions(-)
> 
> --- a/src/backend/commands/matview.c
> +++ b/src/backend/commands/matview.c
> @@ -164,6 +164,17 @@
>                                            lockmode, false, false,
>                                            RangeVarCallbackOwnsTable, NULL);
>      matviewRel = heap_open(matviewOid, NoLock);
> +    relowner = matviewRel->rd_rel->relowner;
> +
> +    /*
> +     * Switch to the owner's userid, so that any functions are run as that
> +     * user.  Also lock down security-restricted operations and arrange to
> +     * make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
>  
>      /* Make sure it is a materialized view. */
>      if (matviewRel->rd_rel->relkind != RELKIND_MATVIEW)
> @@ -269,19 +280,6 @@
>       */
>      SetMatViewPopulatedState(matviewRel, !stmt->skipData);
>  
> -    relowner = matviewRel->rd_rel->relowner;
> -
> -    /*
> -     * Switch to the owner's userid, so that any functions are run as that
> -     * user.  Also arrange to make GUC variable changes local to this command.
> -     * Don't lock it down too tight to create a temporary table just yet.  We
> -     * will switch modes when we are about to execute user code.
> -     */
> -    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> -    SetUserIdAndSecContext(relowner,
> -                           save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
> -    save_nestlevel = NewGUCNestLevel();
> -
>      /* Concurrent refresh builds new data in temp tablespace, and does diff. */
>      if (concurrent)
>      {
> @@ -304,12 +302,6 @@
>      LockRelationOid(OIDNewHeap, AccessExclusiveLock);
>      dest = CreateTransientRelDestReceiver(OIDNewHeap);
>  
> -    /*
> -     * Now lock down security-restricted operations.
> -     */
> -    SetUserIdAndSecContext(relowner,
> -                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> -
>      /* Generate the data, if wanted. */
>      if (!stmt->skipData)
>          refresh_matview_datafill(dest, dataQuery, queryString);
> --- a/src/test/regress/expected/privileges.out
> +++ b/src/test/regress/expected/privileges.out
> @@ -1323,6 +1323,21 @@
>  SQL statement "SELECT unwanted_grant()"
>  PL/pgSQL function sro_trojan() line 1 at PERFORM
>  SQL function "mv_action" statement 1
> +-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
> +SET SESSION AUTHORIZATION regress_sro_user;
> +CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
> +    IMMUTABLE LANGUAGE plpgsql AS $$
> +BEGIN
> +    PERFORM unwanted_grant();
> +    RAISE WARNING 'owned';
> +    RETURN 1;
> +EXCEPTION WHEN OTHERS THEN
> +    RETURN 2;
> +END$$;
> +CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
> +CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
> +\c -
> +REFRESH MATERIALIZED VIEW sro_index_mv;
>  DROP OWNED BY regress_sro_user;
>  DROP ROLE regress_sro_user;
>  -- Admin options
> --- a/src/test/regress/sql/privileges.sql
> +++ b/src/test/regress/sql/privileges.sql
> @@ -824,6 +824,22 @@
>  REFRESH MATERIALIZED VIEW sro_mv;
>  BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
>  
> +-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
> +SET SESSION AUTHORIZATION regress_sro_user;
> +CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
> +    IMMUTABLE LANGUAGE plpgsql AS $$
> +BEGIN
> +    PERFORM unwanted_grant();
> +    RAISE WARNING 'owned';
> +    RETURN 1;
> +EXCEPTION WHEN OTHERS THEN
> +    RETURN 2;
> +END$$;
> +CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
> +CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
> +\c -
> +REFRESH MATERIALIZED VIEW sro_index_mv;
> +
>  DROP OWNED BY regress_sro_user;
>  DROP ROLE regress_sro_user;
>  

> From 88b39e61486a8925a3861d50c306a51eaa1af8d6 Mon Sep 17 00:00:00 2001
> From: Noah Misch <noah@leadboat.com>
> Date: Sat, 25 Jun 2022 09:07:41 -0700
> Subject: [PATCH] CREATE INDEX: use the original userid for more ACL checks.
> 
> Commit a117cebd638dd02e5c2e791c25e43745f233111b used the original userid
> for ACL checks located directly in DefineIndex(), but it still adopted
> the table owner userid for more ACL checks than intended.  That broke
> dump/reload of indexes that refer to an operator class, collation, or
> exclusion operator in a schema other than "public" or "pg_catalog".
> Back-patch to v10 (all supported versions), like the earlier commit.
> 
> Nathan Bossart and Noah Misch
> 
> Discussion: https://postgr.es/m/f8a4105f076544c180a87ef0c4822352@stmuk.bayern.de
> ---
>  contrib/citext/Makefile                      |    2 
>  contrib/citext/expected/create_index_acl.out |   81 +++++++++++++++++++++++
>  contrib/citext/sql/create_index_acl.sql      |   82 ++++++++++++++++++++++++
>  src/backend/commands/indexcmds.c             |   92 +++++++++++++++++++++++----
>  4 files changed, 244 insertions(+), 13 deletions(-)
>  create mode 100644 contrib/citext/expected/create_index_acl.out
>  create mode 100644 contrib/citext/sql/create_index_acl.sql
> 
> --- a/contrib/citext/Makefile
> +++ b/contrib/citext/Makefile
> @@ -7,7 +7,7 @@
>      citext--1.0--1.1.sql citext--unpackaged--1.0.sql
>  PGFILEDESC = "citext - case-insensitive character string data type"
>  
> -REGRESS = citext
> +REGRESS = create_index_acl citext
>  
>  ifdef USE_PGXS
>  PG_CONFIG = pg_config
> --- /dev/null
> +++ b/contrib/citext/expected/create_index_acl.out
> @@ -0,0 +1,81 @@
> +-- Each DefineIndex() ACL check uses either the original userid or the table
> +-- owner userid; see its header comment.  Here, confirm that DefineIndex()
> +-- uses its original userid where necessary.  The test works by creating
> +-- indexes that refer to as many sorts of objects as possible, with the table
> +-- owner having as few applicable privileges as possible.  (The privileges.sql
> +-- regress_sro_user tests look for the opposite defect; they confirm that
> +-- DefineIndex() uses the table owner userid where necessary.)
> +-- Don't override tablespaces; this version lacks allow_in_place_tablespaces.
> +BEGIN;
> +CREATE ROLE regress_minimal;
> +CREATE SCHEMA s;
> +CREATE EXTENSION citext SCHEMA s;
> +-- Revoke all conceivably-relevant ACLs within the extension.  The system
> +-- doesn't check all these ACLs, but this will provide some coverage if that
> +-- ever changes.
> +REVOKE ALL ON TYPE s.citext FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_lt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_le(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_eq(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_ge(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_gt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_cmp(s.citext, s.citext) FROM PUBLIC;
> +-- Functions sufficient for making an index column that has the side effect of
> +-- changing search_path at expression planning time.
> +CREATE FUNCTION public.setter() RETURNS bool VOLATILE
> +  LANGUAGE SQL AS $$SET search_path = s; SELECT true$$;
> +CREATE FUNCTION s.const() RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT public.setter()$$;
> +CREATE FUNCTION s.index_this_expr(s.citext, bool) RETURNS s.citext IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1$$;
> +REVOKE ALL ON FUNCTION public.setter() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.const() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.index_this_expr(s.citext, bool) FROM PUBLIC;
> +-- Even for an empty table, expression planning calls s.const & public.setter.
> +GRANT EXECUTE ON FUNCTION public.setter() TO regress_minimal;
> +GRANT EXECUTE ON FUNCTION s.const() TO regress_minimal;
> +-- Function for index predicate.
> +CREATE FUNCTION s.index_row_if(s.citext) RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1 IS NOT NULL$$;
> +REVOKE ALL ON FUNCTION s.index_row_if(s.citext) FROM PUBLIC;
> +-- Even for an empty table, CREATE INDEX checks ii_Predicate permissions.
> +GRANT EXECUTE ON FUNCTION s.index_row_if(s.citext) TO regress_minimal;
> +-- Non-extension, non-function objects.
> +CREATE COLLATION s.coll (LOCALE="C");
> +CREATE TABLE s.x (y s.citext);
> +ALTER TABLE s.x OWNER TO regress_minimal;
> +-- Empty-table DefineIndex()
> +CREATE UNIQUE INDEX u0rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e0rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Make the table nonempty.
> +INSERT INTO s.x VALUES ('foo'), ('bar');
> +-- If the INSERT runs the planner on index expressions, a search_path change
> +-- survives.  As of 2022-06, the INSERT reuses a cached plan.  It does so even
> +-- under debug_discard_caches, since each index is new-in-transaction.  If
> +-- future work changes a cache lifecycle, this RESET may become necessary.
> +RESET search_path;
> +-- For a nonempty table, owner needs permissions throughout ii_Expressions.
> +GRANT EXECUTE ON FUNCTION s.index_this_expr(s.citext, bool) TO regress_minimal;
> +CREATE UNIQUE INDEX u2rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e2rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Shall not find s.coll via search_path, despite the s.const->public.setter
> +-- call having set search_path=s during expression planning.  Suppress the
> +-- message itself, which depends on the database encoding.
> +\set VERBOSITY terse
> +DO $$
> +BEGIN
> +ALTER TABLE s.x ADD CONSTRAINT underqualified EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +EXCEPTION WHEN OTHERS THEN RAISE EXCEPTION '%', sqlstate; END$$;
> +ERROR:  42704
> +\set VERBOSITY default
> +ROLLBACK;
> --- /dev/null
> +++ b/contrib/citext/sql/create_index_acl.sql
> @@ -0,0 +1,82 @@
> +-- Each DefineIndex() ACL check uses either the original userid or the table
> +-- owner userid; see its header comment.  Here, confirm that DefineIndex()
> +-- uses its original userid where necessary.  The test works by creating
> +-- indexes that refer to as many sorts of objects as possible, with the table
> +-- owner having as few applicable privileges as possible.  (The privileges.sql
> +-- regress_sro_user tests look for the opposite defect; they confirm that
> +-- DefineIndex() uses the table owner userid where necessary.)
> +
> +-- Don't override tablespaces; this version lacks allow_in_place_tablespaces.
> +
> +BEGIN;
> +CREATE ROLE regress_minimal;
> +CREATE SCHEMA s;
> +CREATE EXTENSION citext SCHEMA s;
> +-- Revoke all conceivably-relevant ACLs within the extension.  The system
> +-- doesn't check all these ACLs, but this will provide some coverage if that
> +-- ever changes.
> +REVOKE ALL ON TYPE s.citext FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_lt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_le(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_eq(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_ge(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_gt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_cmp(s.citext, s.citext) FROM PUBLIC;
> +-- Functions sufficient for making an index column that has the side effect of
> +-- changing search_path at expression planning time.
> +CREATE FUNCTION public.setter() RETURNS bool VOLATILE
> +  LANGUAGE SQL AS $$SET search_path = s; SELECT true$$;
> +CREATE FUNCTION s.const() RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT public.setter()$$;
> +CREATE FUNCTION s.index_this_expr(s.citext, bool) RETURNS s.citext IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1$$;
> +REVOKE ALL ON FUNCTION public.setter() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.const() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.index_this_expr(s.citext, bool) FROM PUBLIC;
> +-- Even for an empty table, expression planning calls s.const & public.setter.
> +GRANT EXECUTE ON FUNCTION public.setter() TO regress_minimal;
> +GRANT EXECUTE ON FUNCTION s.const() TO regress_minimal;
> +-- Function for index predicate.
> +CREATE FUNCTION s.index_row_if(s.citext) RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1 IS NOT NULL$$;
> +REVOKE ALL ON FUNCTION s.index_row_if(s.citext) FROM PUBLIC;
> +-- Even for an empty table, CREATE INDEX checks ii_Predicate permissions.
> +GRANT EXECUTE ON FUNCTION s.index_row_if(s.citext) TO regress_minimal;
> +-- Non-extension, non-function objects.
> +CREATE COLLATION s.coll (LOCALE="C");
> +CREATE TABLE s.x (y s.citext);
> +ALTER TABLE s.x OWNER TO regress_minimal;
> +-- Empty-table DefineIndex()
> +CREATE UNIQUE INDEX u0rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e0rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Make the table nonempty.
> +INSERT INTO s.x VALUES ('foo'), ('bar');
> +-- If the INSERT runs the planner on index expressions, a search_path change
> +-- survives.  As of 2022-06, the INSERT reuses a cached plan.  It does so even
> +-- under debug_discard_caches, since each index is new-in-transaction.  If
> +-- future work changes a cache lifecycle, this RESET may become necessary.
> +RESET search_path;
> +-- For a nonempty table, owner needs permissions throughout ii_Expressions.
> +GRANT EXECUTE ON FUNCTION s.index_this_expr(s.citext, bool) TO regress_minimal;
> +CREATE UNIQUE INDEX u2rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e2rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Shall not find s.coll via search_path, despite the s.const->public.setter
> +-- call having set search_path=s during expression planning.  Suppress the
> +-- message itself, which depends on the database encoding.
> +\set VERBOSITY terse
> +DO $$
> +BEGIN
> +ALTER TABLE s.x ADD CONSTRAINT underqualified EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +EXCEPTION WHEN OTHERS THEN RAISE EXCEPTION '%', sqlstate; END$$;
> +\set VERBOSITY default
> +ROLLBACK;
> --- a/src/backend/commands/indexcmds.c
> +++ b/src/backend/commands/indexcmds.c
> @@ -70,7 +70,10 @@
>                    Oid relId,
>                    char *accessMethodName, Oid accessMethodId,
>                    bool amcanorder,
> -                  bool isconstraint);
> +                  bool isconstraint,
> +                  Oid ddl_userid,
> +                  int ddl_sec_context,
> +                  int *ddl_save_nestlevel);
>  static Oid GetIndexOpClass(List *opclass, Oid attrType,
>                  char *accessMethodName, Oid accessMethodId);
>  static char *ChooseIndexName(const char *tabname, Oid namespaceId,
> @@ -176,8 +179,7 @@
>       * Compute the operator classes, collations, and exclusion operators for
>       * the new index, so we can test whether it's compatible with the existing
>       * one.  Note that ComputeIndexAttrs might fail here, but that's OK:
> -     * DefineIndex would have called this function with the same arguments
> -     * later on, and it would have failed then anyway.
> +     * DefineIndex would have failed later.
>       */
>      indexInfo = makeNode(IndexInfo);
>      indexInfo->ii_Expressions = NIL;
> @@ -195,7 +197,7 @@
>                        coloptions, attributeList,
>                        exclusionOpNames, relationId,
>                        accessMethodName, accessMethodId,
> -                      amcanorder, isconstraint);
> +                      amcanorder, isconstraint, InvalidOid, 0, NULL);
>  
>  
>      /* Get the soon-obsolete pg_index tuple. */
> @@ -288,6 +290,19 @@
>   * DefineIndex
>   *        Creates a new index.
>   *
> + * This function manages the current userid according to the needs of pg_dump.
> + * Recreating old-database catalog entries in new-database is fine, regardless
> + * of which users would have permission to recreate those entries now.  That's
> + * just preservation of state.  Running opaque expressions, like calling a
> + * function named in a catalog entry or evaluating a pg_node_tree in a catalog
> + * entry, as anyone other than the object owner, is not fine.  To adhere to
> + * those principles and to remain fail-safe, use the table owner userid for
> + * most ACL checks.  Use the original userid for ACL checks reached without
> + * traversing opaque expressions.  (pg_dump can predict such ACL checks from
> + * catalogs.)  Overall, this is a mess.  Future DDL development should
> + * consider offering one DDL command for catalog setup and a separate DDL
> + * command for steps that run opaque expressions.
> + *
>   * 'relationId': the OID of the heap relation on which the index is to be
>   *        created
>   * 'stmt': IndexStmt describing the properties of the new index.
> @@ -598,7 +613,8 @@
>                        coloptions, stmt->indexParams,
>                        stmt->excludeOpNames, relationId,
>                        accessMethodName, accessMethodId,
> -                      amcanorder, stmt->isconstraint);
> +                      amcanorder, stmt->isconstraint, root_save_userid,
> +                      root_save_sec_context, &root_save_nestlevel);
>  
>      /*
>       * Extra checks when creating a PRIMARY KEY index.
> @@ -706,9 +722,8 @@
>  
>      /*
>       * Roll back any GUC changes executed by index functions, and keep
> -     * subsequent changes local to this command.  It's barely possible that
> -     * some index function changed a behavior-affecting GUC, e.g. xmloption,
> -     * that affects subsequent steps.
> +     * subsequent changes local to this command.  This is essential if some
> +     * index function changed a behavior-affecting GUC, e.g. search_path.
>       */
>      AtEOXact_GUC(false, root_save_nestlevel);
>      root_save_nestlevel = NewGUCNestLevel();
> @@ -1063,6 +1078,10 @@
>  /*
>   * Compute per-index-column information, including indexed column numbers
>   * or index expressions, opclasses, and indoptions.
> + *
> + * If the caller switched to the table owner, ddl_userid is the role for ACL
> + * checks reached without traversing opaque expressions.  Otherwise, it's
> + * InvalidOid, and other ddl_* arguments are undefined.
>   */
>  static void
>  ComputeIndexAttrs(IndexInfo *indexInfo,
> @@ -1076,11 +1095,16 @@
>                    char *accessMethodName,
>                    Oid accessMethodId,
>                    bool amcanorder,
> -                  bool isconstraint)
> +                  bool isconstraint,
> +                  Oid ddl_userid,
> +                  int ddl_sec_context,
> +                  int *ddl_save_nestlevel)
>  {
>      ListCell   *nextExclOp;
>      ListCell   *lc;
>      int            attn;
> +    Oid            save_userid;
> +    int            save_sec_context;
>  
>      /* Allocate space for exclusion operator info, if needed */
>      if (exclusionOpNames)
> @@ -1096,6 +1120,9 @@
>      else
>          nextExclOp = NULL;
>  
> +    if (OidIsValid(ddl_userid))
> +        GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +
>      /*
>       * process attributeList
>       */
> @@ -1190,10 +1217,24 @@
>          typeOidP[attn] = atttype;
>  
>          /*
> -         * Apply collation override if any
> +         * Apply collation override if any.  Use of ddl_userid is necessary
> +         * due to ACL checks therein, and it's safe because collations don't
> +         * contain opaque expressions (or non-opaque expressions).
>           */
>          if (attribute->collation)
> +        {
> +            if (OidIsValid(ddl_userid))
> +            {
> +                AtEOXact_GUC(false, *ddl_save_nestlevel);
> +                SetUserIdAndSecContext(ddl_userid, ddl_sec_context);
> +            }
>              attcollation = get_collation_oid(attribute->collation, false);
> +            if (OidIsValid(ddl_userid))
> +            {
> +                SetUserIdAndSecContext(save_userid, save_sec_context);
> +                *ddl_save_nestlevel = NewGUCNestLevel();
> +            }
> +        }
>  
>          /*
>           * Check we have a collation iff it's a collatable type.  The only
> @@ -1221,12 +1262,25 @@
>          collationOidP[attn] = attcollation;
>  
>          /*
> -         * Identify the opclass to use.
> +         * Identify the opclass to use.  Use of ddl_userid is necessary due to
> +         * ACL checks therein.  This is safe despite opclasses containing
> +         * opaque expressions (specifically, functions), because only
> +         * superusers can define opclasses.
>           */
> +        if (OidIsValid(ddl_userid))
> +        {
> +            AtEOXact_GUC(false, *ddl_save_nestlevel);
> +            SetUserIdAndSecContext(ddl_userid, ddl_sec_context);
> +        }
>          classOidP[attn] = GetIndexOpClass(attribute->opclass,
>                                            atttype,
>                                            accessMethodName,
>                                            accessMethodId);
> +        if (OidIsValid(ddl_userid))
> +        {
> +            SetUserIdAndSecContext(save_userid, save_sec_context);
> +            *ddl_save_nestlevel = NewGUCNestLevel();
> +        }
>  
>          /*
>           * Identify the exclusion operator, if any.
> @@ -1240,9 +1294,23 @@
>  
>              /*
>               * Find the operator --- it must accept the column datatype
> -             * without runtime coercion (but binary compatibility is OK)
> +             * without runtime coercion (but binary compatibility is OK).
> +             * Operators contain opaque expressions (specifically, functions).
> +             * compatible_oper_opid() boils down to oper() and
> +             * IsBinaryCoercible().  PostgreSQL would have security problems
> +             * elsewhere if oper() started calling opaque expressions.
>               */
> +            if (OidIsValid(ddl_userid))
> +            {
> +                AtEOXact_GUC(false, *ddl_save_nestlevel);
> +                SetUserIdAndSecContext(ddl_userid, ddl_sec_context);
> +            }
>              opid = compatible_oper_opid(opname, atttype, atttype, false);
> +            if (OidIsValid(ddl_userid))
> +            {
> +                SetUserIdAndSecContext(save_userid, save_sec_context);
> +                *ddl_save_nestlevel = NewGUCNestLevel();
> +            }
>  
>              /*
>               * Only allow commutative operators to be used in exclusion

> From ef792f7856dea2576dcd9cab92b2b05fe955696b Mon Sep 17 00:00:00 2001
> From: Noah Misch <noah@leadboat.com>
> Date: Mon, 9 May 2022 08:35:08 -0700
> Subject: [PATCH] Make relation-enumerating operations be security-restricted
>  operations.
> 
> When a feature enumerates relations and runs functions associated with
> all found relations, the feature's user shall not need to trust every
> user having permission to create objects.  BRIN-specific functionality
> in autovacuum neglected to account for this, as did pg_amcheck and
> CLUSTER.  An attacker having permission to create non-temp objects in at
> least one schema could execute arbitrary SQL functions under the
> identity of the bootstrap superuser.  CREATE INDEX (not a
> relation-enumerating operation) and REINDEX protected themselves too
> late.  This change extends to the non-enumerating amcheck interface.
> Back-patch to v10 (all supported versions).
> 
> Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin.
> Reported by Alexander Lakhin.
> 
> Security: CVE-2022-1552
> ---
>  src/backend/catalog/index.c              |   41 +++++++++++++++++++--------
>  src/backend/commands/cluster.c           |   35 ++++++++++++++++++-----
>  src/backend/commands/indexcmds.c         |   47 +++++++++++++++++++++++++++++--
>  src/backend/utils/init/miscinit.c        |   24 +++++++++------
>  src/test/regress/expected/privileges.out |   35 +++++++++++++++++++++++
>  src/test/regress/sql/privileges.sql      |   34 ++++++++++++++++++++++
>  6 files changed, 187 insertions(+), 29 deletions(-)
> 
> --- a/src/backend/catalog/index.c
> +++ b/src/backend/catalog/index.c
> @@ -2743,7 +2743,17 @@
>  
>      /* Open and lock the parent heap relation */
>      heapRelation = heap_open(heapId, ShareUpdateExclusiveLock);
> -    /* And the target index relation */
> +
> +    /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
> +
>      indexRelation = index_open(indexId, RowExclusiveLock);
>  
>      /*
> @@ -2757,16 +2767,6 @@
>      indexInfo->ii_Concurrent = true;
>  
>      /*
> -     * Switch to the table owner's userid, so that any index functions are run
> -     * as that user.  Also lock down security-restricted operations and
> -     * arrange to make GUC variable changes local to this command.
> -     */
> -    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> -    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
> -                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> -    save_nestlevel = NewGUCNestLevel();
> -
> -    /*
>       * Scan the index and gather up all the TIDs into a tuplesort object.
>       */
>      ivinfo.index = indexRelation;
> @@ -3178,6 +3178,9 @@
>      Relation    iRel,
>                  heapRelation;
>      Oid            heapId;
> +    Oid            save_userid;
> +    int            save_sec_context;
> +    int            save_nestlevel;
>      IndexInfo  *indexInfo;
>      volatile bool skipped_constraint = false;
>  
> @@ -3189,6 +3192,16 @@
>      heapRelation = heap_open(heapId, ShareLock);
>  
>      /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
> +
> +    /*
>       * Open the target index relation and get an exclusive lock on it, to
>       * ensure that no one else is touching this particular index.
>       */
> @@ -3324,6 +3337,12 @@
>          heap_close(pg_index, RowExclusiveLock);
>      }
>  
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(save_userid, save_sec_context);
> +
>      /* Close rels, but keep locks */
>      index_close(iRel, NoLock);
>      heap_close(heapRelation, NoLock);
> --- a/src/backend/commands/cluster.c
> +++ b/src/backend/commands/cluster.c
> @@ -41,6 +41,7 @@
>  #include "storage/smgr.h"
>  #include "utils/acl.h"
>  #include "utils/fmgroids.h"
> +#include "utils/guc.h"
>  #include "utils/inval.h"
>  #include "utils/lsyscache.h"
>  #include "utils/memutils.h"
> @@ -259,6 +260,9 @@
>  cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose)
>  {
>      Relation    OldHeap;
> +    Oid            save_userid;
> +    int            save_sec_context;
> +    int            save_nestlevel;
>  
>      /* Check for user-requested abort. */
>      CHECK_FOR_INTERRUPTS();
> @@ -276,6 +280,16 @@
>          return;
>  
>      /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(OldHeap->rd_rel->relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
> +
> +    /*
>       * Since we may open a new transaction for each relation, we have to check
>       * that the relation still is what we think it is.
>       *
> @@ -289,10 +303,10 @@
>          Form_pg_index indexForm;
>  
>          /* Check that the user still owns the relation */
> -        if (!pg_class_ownercheck(tableOid, GetUserId()))
> +        if (!pg_class_ownercheck(tableOid, save_userid))
>          {
>              relation_close(OldHeap, AccessExclusiveLock);
> -            return;
> +            goto out;
>          }
>  
>          /*
> @@ -306,7 +320,7 @@
>          if (RELATION_IS_OTHER_TEMP(OldHeap))
>          {
>              relation_close(OldHeap, AccessExclusiveLock);
> -            return;
> +            goto out;
>          }
>  
>          if (OidIsValid(indexOid))
> @@ -317,7 +331,7 @@
>              if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid)))
>              {
>                  relation_close(OldHeap, AccessExclusiveLock);
> -                return;
> +                goto out;
>              }
>  
>              /*
> @@ -327,14 +341,14 @@
>              if (!HeapTupleIsValid(tuple))        /* probably can't happen */
>              {
>                  relation_close(OldHeap, AccessExclusiveLock);
> -                return;
> +                goto out;
>              }
>              indexForm = (Form_pg_index) GETSTRUCT(tuple);
>              if (!indexForm->indisclustered)
>              {
>                  ReleaseSysCache(tuple);
>                  relation_close(OldHeap, AccessExclusiveLock);
> -                return;
> +                goto out;
>              }
>              ReleaseSysCache(tuple);
>          }
> @@ -388,7 +402,7 @@
>          !RelationIsPopulated(OldHeap))
>      {
>          relation_close(OldHeap, AccessExclusiveLock);
> -        return;
> +        goto out;
>      }
>  
>      /*
> @@ -403,6 +417,13 @@
>      rebuild_relation(OldHeap, indexOid, verbose);
>  
>      /* NB: rebuild_relation does heap_close() on OldHeap */
> +
> +out:
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(save_userid, save_sec_context);
>  }
>  
>  /*
> --- a/src/backend/commands/indexcmds.c
> +++ b/src/backend/commands/indexcmds.c
> @@ -44,6 +44,7 @@
>  #include "utils/acl.h"
>  #include "utils/builtins.h"
>  #include "utils/fmgroids.h"
> +#include "utils/guc.h"
>  #include "utils/inval.h"
>  #include "utils/lsyscache.h"
>  #include "utils/memutils.h"
> @@ -329,8 +330,13 @@
>      LOCKTAG        heaplocktag;
>      LOCKMODE    lockmode;
>      Snapshot    snapshot;
> +    Oid            root_save_userid;
> +    int            root_save_sec_context;
> +    int            root_save_nestlevel;
>      int            i;
>  
> +    root_save_nestlevel = NewGUCNestLevel();
> +
>      /*
>       * Force non-concurrent build on temporary relations, even if CONCURRENTLY
>       * was requested.  Other backends can't access a temporary relation, so
> @@ -371,6 +377,15 @@
>      lockmode = concurrent ? ShareUpdateExclusiveLock : ShareLock;
>      rel = heap_open(relationId, lockmode);
>  
> +    /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations.  We
> +     * already arranged to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context);
> +    SetUserIdAndSecContext(rel->rd_rel->relowner,
> +                           root_save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +
>      relationId = RelationGetRelid(rel);
>      namespaceId = RelationGetNamespace(rel);
>  
> @@ -412,7 +427,7 @@
>      {
>          AclResult    aclresult;
>  
> -        aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
> +        aclresult = pg_namespace_aclcheck(namespaceId, root_save_userid,
>                                            ACL_CREATE);
>          if (aclresult != ACLCHECK_OK)
>              aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
> @@ -439,7 +454,7 @@
>      {
>          AclResult    aclresult;
>  
> -        aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(),
> +        aclresult = pg_tablespace_aclcheck(tablespaceId, root_save_userid,
>                                             ACL_CREATE);
>          if (aclresult != ACLCHECK_OK)
>              aclcheck_error(aclresult, ACL_KIND_TABLESPACE,
> @@ -622,11 +637,23 @@
>                       skip_build || concurrent,
>                       concurrent, !check_rights);
>  
> +    /*
> +     * Roll back any GUC changes executed by index functions, and keep
> +     * subsequent changes local to this command.  It's barely possible that
> +     * some index function changed a behavior-affecting GUC, e.g. xmloption,
> +     * that affects subsequent steps.
> +     */
> +    AtEOXact_GUC(false, root_save_nestlevel);
> +    root_save_nestlevel = NewGUCNestLevel();
> +
>      /* Add any requested comment */
>      if (stmt->idxcomment != NULL)
>          CreateComments(indexRelationId, RelationRelationId, 0,
>                         stmt->idxcomment);
>  
> +    AtEOXact_GUC(false, root_save_nestlevel);
> +    SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
> +
>      if (!concurrent)
>      {
>          /* Close the heap and we're done, in the non-concurrent case */
> @@ -705,6 +732,16 @@
>      /* Open and lock the parent heap relation */
>      rel = heap_openrv(stmt->relation, ShareUpdateExclusiveLock);
>  
> +    /*
> +     * Switch to the table owner's userid, so that any index functions are run
> +     * as that user.  Also lock down security-restricted operations and
> +     * arrange to make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&root_save_userid, &root_save_sec_context);
> +    SetUserIdAndSecContext(rel->rd_rel->relowner,
> +                           root_save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    root_save_nestlevel = NewGUCNestLevel();
> +
>      /* And the target index relation */
>      indexRelation = index_open(indexRelationId, RowExclusiveLock);
>  
> @@ -720,6 +757,12 @@
>      /* Now build the index */
>      index_build(rel, indexRelation, indexInfo, stmt->primary, false);
>  
> +    /* Roll back any GUC changes executed by index functions */
> +    AtEOXact_GUC(false, root_save_nestlevel);
> +
> +    /* Restore userid and security context */
> +    SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
> +
>      /* Close both the relations, but keep the locks */
>      heap_close(rel, NoLock);
>      index_close(indexRelation, NoLock);
> --- a/src/backend/utils/init/miscinit.c
> +++ b/src/backend/utils/init/miscinit.c
> @@ -235,15 +235,21 @@
>   * with guc.c's internal state, so SET ROLE has to be disallowed.
>   *
>   * SECURITY_RESTRICTED_OPERATION indicates that we are inside an operation
> - * that does not wish to trust called user-defined functions at all.  This
> - * bit prevents not only SET ROLE, but various other changes of session state
> - * that normally is unprotected but might possibly be used to subvert the
> - * calling session later.  An example is replacing an existing prepared
> - * statement with new code, which will then be executed with the outer
> - * session's permissions when the prepared statement is next used.  Since
> - * these restrictions are fairly draconian, we apply them only in contexts
> - * where the called functions are really supposed to be side-effect-free
> - * anyway, such as VACUUM/ANALYZE/REINDEX.
> + * that does not wish to trust called user-defined functions at all.  The
> + * policy is to use this before operations, e.g. autovacuum and REINDEX, that
> + * enumerate relations of a database or schema and run functions associated
> + * with each found relation.  The relation owner is the new user ID.  Set this
> + * as soon as possible after locking the relation.  Restore the old user ID as
> + * late as possible before closing the relation; restoring it shortly after
> + * close is also tolerable.  If a command has both relation-enumerating and
> + * non-enumerating modes, e.g. ANALYZE, both modes set this bit.  This bit
> + * prevents not only SET ROLE, but various other changes of session state that
> + * normally is unprotected but might possibly be used to subvert the calling
> + * session later.  An example is replacing an existing prepared statement with
> + * new code, which will then be executed with the outer session's permissions
> + * when the prepared statement is next used.  These restrictions are fairly
> + * draconian, but the functions called in relation-enumerating operations are
> + * really supposed to be side-effect-free anyway.
>   *
>   * Unlike GetUserId, GetUserIdAndSecContext does *not* Assert that the current
>   * value of CurrentUserId is valid; nor does SetUserIdAndSecContext require
> --- a/src/test/regress/expected/privileges.out
> +++ b/src/test/regress/expected/privileges.out
> @@ -1194,6 +1194,41 @@
>  -- security-restricted operations
>  \c -
>  CREATE ROLE regress_sro_user;
> +-- Check that index expressions and predicates are run as the table's owner
> +-- A dummy index function checking current_user
> +CREATE FUNCTION sro_ifun(int) RETURNS int AS $$
> +BEGIN
> +    -- Below we set the table's owner to regress_sro_user
> +    IF current_user <> 'regress_sro_user' THEN
> +        RAISE 'called by %', current_user;
> +    END IF;
> +    RETURN $1;
> +END;
> +$$ LANGUAGE plpgsql IMMUTABLE;
> +-- Create a table owned by regress_sro_user
> +CREATE TABLE sro_tab (a int);
> +ALTER TABLE sro_tab OWNER TO regress_sro_user;
> +INSERT INTO sro_tab VALUES (1), (2), (3);
> +-- Create an expression index with a predicate
> +CREATE INDEX sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +DROP INDEX sro_idx;
> +-- Do the same concurrently
> +CREATE INDEX CONCURRENTLY sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +-- REINDEX
> +REINDEX TABLE sro_tab;
> +REINDEX INDEX sro_idx;
> +REINDEX TABLE CONCURRENTLY sro_tab;  -- v12+ feature
> +ERROR:  syntax error at or near "CONCURRENTLY"
> +LINE 1: REINDEX TABLE CONCURRENTLY sro_tab;
> +                      ^
> +DROP INDEX sro_idx;
> +-- CLUSTER
> +CREATE INDEX sro_cluster_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)));
> +CLUSTER sro_tab USING sro_cluster_idx;
> +DROP INDEX sro_cluster_idx;
> +DROP TABLE sro_tab;
>  SET SESSION AUTHORIZATION regress_sro_user;
>  CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
>      'GRANT regressgroup2 TO regress_sro_user';
> --- a/src/test/regress/sql/privileges.sql
> +++ b/src/test/regress/sql/privileges.sql
> @@ -724,6 +724,40 @@
>  \c -
>  CREATE ROLE regress_sro_user;
>  
> +-- Check that index expressions and predicates are run as the table's owner
> +
> +-- A dummy index function checking current_user
> +CREATE FUNCTION sro_ifun(int) RETURNS int AS $$
> +BEGIN
> +    -- Below we set the table's owner to regress_sro_user
> +    IF current_user <> 'regress_sro_user' THEN
> +        RAISE 'called by %', current_user;
> +    END IF;
> +    RETURN $1;
> +END;
> +$$ LANGUAGE plpgsql IMMUTABLE;
> +-- Create a table owned by regress_sro_user
> +CREATE TABLE sro_tab (a int);
> +ALTER TABLE sro_tab OWNER TO regress_sro_user;
> +INSERT INTO sro_tab VALUES (1), (2), (3);
> +-- Create an expression index with a predicate
> +CREATE INDEX sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +DROP INDEX sro_idx;
> +-- Do the same concurrently
> +CREATE INDEX CONCURRENTLY sro_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)))
> +    WHERE sro_ifun(a + 10) > sro_ifun(10);
> +-- REINDEX
> +REINDEX TABLE sro_tab;
> +REINDEX INDEX sro_idx;
> +REINDEX TABLE CONCURRENTLY sro_tab;  -- v12+ feature
> +DROP INDEX sro_idx;
> +-- CLUSTER
> +CREATE INDEX sro_cluster_idx ON sro_tab ((sro_ifun(a) + sro_ifun(0)));
> +CLUSTER sro_tab USING sro_cluster_idx;
> +DROP INDEX sro_cluster_idx;
> +DROP TABLE sro_tab;
> +
>  SET SESSION AUTHORIZATION regress_sro_user;
>  CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
>      'GRANT regressgroup2 TO regress_sro_user';

> From f26d5702857a9c027f84850af48b0eea0f3aa15c Mon Sep 17 00:00:00 2001
> From: Noah Misch <noah@leadboat.com>
> Date: Mon, 9 May 2022 08:35:08 -0700
> Subject: [PATCH] In REFRESH MATERIALIZED VIEW, set user ID before running user
>  code.
> 
> It intended to, but did not, achieve this.  Adopt the new standard of
> setting user ID just after locking the relation.  Back-patch to v10 (all
> supported versions).
> 
> Reviewed by Simon Riggs.  Reported by Alvaro Herrera.
> 
> Security: CVE-2022-1552
> ---
>  src/backend/commands/matview.c           |   30 +++++++++++-------------------
>  src/test/regress/expected/privileges.out |   15 +++++++++++++++
>  src/test/regress/sql/privileges.sql      |   16 ++++++++++++++++
>  3 files changed, 42 insertions(+), 19 deletions(-)
> 
> --- a/src/backend/commands/matview.c
> +++ b/src/backend/commands/matview.c
> @@ -161,6 +161,17 @@
>                                            lockmode, false, false,
>                                            RangeVarCallbackOwnsTable, NULL);
>      matviewRel = heap_open(matviewOid, NoLock);
> +    relowner = matviewRel->rd_rel->relowner;
> +
> +    /*
> +     * Switch to the owner's userid, so that any functions are run as that
> +     * user.  Also lock down security-restricted operations and arrange to
> +     * make GUC variable changes local to this command.
> +     */
> +    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +    SetUserIdAndSecContext(relowner,
> +                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> +    save_nestlevel = NewGUCNestLevel();
>  
>      /* Make sure it is a materialized view. */
>      if (matviewRel->rd_rel->relkind != RELKIND_MATVIEW)
> @@ -233,19 +244,6 @@
>       */
>      SetMatViewPopulatedState(matviewRel, !stmt->skipData);
>  
> -    relowner = matviewRel->rd_rel->relowner;
> -
> -    /*
> -     * Switch to the owner's userid, so that any functions are run as that
> -     * user.  Also arrange to make GUC variable changes local to this command.
> -     * Don't lock it down too tight to create a temporary table just yet.  We
> -     * will switch modes when we are about to execute user code.
> -     */
> -    GetUserIdAndSecContext(&save_userid, &save_sec_context);
> -    SetUserIdAndSecContext(relowner,
> -                           save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
> -    save_nestlevel = NewGUCNestLevel();
> -
>      /* Concurrent refresh builds new data in temp tablespace, and does diff. */
>      if (concurrent)
>          tableSpace = GetDefaultTablespace(RELPERSISTENCE_TEMP);
> @@ -262,12 +260,6 @@
>      LockRelationOid(OIDNewHeap, AccessExclusiveLock);
>      dest = CreateTransientRelDestReceiver(OIDNewHeap);
>  
> -    /*
> -     * Now lock down security-restricted operations.
> -     */
> -    SetUserIdAndSecContext(relowner,
> -                           save_sec_context | SECURITY_RESTRICTED_OPERATION);
> -
>      /* Generate the data, if wanted. */
>      if (!stmt->skipData)
>          refresh_matview_datafill(dest, dataQuery, queryString);
> --- a/src/test/regress/expected/privileges.out
> +++ b/src/test/regress/expected/privileges.out
> @@ -1266,6 +1266,21 @@
>  SQL statement "SELECT unwanted_grant()"
>  PL/pgSQL function sro_trojan() line 1 at PERFORM
>  SQL function "mv_action" statement 1
> +-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
> +SET SESSION AUTHORIZATION regress_sro_user;
> +CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
> +    IMMUTABLE LANGUAGE plpgsql AS $$
> +BEGIN
> +    PERFORM unwanted_grant();
> +    RAISE WARNING 'owned';
> +    RETURN 1;
> +EXCEPTION WHEN OTHERS THEN
> +    RETURN 2;
> +END$$;
> +CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
> +CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
> +\c -
> +REFRESH MATERIALIZED VIEW sro_index_mv;
>  DROP OWNED BY regress_sro_user;
>  DROP ROLE regress_sro_user;
>  -- Admin options
> --- a/src/test/regress/sql/privileges.sql
> +++ b/src/test/regress/sql/privileges.sql
> @@ -784,6 +784,22 @@
>  REFRESH MATERIALIZED VIEW sro_mv;
>  BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
>  
> +-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
> +SET SESSION AUTHORIZATION regress_sro_user;
> +CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
> +    IMMUTABLE LANGUAGE plpgsql AS $$
> +BEGIN
> +    PERFORM unwanted_grant();
> +    RAISE WARNING 'owned';
> +    RETURN 1;
> +EXCEPTION WHEN OTHERS THEN
> +    RETURN 2;
> +END$$;
> +CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
> +CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
> +\c -
> +REFRESH MATERIALIZED VIEW sro_index_mv;
> +
>  DROP OWNED BY regress_sro_user;
>  DROP ROLE regress_sro_user;
>  

> From 88b39e61486a8925a3861d50c306a51eaa1af8d6 Mon Sep 17 00:00:00 2001
> From: Noah Misch <noah@leadboat.com>
> Date: Sat, 25 Jun 2022 09:07:41 -0700
> Subject: [PATCH] CREATE INDEX: use the original userid for more ACL checks.
> 
> Commit a117cebd638dd02e5c2e791c25e43745f233111b used the original userid
> for ACL checks located directly in DefineIndex(), but it still adopted
> the table owner userid for more ACL checks than intended.  That broke
> dump/reload of indexes that refer to an operator class, collation, or
> exclusion operator in a schema other than "public" or "pg_catalog".
> Back-patch to v10 (all supported versions), like the earlier commit.
> 
> Nathan Bossart and Noah Misch
> 
> Discussion: https://postgr.es/m/f8a4105f076544c180a87ef0c4822352@stmuk.bayern.de
> ---
>  contrib/citext/Makefile                      |    2 
>  contrib/citext/expected/create_index_acl.out |   81 +++++++++++++++++++++++
>  contrib/citext/sql/create_index_acl.sql      |   82 ++++++++++++++++++++++++
>  src/backend/commands/indexcmds.c             |   92 +++++++++++++++++++++++----
>  4 files changed, 244 insertions(+), 13 deletions(-)
>  create mode 100644 contrib/citext/expected/create_index_acl.out
>  create mode 100644 contrib/citext/sql/create_index_acl.sql
> 
> --- a/contrib/citext/Makefile
> +++ b/contrib/citext/Makefile
> @@ -7,7 +7,7 @@
>         citext--1.1--1.0.sql citext--unpackaged--1.0.sql
>  PGFILEDESC = "citext - case-insensitive character string data type"
>  
> -REGRESS = citext
> +REGRESS = create_index_acl citext
>  
>  ifdef USE_PGXS
>  PG_CONFIG = pg_config
> --- /dev/null
> +++ b/contrib/citext/expected/create_index_acl.out
> @@ -0,0 +1,81 @@
> +-- Each DefineIndex() ACL check uses either the original userid or the table
> +-- owner userid; see its header comment.  Here, confirm that DefineIndex()
> +-- uses its original userid where necessary.  The test works by creating
> +-- indexes that refer to as many sorts of objects as possible, with the table
> +-- owner having as few applicable privileges as possible.  (The privileges.sql
> +-- regress_sro_user tests look for the opposite defect; they confirm that
> +-- DefineIndex() uses the table owner userid where necessary.)
> +-- Don't override tablespaces; this version lacks allow_in_place_tablespaces.
> +BEGIN;
> +CREATE ROLE regress_minimal;
> +CREATE SCHEMA s;
> +CREATE EXTENSION citext SCHEMA s;
> +-- Revoke all conceivably-relevant ACLs within the extension.  The system
> +-- doesn't check all these ACLs, but this will provide some coverage if that
> +-- ever changes.
> +REVOKE ALL ON TYPE s.citext FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_lt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_le(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_eq(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_ge(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_gt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_cmp(s.citext, s.citext) FROM PUBLIC;
> +-- Functions sufficient for making an index column that has the side effect of
> +-- changing search_path at expression planning time.
> +CREATE FUNCTION public.setter() RETURNS bool VOLATILE
> +  LANGUAGE SQL AS $$SET search_path = s; SELECT true$$;
> +CREATE FUNCTION s.const() RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT public.setter()$$;
> +CREATE FUNCTION s.index_this_expr(s.citext, bool) RETURNS s.citext IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1$$;
> +REVOKE ALL ON FUNCTION public.setter() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.const() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.index_this_expr(s.citext, bool) FROM PUBLIC;
> +-- Even for an empty table, expression planning calls s.const & public.setter.
> +GRANT EXECUTE ON FUNCTION public.setter() TO regress_minimal;
> +GRANT EXECUTE ON FUNCTION s.const() TO regress_minimal;
> +-- Function for index predicate.
> +CREATE FUNCTION s.index_row_if(s.citext) RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1 IS NOT NULL$$;
> +REVOKE ALL ON FUNCTION s.index_row_if(s.citext) FROM PUBLIC;
> +-- Even for an empty table, CREATE INDEX checks ii_Predicate permissions.
> +GRANT EXECUTE ON FUNCTION s.index_row_if(s.citext) TO regress_minimal;
> +-- Non-extension, non-function objects.
> +CREATE COLLATION s.coll (LOCALE="C");
> +CREATE TABLE s.x (y s.citext);
> +ALTER TABLE s.x OWNER TO regress_minimal;
> +-- Empty-table DefineIndex()
> +CREATE UNIQUE INDEX u0rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e0rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Make the table nonempty.
> +INSERT INTO s.x VALUES ('foo'), ('bar');
> +-- If the INSERT runs the planner on index expressions, a search_path change
> +-- survives.  As of 2022-06, the INSERT reuses a cached plan.  It does so even
> +-- under debug_discard_caches, since each index is new-in-transaction.  If
> +-- future work changes a cache lifecycle, this RESET may become necessary.
> +RESET search_path;
> +-- For a nonempty table, owner needs permissions throughout ii_Expressions.
> +GRANT EXECUTE ON FUNCTION s.index_this_expr(s.citext, bool) TO regress_minimal;
> +CREATE UNIQUE INDEX u2rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e2rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Shall not find s.coll via search_path, despite the s.const->public.setter
> +-- call having set search_path=s during expression planning.  Suppress the
> +-- message itself, which depends on the database encoding.
> +\set VERBOSITY terse
> +DO $$
> +BEGIN
> +ALTER TABLE s.x ADD CONSTRAINT underqualified EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +EXCEPTION WHEN OTHERS THEN RAISE EXCEPTION '%', sqlstate; END$$;
> +ERROR:  42704
> +\set VERBOSITY default
> +ROLLBACK;
> --- /dev/null
> +++ b/contrib/citext/sql/create_index_acl.sql
> @@ -0,0 +1,82 @@
> +-- Each DefineIndex() ACL check uses either the original userid or the table
> +-- owner userid; see its header comment.  Here, confirm that DefineIndex()
> +-- uses its original userid where necessary.  The test works by creating
> +-- indexes that refer to as many sorts of objects as possible, with the table
> +-- owner having as few applicable privileges as possible.  (The privileges.sql
> +-- regress_sro_user tests look for the opposite defect; they confirm that
> +-- DefineIndex() uses the table owner userid where necessary.)
> +
> +-- Don't override tablespaces; this version lacks allow_in_place_tablespaces.
> +
> +BEGIN;
> +CREATE ROLE regress_minimal;
> +CREATE SCHEMA s;
> +CREATE EXTENSION citext SCHEMA s;
> +-- Revoke all conceivably-relevant ACLs within the extension.  The system
> +-- doesn't check all these ACLs, but this will provide some coverage if that
> +-- ever changes.
> +REVOKE ALL ON TYPE s.citext FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_lt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_le(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_eq(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_ge(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_gt(s.citext, s.citext) FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.citext_cmp(s.citext, s.citext) FROM PUBLIC;
> +-- Functions sufficient for making an index column that has the side effect of
> +-- changing search_path at expression planning time.
> +CREATE FUNCTION public.setter() RETURNS bool VOLATILE
> +  LANGUAGE SQL AS $$SET search_path = s; SELECT true$$;
> +CREATE FUNCTION s.const() RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT public.setter()$$;
> +CREATE FUNCTION s.index_this_expr(s.citext, bool) RETURNS s.citext IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1$$;
> +REVOKE ALL ON FUNCTION public.setter() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.const() FROM PUBLIC;
> +REVOKE ALL ON FUNCTION s.index_this_expr(s.citext, bool) FROM PUBLIC;
> +-- Even for an empty table, expression planning calls s.const & public.setter.
> +GRANT EXECUTE ON FUNCTION public.setter() TO regress_minimal;
> +GRANT EXECUTE ON FUNCTION s.const() TO regress_minimal;
> +-- Function for index predicate.
> +CREATE FUNCTION s.index_row_if(s.citext) RETURNS bool IMMUTABLE
> +  LANGUAGE SQL AS $$SELECT $1 IS NOT NULL$$;
> +REVOKE ALL ON FUNCTION s.index_row_if(s.citext) FROM PUBLIC;
> +-- Even for an empty table, CREATE INDEX checks ii_Predicate permissions.
> +GRANT EXECUTE ON FUNCTION s.index_row_if(s.citext) TO regress_minimal;
> +-- Non-extension, non-function objects.
> +CREATE COLLATION s.coll (LOCALE="C");
> +CREATE TABLE s.x (y s.citext);
> +ALTER TABLE s.x OWNER TO regress_minimal;
> +-- Empty-table DefineIndex()
> +CREATE UNIQUE INDEX u0rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e0rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Make the table nonempty.
> +INSERT INTO s.x VALUES ('foo'), ('bar');
> +-- If the INSERT runs the planner on index expressions, a search_path change
> +-- survives.  As of 2022-06, the INSERT reuses a cached plan.  It does so even
> +-- under debug_discard_caches, since each index is new-in-transaction.  If
> +-- future work changes a cache lifecycle, this RESET may become necessary.
> +RESET search_path;
> +-- For a nonempty table, owner needs permissions throughout ii_Expressions.
> +GRANT EXECUTE ON FUNCTION s.index_this_expr(s.citext, bool) TO regress_minimal;
> +CREATE UNIQUE INDEX u2rows ON s.x USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll s.citext_ops)
> +  WHERE s.index_row_if(y);
> +ALTER TABLE s.x ADD CONSTRAINT e2rows EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE s.coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +-- Shall not find s.coll via search_path, despite the s.const->public.setter
> +-- call having set search_path=s during expression planning.  Suppress the
> +-- message itself, which depends on the database encoding.
> +\set VERBOSITY terse
> +DO $$
> +BEGIN
> +ALTER TABLE s.x ADD CONSTRAINT underqualified EXCLUDE USING btree
> +  ((s.index_this_expr(y, s.const())) COLLATE coll WITH s.=)
> +  WHERE (s.index_row_if(y));
> +EXCEPTION WHEN OTHERS THEN RAISE EXCEPTION '%', sqlstate; END$$;
> +\set VERBOSITY default
> +ROLLBACK;
> --- a/src/backend/commands/indexcmds.c
> +++ b/src/backend/commands/indexcmds.c
> @@ -65,7 +65,10 @@
>                    Oid relId,
>                    char *accessMethodName, Oid accessMethodId,
>                    bool amcanorder,
> -                  bool isconstraint);
> +                  bool isconstraint,
> +                  Oid ddl_userid,
> +                  int ddl_sec_context,
> +                  int *ddl_save_nestlevel);
>  static Oid GetIndexOpClass(List *opclass, Oid attrType,
>                  char *accessMethodName, Oid accessMethodId);
>  static char *ChooseIndexName(const char *tabname, Oid namespaceId,
> @@ -168,8 +171,7 @@
>       * Compute the operator classes, collations, and exclusion operators for
>       * the new index, so we can test whether it's compatible with the existing
>       * one.  Note that ComputeIndexAttrs might fail here, but that's OK:
> -     * DefineIndex would have called this function with the same arguments
> -     * later on, and it would have failed then anyway.
> +     * DefineIndex would have failed later.
>       */
>      indexInfo = makeNode(IndexInfo);
>      indexInfo->ii_Expressions = NIL;
> @@ -187,7 +189,7 @@
>                        coloptions, attributeList,
>                        exclusionOpNames, relationId,
>                        accessMethodName, accessMethodId,
> -                      amcanorder, isconstraint);
> +                      amcanorder, isconstraint, InvalidOid, 0, NULL);
>  
>  
>      /* Get the soon-obsolete pg_index tuple. */
> @@ -280,6 +282,19 @@
>   * DefineIndex
>   *        Creates a new index.
>   *
> + * This function manages the current userid according to the needs of pg_dump.
> + * Recreating old-database catalog entries in new-database is fine, regardless
> + * of which users would have permission to recreate those entries now.  That's
> + * just preservation of state.  Running opaque expressions, like calling a
> + * function named in a catalog entry or evaluating a pg_node_tree in a catalog
> + * entry, as anyone other than the object owner, is not fine.  To adhere to
> + * those principles and to remain fail-safe, use the table owner userid for
> + * most ACL checks.  Use the original userid for ACL checks reached without
> + * traversing opaque expressions.  (pg_dump can predict such ACL checks from
> + * catalogs.)  Overall, this is a mess.  Future DDL development should
> + * consider offering one DDL command for catalog setup and a separate DDL
> + * command for steps that run opaque expressions.
> + *
>   * 'relationId': the OID of the heap relation on which the index is to be
>   *        created
>   * 'stmt': IndexStmt describing the properties of the new index.
> @@ -581,7 +596,8 @@
>                        coloptions, stmt->indexParams,
>                        stmt->excludeOpNames, relationId,
>                        accessMethodName, accessMethodId,
> -                      amcanorder, stmt->isconstraint);
> +                      amcanorder, stmt->isconstraint, root_save_userid,
> +                      root_save_sec_context, &root_save_nestlevel);
>  
>      /*
>       * Extra checks when creating a PRIMARY KEY index.
> @@ -639,9 +655,8 @@
>  
>      /*
>       * Roll back any GUC changes executed by index functions, and keep
> -     * subsequent changes local to this command.  It's barely possible that
> -     * some index function changed a behavior-affecting GUC, e.g. xmloption,
> -     * that affects subsequent steps.
> +     * subsequent changes local to this command.  This is essential if some
> +     * index function changed a behavior-affecting GUC, e.g. search_path.
>       */
>      AtEOXact_GUC(false, root_save_nestlevel);
>      root_save_nestlevel = NewGUCNestLevel();
> @@ -996,6 +1011,10 @@
>  /*
>   * Compute per-index-column information, including indexed column numbers
>   * or index expressions, opclasses, and indoptions.
> + *
> + * If the caller switched to the table owner, ddl_userid is the role for ACL
> + * checks reached without traversing opaque expressions.  Otherwise, it's
> + * InvalidOid, and other ddl_* arguments are undefined.
>   */
>  static void
>  ComputeIndexAttrs(IndexInfo *indexInfo,
> @@ -1009,11 +1028,16 @@
>                    char *accessMethodName,
>                    Oid accessMethodId,
>                    bool amcanorder,
> -                  bool isconstraint)
> +                  bool isconstraint,
> +                  Oid ddl_userid,
> +                  int ddl_sec_context,
> +                  int *ddl_save_nestlevel)
>  {
>      ListCell   *nextExclOp;
>      ListCell   *lc;
>      int            attn;
> +    Oid            save_userid;
> +    int            save_sec_context;
>  
>      /* Allocate space for exclusion operator info, if needed */
>      if (exclusionOpNames)
> @@ -1029,6 +1053,9 @@
>      else
>          nextExclOp = NULL;
>  
> +    if (OidIsValid(ddl_userid))
> +        GetUserIdAndSecContext(&save_userid, &save_sec_context);
> +
>      /*
>       * process attributeList
>       */
> @@ -1123,10 +1150,24 @@
>          typeOidP[attn] = atttype;
>  
>          /*
> -         * Apply collation override if any
> +         * Apply collation override if any.  Use of ddl_userid is necessary
> +         * due to ACL checks therein, and it's safe because collations don't
> +         * contain opaque expressions (or non-opaque expressions).
>           */
>          if (attribute->collation)
> +        {
> +            if (OidIsValid(ddl_userid))
> +            {
> +                AtEOXact_GUC(false, *ddl_save_nestlevel);
> +                SetUserIdAndSecContext(ddl_userid, ddl_sec_context);
> +            }
>              attcollation = get_collation_oid(attribute->collation, false);
> +            if (OidIsValid(ddl_userid))
> +            {
> +                SetUserIdAndSecContext(save_userid, save_sec_context);
> +                *ddl_save_nestlevel = NewGUCNestLevel();
> +            }
> +        }
>  
>          /*
>           * Check we have a collation iff it's a collatable type.  The only
> @@ -1154,12 +1195,25 @@
>          collationOidP[attn] = attcollation;
>  
>          /*
> -         * Identify the opclass to use.
> +         * Identify the opclass to use.  Use of ddl_userid is necessary due to
> +         * ACL checks therein.  This is safe despite opclasses containing
> +         * opaque expressions (specifically, functions), because only
> +         * superusers can define opclasses.
>           */
> +        if (OidIsValid(ddl_userid))
> +        {
> +            AtEOXact_GUC(false, *ddl_save_nestlevel);
> +            SetUserIdAndSecContext(ddl_userid, ddl_sec_context);
> +        }
>          classOidP[attn] = GetIndexOpClass(attribute->opclass,
>                                            atttype,
>                                            accessMethodName,
>                                            accessMethodId);
> +        if (OidIsValid(ddl_userid))
> +        {
> +            SetUserIdAndSecContext(save_userid, save_sec_context);
> +            *ddl_save_nestlevel = NewGUCNestLevel();
> +        }
>  
>          /*
>           * Identify the exclusion operator, if any.
> @@ -1173,9 +1227,23 @@
>  
>              /*
>               * Find the operator --- it must accept the column datatype
> -             * without runtime coercion (but binary compatibility is OK)
> +             * without runtime coercion (but binary compatibility is OK).
> +             * Operators contain opaque expressions (specifically, functions).
> +             * compatible_oper_opid() boils down to oper() and
> +             * IsBinaryCoercible().  PostgreSQL would have security problems
> +             * elsewhere if oper() started calling opaque expressions.
>               */
> +            if (OidIsValid(ddl_userid))
> +            {
> +                AtEOXact_GUC(false, *ddl_save_nestlevel);
> +                SetUserIdAndSecContext(ddl_userid, ddl_sec_context);
> +            }
>              opid = compatible_oper_opid(opname, atttype, atttype, false);
> +            if (OidIsValid(ddl_userid))
> +            {
> +                SetUserIdAndSecContext(save_userid, save_sec_context);
> +                *ddl_save_nestlevel = NewGUCNestLevel();
> +            }
>  
>              /*
>               * Only allow commutative operators to be used in exclusion


-- 
Roberto C. Sánchez
http://people.connexer.com/~roberto
http://www.connexer.com