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

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

From

Roberto C. Sánchez

Date:

08 June 2022, 19:04:04

Hello Devs,

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.

The first thing I did was to adapt the full patches, with functional
changes and regression tests.  Since amcheck was new to version 10, I
dropped that part of the patch.  Additionally, since partitioned tables
were new in 10 I dropped those parts of the tests.  The absence of block
range indices in 9.4 means I also dropped that part of the change and
associated test as well.

Once everything built successfully, I built again with only the
regression tests to confirm that the vulnerability was presented and
triggerred by the regression test [*].

When building with only the adapted regression tests, the 9.6 build
failed with this in the test output:

+ ERROR:  sro_ifun(10) called by pbuilder
+ CONTEXT:  PL/pgSQL function sro_ifun(integer) line 4 at ASSERT

This seems to indicate that the vulnerability was encountered and that
the function was called as the invoking user rather than the owning
user.  Naturally, there were further differneces in the test output
owing to the index creation failure.

For 9.4, the error looked like this:

+ ERROR:  called by pbuilder

As a result of ASSERT not being present in 9.4 I had to resort to an IF
statement with a RAISE.  However, it appears to be the identical
problem.

There are 4 patches attached to this mail, one for each of the two
commits referenced above as adapted for 9.6 and 9.4.  Please advise on
whether adjustments are needed or whether I can proceed with publishing
updated 9.6 and 9.4 packages for Debian with said patches.

Regards,

-Roberto

[*] Side note: my approach revealed that the adapted regression tests
trigger the vulnerability in both 9.6 and 9.4.  However, the SUSE
security information page for CVE-2022-1552 [0] lists 9.6 as "not
affected".  Presumably this is based on the language in the upstream
advisory "Versions Affected: 10 - 14."

[0] https://www.suse.com/security/cve/CVE-2022-1552.html

-- 
Roberto C. Sánchez

Attachment

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

From

Tom Lane

Date:

08 June 2022, 23:15:47

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

We're going to have to tweak that code somehow, and it's not yet
entirely clear how.

            regards, tom lane

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

From

Roberto C. Sánchez

Date:

09 June 2022, 00:31:09

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.

Regards,

-Roberto
-- 
Roberto C. Sánchez

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

From

Roberto C. Sánchez

Date:

05 July 2022, 01:06:51

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

Attachment

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

From

Roberto C. Sánchez

Date:

27 July 2022, 18:52:47

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