Fix search_path for all maintenance commands

Jeff Davis <pgsql@j-davis.com>

From: Jeff Davis <pgsql@j-davis.com>
To: pgsql-hackers@postgresql.org
Cc: Nathan Bossart <nathandbossart@gmail.com>
Date: 2023-05-26T23:21:50Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix search_path to a safe value during maintenance operations.

  2. Make relation-enumerating operations be security-restricted operations.

Attachments

Maintenance commands (ANALYZE, CLUSTER, REFRESH MATERIALIZED VIEW,
REINDEX, and VACUUM) currently run as the table owner, and as a
SECURITY_RESTRICTED_OPERATION.

I propose that we also fix the search_path to "pg_catalog, pg_temp"
when running maintenance commands, for two reasons:

1. Make the behavior of maintenance commands more consistent because
they'd always have the same search_path.

2. Now that we have the MAINTAIN privilege in 16, privileged non-
superusers can execute maintenance commands on other users' tables.
That raises the possibility that a user with MAINTAIN privilege may be
able to use search_path tricks to escalate privileges to the table
owner. The MAINTAIN privilege is only given to highly-privileged users,
but there's still some risk. For this reason I also propose that it
goes in v16.


There's one interesting part: in the code path for creating a
materialized view, ExecCreateTableAs() has this comment:

/*                                                                    
 * For materialized views, lock down security-restricted operations and
 * arrange to make GUC variable changes local to this command.  This is
 * not necessary for security, but this keeps the behavior similar to 
 * REFRESH MATERIALIZED VIEW.  Otherwise, one could create a
materialized                                                      
 * view not possible to refresh.                                      
 */

My patch doesn't address this ExecCreateTableAs() check. To do so, we
would need to set the search path after DefineRelation(), otherwise it
will try to create the object in pg_catalog. But DefineRelation() is
happening at execution time, well after we entered the
SECURITY_RESTRICTED_OPERATION, and it doesn't seem good to separate the
SECURITY_RESTRICTED_OPERATION from setting search_path.

This ExecCreateTableAs() check doesn't seem terribly important, so I
don't think it's necessary to improve it as a part of this patch (it
won't be perfect anyway: functions can behave inconsistently for all
kinds of reasons). But I'm open to suggestion if someone knows a good
way to do it.


-- 
Jeff Davis
PostgreSQL Contributor Team - AWS