From c3d8de0907b53e3ce8988c71181edb6bcecd5aa6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 4 Nov 2005 17:25:15 +0000 Subject: [PATCH] Disregard superuserness when checking to see if a role GRANT would create circularity of role memberships. This is a minimum-impact fix for the problem reported by Florian Pflug. I thought about removing the superuser_arg test from is_member_of_role() altogether, as it seems redundant for many of the callers --- but not all, and it's way too late in the 8.1 cycle to be making large changes. Perhaps reconsider this later. --- src/backend/commands/user.c | 7 ++++--- src/backend/utils/adt/acl.c | 22 +++++++++++++++++++++- src/include/utils/acl.h | 3 ++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index d26b3f8c8c..91befbc6ab 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.163 2005/10/29 00:31:51 petere Exp $ + * $PostgreSQL: pgsql/src/backend/commands/user.c,v 1.164 2005/11/04 17:25:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1214,9 +1214,10 @@ AddRoleMems(const char *rolename, Oid roleid, * Refuse creation of membership loops, including the trivial case * where a role is made a member of itself. We do this by checking to * see if the target role is already a member of the proposed member - * role. + * role. We have to ignore possible superuserness, however, else we + * could never grant membership in a superuser-privileged role. */ - if (is_member_of_role(roleid, memberid)) + if (is_member_of_role_nosuper(roleid, memberid)) ereport(ERROR, (errcode(ERRCODE_INVALID_GRANT_OPERATION), (errmsg("role \"%s\" is a member of role \"%s\"", diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 5fcb9b25fc..6d1402356e 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.126 2005/10/15 02:49:27 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.127 2005/11/04 17:25:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3067,6 +3067,26 @@ check_is_member_of_role(Oid member, Oid role) GetUserNameFromId(role)))); } +/* + * Is member a member of role, not considering superuserness? + * + * This is identical to is_member_of_role except we ignore superuser + * status. + */ +bool +is_member_of_role_nosuper(Oid member, Oid role) +{ + /* Fast path for simple case */ + if (member == role) + return true; + + /* + * Find all the roles that member is a member of, including multi-level + * recursion, then see if target role is any one of them. + */ + return list_member_oid(roles_is_member_of(member), role); +} + /* * Is member an admin of role (directly or indirectly)? That is, is it diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index c02cc34218..da4c6baa80 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.85 2005/10/15 02:49:46 momjian Exp $ + * $PostgreSQL: pgsql/src/include/utils/acl.h,v 1.86 2005/11/04 17:25:15 tgl Exp $ * * NOTES * An ACL array is simply an array of AclItems, representing the union @@ -212,6 +212,7 @@ extern int aclmembers(const Acl *acl, Oid **roleids); extern bool has_privs_of_role(Oid member, Oid role); extern bool is_member_of_role(Oid member, Oid role); +extern bool is_member_of_role_nosuper(Oid member, Oid role); extern bool is_admin_of_role(Oid member, Oid role); extern void check_is_member_of_role(Oid member, Oid role);