From 7cb7ff28dded581ebefcadac7d043f8b123462b8 Mon Sep 17 00:00:00 2001 From: reshke Date: Mon, 10 Aug 2026 07:27:18 +0000 Subject: [PATCH v2] Fix circular grant bug When checking for circular priviledges, use aclmask_direct to get grantor's independently-derived privileges, not via another roles membership --- src/backend/utils/adt/acl.c | 18 ++++++++++----- src/test/regress/expected/privileges.out | 26 +++++++++++++++++++++ src/test/regress/sql/privileges.sql | 29 ++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index e2547d719ed..56c394f5a42 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -98,6 +98,8 @@ static void check_acl(const Acl *acl); static const char *aclparse(const char *s, AclItem *aip, Node *escontext); static bool aclitem_match(const AclItem *a1, const AclItem *a2); static int aclitemComparator(const void *arg1, const void *arg2); +static AclMode aclmask_direct(const Acl *acl, Oid roleid, Oid ownerId, + AclMode mask, AclMaskHow how); 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, @@ -1297,12 +1299,16 @@ cc_restart: } } - /* Now we can compute grantor's independently-derived privileges */ - own_privs = aclmask(acl, - mod_aip->ai_grantor, - ownerId, - ACL_GRANT_OPTION_FOR(ACLITEM_GET_GOPTIONS(*mod_aip)), - ACLMASK_ALL); + /* + * Now we can compute grantor's independently-derived privileges. + * We use aclmask_direct here to filter-out inherited priviledges, so + * this check remains true after role beign revoked from other role. + */ + own_privs = aclmask_direct(acl, + mod_aip->ai_grantor, + ownerId, + ACL_GRANT_OPTION_FOR(ACLITEM_GET_GOPTIONS(*mod_aip)), + ACLMASK_ALL); own_privs = ACL_OPTION_TO_PRIVS(own_privs); if ((ACLITEM_GET_GOPTIONS(*mod_aip) & ~own_privs) != 0) diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 5e3c9510490..d3600b79077 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -13,6 +13,9 @@ DROP ROLE IF EXISTS regress_priv_user4; DROP ROLE IF EXISTS regress_priv_user5; DROP ROLE IF EXISTS regress_priv_user6; DROP ROLE IF EXISTS regress_priv_user7; +DROP ROLE IF EXISTS regress_priv_circ_r1; +DROP ROLE IF EXISTS regress_priv_circ_r2; +DROP ROLE IF EXISTS regress_priv_circ_r3; SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid; lo_unlink ----------- @@ -1896,6 +1899,29 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OP t (1 row) +-- A grant option held only indirectly (via role membership) must not be +-- grantable back to the grantor, else revoking the membership leaves a +-- circular grant that pg_dump/pg_restore cannot reproduce. +RESET SESSION AUTHORIZATION; +CREATE ROLE regress_priv_circ_r1 LOGIN; +CREATE ROLE regress_priv_circ_r2 LOGIN; +CREATE ROLE regress_priv_circ_r3; +GRANT regress_priv_circ_r3 TO regress_priv_circ_r2; +GRANT CREATE ON SCHEMA public TO regress_priv_circ_r1; +SET ROLE regress_priv_circ_r1; +CREATE VIEW regress_priv_circ_v AS SELECT; +GRANT SELECT ON regress_priv_circ_v TO regress_priv_circ_r2 WITH GRANT OPTION; +GRANT SELECT ON regress_priv_circ_v TO regress_priv_circ_r3 WITH GRANT OPTION; +SET ROLE regress_priv_circ_r2; +-- r2 only inherits the grant option from r3, so it may not grant it to itself +GRANT SELECT ON regress_priv_circ_v TO regress_priv_circ_r2 WITH GRANT OPTION; -- fail +ERROR: grant options cannot be granted back to your own grantor +RESET ROLE; +DROP VIEW regress_priv_circ_v; +REVOKE CREATE ON SCHEMA public FROM regress_priv_circ_r1; +DROP ROLE regress_priv_circ_r1; +DROP ROLE regress_priv_circ_r2; +DROP ROLE regress_priv_circ_r3; -- security-restricted operations \c - CREATE ROLE regress_sro_user; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index d3e87fa617f..94806939141 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -17,6 +17,9 @@ DROP ROLE IF EXISTS regress_priv_user4; DROP ROLE IF EXISTS regress_priv_user5; DROP ROLE IF EXISTS regress_priv_user6; DROP ROLE IF EXISTS regress_priv_user7; +DROP ROLE IF EXISTS regress_priv_circ_r1; +DROP ROLE IF EXISTS regress_priv_circ_r2; +DROP ROLE IF EXISTS regress_priv_circ_r3; SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3000 ORDER BY oid; @@ -1222,6 +1225,32 @@ SELECT has_table_privilege('regress_priv_user3', 'atest4', 'SELECT'); -- false SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true +-- A grant option held only indirectly (via role membership) must not be +-- grantable back to the grantor, else revoking the membership leaves a +-- circular grant that pg_dump/pg_restore cannot reproduce. +RESET SESSION AUTHORIZATION; +CREATE ROLE regress_priv_circ_r1 LOGIN; +CREATE ROLE regress_priv_circ_r2 LOGIN; +CREATE ROLE regress_priv_circ_r3; +GRANT regress_priv_circ_r3 TO regress_priv_circ_r2; +GRANT CREATE ON SCHEMA public TO regress_priv_circ_r1; + +SET ROLE regress_priv_circ_r1; +CREATE VIEW regress_priv_circ_v AS SELECT; +GRANT SELECT ON regress_priv_circ_v TO regress_priv_circ_r2 WITH GRANT OPTION; +GRANT SELECT ON regress_priv_circ_v TO regress_priv_circ_r3 WITH GRANT OPTION; + +SET ROLE regress_priv_circ_r2; +-- r2 only inherits the grant option from r3, so it may not grant it to itself +GRANT SELECT ON regress_priv_circ_v TO regress_priv_circ_r2 WITH GRANT OPTION; -- fail + +RESET ROLE; +DROP VIEW regress_priv_circ_v; +REVOKE CREATE ON SCHEMA public FROM regress_priv_circ_r1; +DROP ROLE regress_priv_circ_r1; +DROP ROLE regress_priv_circ_r2; +DROP ROLE regress_priv_circ_r3; + -- security-restricted operations \c - -- 2.50.1 (Apple Git-155)