From fea164a72a7bfd50d77ba5fb418d357f8f2bb7d0 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 17 Feb 2014 09:33:31 -0500 Subject: Shore up ADMIN OPTION restrictions. Granting a role without ADMIN OPTION is supposed to prevent the grantee from adding or removing members from the granted role. Issuing SET ROLE before the GRANT bypassed that, because the role itself had an implicit right to add or remove members. Plug that hole by recognizing that implicit right only when the session user matches the current role. Additionally, do not recognize it during a security-restricted operation or during execution of a SECURITY DEFINER function. The restriction on SECURITY DEFINER is not security-critical. However, it seems best for a user testing his own SECURITY DEFINER function to see the same behavior others will see. Back-patch to 8.4 (all supported versions). The SQL standards do not conflate roles and users as PostgreSQL does; only SQL roles have members, and only SQL users initiate sessions. An application using PostgreSQL users and roles as SQL users and roles will never attempt to grant membership in the role that is the session user, so the implicit right to add or remove members will never arise. The security impact was mostly that a role member could revoke access from others, contrary to the wishes of his own grantor. Unapproved role member additions are less notable, because the member can still largely achieve that by creating a view or a SECURITY DEFINER function. Reviewed by Andres Freund and Tom Lane. Reported, independently, by Jonas Sundman and Noah Misch. Security: CVE-2014-0060 --- src/backend/commands/user.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'src/backend/commands/user.c') diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index bcdc392a817..7f5b8473d81 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -1366,7 +1366,16 @@ AddRoleMems(const char *rolename, Oid roleid, rolename))); } - /* XXX not sure about this check */ + /* + * The role membership grantor of record has little significance at + * present. Nonetheless, inasmuch as users might look to it for a crude + * audit trail, let only superusers impute the grant to a third party. + * + * Before lifting this restriction, give the member == role case of + * is_admin_of_role() a fresh look. Ensure that the current role cannot + * use an explicit grantor specification to take advantage of the session + * user's self-admin right. + */ if (grantorId != GetUserId() && !superuser()) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), -- cgit v1.2.3