From 1a0c40e4257d6b1fd19d850761dbbcc21800aef7 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Thu, 25 Jun 2026 11:18:20 -0700
Subject: [PATCH v1] Prevent broken grant chains when indirect grant options
 are held

recursive_revoke() is meant to protect against cases where a grantor
loses their grant option after issuing dependent grants to other roles.
In that case, the user must specify CASCADE to prune the dependent ACLs.

However, if a grantor happens to hold grant options (or table ownership)
indirectly, whether via INHERIT or SUPERUSER, recursive_revoke()
short-circuits and allows the REVOKE to break the grant chain. Symptoms
include general user confusion as well as problems during dump/restore,
which cannot recreate the original state.

To fix, switch from aclmask() to aclmask_direct().

Discussion: https://postgr.es/m/CAOYmi%2B%3DKTLd%2BXsEP%3DTDiZ48iVf-CEc7JrZd5uhWPYWKEfOgyyQ%40mail.gmail.com
Backpatch-through: 14
---
 src/backend/utils/adt/acl.c              | 19 +++--
 src/test/regress/expected/privileges.out | 88 ++++++++++++++++++++++++
 src/test/regress/sql/privileges.sql      | 44 ++++++++++++
 3 files changed, 147 insertions(+), 4 deletions(-)

diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 01caa12eca7..21a874392d9 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -102,6 +102,8 @@ static void check_circularity(const Acl *old_acl, const AclItem *mod_aip,
 							  Oid ownerId);
 static Acl *recursive_revoke(Acl *acl, Oid grantee, AclMode revoke_privs,
 							 Oid ownerId, DropBehavior behavior);
+static AclMode aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId,
+							  AclMode mask, AclMaskHow how);
 
 static AclMode convert_any_priv_string(text *priv_type_text,
 									   const priv_map *privileges);
@@ -1344,10 +1346,19 @@ recursive_revoke(Acl *acl,
 	if (grantee == ownerId)
 		return acl;
 
-	/* The grantee might still have some grant options via another grantor */
-	still_has = aclmask(acl, grantee, ownerId,
-						ACL_GRANT_OPTION_FOR(revoke_privs),
-						ACLMASK_ALL);
+	/*
+	 * The grantee might still have some grant options via another grantor,
+	 * keeping the chain intact for those privileges. And if _all_ of the
+	 * remaining privileges can still be granted by any role on the chain,
+	 * we're done.
+	 *
+	 * Indirectly held grant options cannot keep the chain alive, so consult
+	 * aclmask_direct() rather than aclmask(). (Otherwise, for example, a
+	 * chain could be broken wherever the grantee was a superuser).
+	 */
+	still_has = aclmask_direct(acl, grantee, ownerId,
+							   ACL_GRANT_OPTION_FOR(revoke_privs),
+							   ACLMASK_ALL);
 	revoke_privs &= ~ACL_OPTION_TO_PRIVS(still_has);
 	if (revoke_privs == ACL_NO_RIGHTS)
 		return acl;
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index f6cc1a1029c..6840ff950c6 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -1862,6 +1862,8 @@ CREATE TABLE atest4 (a int);
 GRANT SELECT ON atest4 TO regress_priv_user2 WITH GRANT OPTION;
 GRANT UPDATE ON atest4 TO regress_priv_user2;
 GRANT SELECT ON atest4 TO GROUP regress_priv_group1 WITH GRANT OPTION;
+GRANT SELECT ON atest4 TO PUBLIC WITH GRANT OPTION; -- fail
+ERROR:  grant options can only be granted to roles
 SET SESSION AUTHORIZATION regress_priv_user2;
 GRANT SELECT ON atest4 TO regress_priv_user3;
 GRANT UPDATE ON atest4 TO regress_priv_user3; -- fail
@@ -1896,6 +1898,91 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OP
  t
 (1 row)
 
+-- Test GRANT OPTION chains where the intermediate grantor has the privileges of
+-- the owner (regress_priv_group1 -> regress_priv_user4 -> regress_priv_user2)
+-- or inherits the option (regress_priv_group2 -> regress_priv_user1 ->
+-- regress_priv_user3).
+-- pin the group membership that we rely on below
+SELECT pg_has_role('regress_priv_user4', 'regress_priv_group1', 'MEMBER'),
+	   pg_has_role('regress_priv_user1', 'regress_priv_group2', 'MEMBER');
+ pg_has_role | pg_has_role 
+-------------+-------------
+ t           | t
+(1 row)
+
+SET SESSION AUTHORIZATION regress_priv_group1;
+CREATE TABLE atest4_groupowned (a int);
+GRANT SELECT ON atest4_groupowned TO regress_priv_user4 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_group2 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user1 WITH GRANT OPTION;
+SET SESSION AUTHORIZATION regress_priv_user4;
+GRANT SELECT ON atest4_groupowned TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_user1;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user3;
+SET SESSION AUTHORIZATION regress_priv_group1;
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4; -- fail
+ERROR:  dependent privileges exist
+HINT:  Use CASCADE to revoke them too.
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- true
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- true
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- false
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- still true (inherited)
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1; -- fail
+ERROR:  dependent privileges exist
+HINT:  Use CASCADE to revoke them too.
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- true
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- true
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- false
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- still true (inherited)
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_group2 RESTRICT; -- ok
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- false
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
 -- security-restricted operations
 \c -
 CREATE ROLE regress_sro_user;
@@ -3296,6 +3383,7 @@ DROP TABLE atest1;
 DROP TABLE atest2;
 DROP TABLE atest3;
 DROP TABLE atest4;
+DROP TABLE atest4_groupowned;
 DROP TABLE atest5;
 DROP TABLE atest6;
 DROP TABLE atestc;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 6cd9bb840ff..c9f80ba898b 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -1205,6 +1205,7 @@ CREATE TABLE atest4 (a int);
 GRANT SELECT ON atest4 TO regress_priv_user2 WITH GRANT OPTION;
 GRANT UPDATE ON atest4 TO regress_priv_user2;
 GRANT SELECT ON atest4 TO GROUP regress_priv_group1 WITH GRANT OPTION;
+GRANT SELECT ON atest4 TO PUBLIC WITH GRANT OPTION; -- fail
 
 SET SESSION AUTHORIZATION regress_priv_user2;
 
@@ -1222,6 +1223,48 @@ SELECT has_table_privilege('regress_priv_user3', 'atest4', 'SELECT'); -- false
 
 SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
 
+-- Test GRANT OPTION chains where the intermediate grantor has the privileges of
+-- the owner (regress_priv_group1 -> regress_priv_user4 -> regress_priv_user2)
+-- or inherits the option (regress_priv_group2 -> regress_priv_user1 ->
+-- regress_priv_user3).
+
+-- pin the group membership that we rely on below
+SELECT pg_has_role('regress_priv_user4', 'regress_priv_group1', 'MEMBER'),
+	   pg_has_role('regress_priv_user1', 'regress_priv_group2', 'MEMBER');
+
+SET SESSION AUTHORIZATION regress_priv_group1;
+
+CREATE TABLE atest4_groupowned (a int);
+
+GRANT SELECT ON atest4_groupowned TO regress_priv_user4 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_group2 WITH GRANT OPTION;
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user1 WITH GRANT OPTION;
+
+SET SESSION AUTHORIZATION regress_priv_user4;
+
+GRANT SELECT ON atest4_groupowned TO regress_priv_user2;
+
+SET SESSION AUTHORIZATION regress_priv_user1;
+
+GRANT UPDATE ON atest4_groupowned TO regress_priv_user3;
+
+SET SESSION AUTHORIZATION regress_priv_group1;
+
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4; -- fail
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- true
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- true
+REVOKE SELECT ON atest4_groupowned FROM regress_priv_user4 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user2', 'atest4_groupowned', 'SELECT'); -- false
+SELECT has_table_privilege('regress_priv_user4', 'atest4_groupowned', 'SELECT'); -- still true (inherited)
+
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1; -- fail
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- true
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- true
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_user1 CASCADE; -- ok
+SELECT has_table_privilege('regress_priv_user3', 'atest4_groupowned', 'UPDATE'); -- false
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- still true (inherited)
+REVOKE UPDATE ON atest4_groupowned FROM regress_priv_group2 RESTRICT; -- ok
+SELECT has_table_privilege('regress_priv_user1', 'atest4_groupowned', 'UPDATE'); -- false
 
 -- security-restricted operations
 \c -
@@ -1955,6 +1998,7 @@ DROP TABLE atest1;
 DROP TABLE atest2;
 DROP TABLE atest3;
 DROP TABLE atest4;
+DROP TABLE atest4_groupowned;
 DROP TABLE atest5;
 DROP TABLE atest6;
 DROP TABLE atestc;
-- 
2.34.1

