Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5d320a1

Browse files
committed
Shore up ADMIN OPTION restrictions.
Granting a role without ADMIN OPTION is supposed to prevent the granteefrom adding or removing members from the granted role. Issuing SET ROLEbefore the GRANT bypassed that, because the role itself had an implicitright to add or remove members. Plug that hole by recognizing thatimplicit right only when the session user matches the current role.Additionally, do not recognize it during a security-restricted operationor during execution of a SECURITY DEFINER function. The restriction onSECURITY DEFINER is not security-critical. However, it seems best for auser testing his own SECURITY DEFINER function to see the same behaviorothers 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. Anapplication using PostgreSQL users and roles as SQL users and roles willnever 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 accessfrom others, contrary to the wishes of his own grantor. Unapproved rolemember additions are less notable, because the member can still largelyachieve that by creating a view or a SECURITY DEFINER function.Reviewed by Andres Freund and Tom Lane. Reported, independently, byJonas Sundman and Noah Misch.Security:CVE-2014-0060
1 parent3b4db30 commit5d320a1

File tree

5 files changed

+120
-18
lines changed

5 files changed

+120
-18
lines changed

‎doc/src/sgml/ref/grant.sgml

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -378,11 +378,13 @@ GRANT <replaceable class="PARAMETER">role_name</replaceable> [, ...] TO <replace
378378
<para>
379379
If <literal>WITH ADMIN OPTION</literal> is specified, the member can
380380
in turn grant membership in the role to others, and revoke membership
381-
in the role as well. Without the admin option, ordinary users cannot do
382-
that. However,
383-
database superusers can grant or revoke membership in any role to anyone.
384-
Roles having <literal>CREATEROLE</> privilege can grant or revoke
385-
membership in any role that is not a superuser.
381+
in the role as well. Without the admin option, ordinary users cannot
382+
do that. A role is not considered to hold <literal>WITH ADMIN
383+
OPTION</literal> on itself, but it may grant or revoke membership in
384+
itself from a database session where the session user matches the
385+
role. Database superusers can grant or revoke membership in any role
386+
to anyone. Roles having <literal>CREATEROLE</> privilege can grant
387+
or revoke membership in any role that is not a superuser.
386388
</para>
387389

388390
<para>

‎src/backend/commands/user.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,16 @@ AddRoleMems(const char *rolename, Oid roleid,
13301330
rolename)));
13311331
}
13321332

1333-
/* XXX not sure about this check */
1333+
/*
1334+
* The role membership grantor of record has little significance at
1335+
* present. Nonetheless, inasmuch as users might look to it for a crude
1336+
* audit trail, let only superusers impute the grant to a third party.
1337+
*
1338+
* Before lifting this restriction, give the member == role case of
1339+
* is_admin_of_role() a fresh look. Ensure that the current role cannot
1340+
* use an explicit grantor specification to take advantage of the session
1341+
* user's self-admin right.
1342+
*/
13341343
if (grantorId!=GetUserId()&& !superuser())
13351344
ereport(ERROR,
13361345
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),

‎src/backend/utils/adt/acl.c

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4314,6 +4314,11 @@ pg_role_aclcheck(Oid role_oid, Oid roleid, AclMode mode)
43144314
{
43154315
if (mode&ACL_GRANT_OPTION_FOR(ACL_CREATE))
43164316
{
4317+
/*
4318+
* XXX For roleid == role_oid, is_admin_of_role() also examines the
4319+
* session and call stack. That suits two-argument pg_has_role(), but
4320+
* it gives the three-argument version a lamentable whimsy.
4321+
*/
43174322
if (is_admin_of_role(roleid,role_oid))
43184323
returnACLCHECK_OK;
43194324
}
@@ -4631,11 +4636,9 @@ is_member_of_role_nosuper(Oid member, Oid role)
46314636

46324637

46334638
/*
4634-
* Is member an admin of role (directly or indirectly)? That is, is it
4635-
* a member WITH ADMIN OPTION?
4636-
*
4637-
* We could cache the result as for is_member_of_role, but currently this
4638-
* is not used in any performance-critical paths, so we don't.
4639+
* Is member an admin of role? That is, is member the role itself (subject to
4640+
* restrictions below), a member (directly or indirectly) WITH ADMIN OPTION,
4641+
* or a superuser?
46394642
*/
46404643
bool
46414644
is_admin_of_role(Oidmember,Oidrole)
@@ -4644,14 +4647,41 @@ is_admin_of_role(Oid member, Oid role)
46444647
List*roles_list;
46454648
ListCell*l;
46464649

4647-
/* Fast path for simple case */
4648-
if (member==role)
4649-
return true;
4650-
4651-
/* Superusers have every privilege, so are part of every role */
46524650
if (superuser_arg(member))
46534651
return true;
46544652

4653+
if (member==role)
4654+
/*
4655+
* A role can admin itself when it matches the session user and we're
4656+
* outside any security-restricted operation, SECURITY DEFINER or
4657+
* similar context. SQL-standard roles cannot self-admin. However,
4658+
* SQL-standard users are distinct from roles, and they are not
4659+
* grantable like roles: PostgreSQL's role-user duality extends the
4660+
* standard. Checking for a session user match has the effect of
4661+
* letting a role self-admin only when it's conspicuously behaving
4662+
* like a user. Note that allowing self-admin under a mere SET ROLE
4663+
* would make WITH ADMIN OPTION largely irrelevant; any member could
4664+
* SET ROLE to issue the otherwise-forbidden command.
4665+
*
4666+
* Withholding self-admin in a security-restricted operation prevents
4667+
* object owners from harnessing the session user identity during
4668+
* administrative maintenance. Suppose Alice owns a database, has
4669+
* issued "GRANT alice TO bob", and runs a daily ANALYZE. Bob creates
4670+
* an alice-owned SECURITY DEFINER function that issues "REVOKE alice
4671+
* FROM carol". If he creates an expression index calling that
4672+
* function, Alice will attempt the REVOKE during each ANALYZE.
4673+
* Checking InSecurityRestrictedOperation() thwarts that attack.
4674+
*
4675+
* Withholding self-admin in SECURITY DEFINER functions makes their
4676+
* behavior independent of the calling user. There's no security or
4677+
* SQL-standard-conformance need for that restriction, though.
4678+
*
4679+
* A role cannot have actual WITH ADMIN OPTION on itself, because that
4680+
* would imply a membership loop. Therefore, we're done either way.
4681+
*/
4682+
returnmember==GetSessionUserId()&&
4683+
!InLocalUserIdChange()&& !InSecurityRestrictedOperation();
4684+
46554685
/*
46564686
* Find all the roles that member is a member of, including multi-level
46574687
* recursion. We build a list in the same way that is_member_of_role does

‎src/test/regress/expected/privileges.out

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ ALTER GROUP regressgroup1 ADD USER regressuser4;
3232
ALTER GROUP regressgroup2 ADD USER regressuser2;-- duplicate
3333
NOTICE: role "regressuser2" is already a member of role "regressgroup2"
3434
ALTER GROUP regressgroup2 DROP USER regressuser2;
35-
ALTER GROUPregressgroup2ADD USERregressuser4;
35+
GRANTregressgroup2TOregressuser4 WITH ADMIN OPTION;
3636
-- test owner privileges
3737
SET SESSION AUTHORIZATION regressuser1;
3838
SELECT session_user, current_user;
@@ -832,6 +832,40 @@ SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION')
832832
t
833833
(1 row)
834834

835+
-- Admin options
836+
SET SESSION AUTHORIZATION regressuser4;
837+
CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS
838+
'GRANT regressgroup2 TO regressuser5';
839+
GRANT regressgroup2 TO regressuser5; -- ok: had ADMIN OPTION
840+
SET ROLE regressgroup2;
841+
GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE suspended privilege
842+
ERROR: must have admin option on role "regressgroup2"
843+
SET SESSION AUTHORIZATION regressuser1;
844+
GRANT regressgroup2 TO regressuser5; -- fails: no ADMIN OPTION
845+
ERROR: must have admin option on role "regressgroup2"
846+
SELECT dogrant_ok();-- ok: SECURITY DEFINER conveys ADMIN
847+
NOTICE: role "regressuser5" is already a member of role "regressgroup2"
848+
CONTEXT: SQL function "dogrant_ok" statement 1
849+
dogrant_ok
850+
------------
851+
852+
(1 row)
853+
854+
SET ROLE regressgroup2;
855+
GRANT regressgroup2 TO regressuser5; -- fails: SET ROLE did not help
856+
ERROR: must have admin option on role "regressgroup2"
857+
SET SESSION AUTHORIZATION regressgroup2;
858+
GRANT regressgroup2 TO regressuser5; -- ok: a role can self-admin
859+
NOTICE: role "regressuser5" is already a member of role "regressgroup2"
860+
CREATE FUNCTION dogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINER AS
861+
'GRANT regressgroup2 TO regressuser5';
862+
SELECT dogrant_fails();-- fails: no self-admin in SECURITY DEFINER
863+
ERROR: must have admin option on role "regressgroup2"
864+
CONTEXT: SQL function "dogrant_fails" statement 1
865+
DROP FUNCTION dogrant_fails();
866+
SET SESSION AUTHORIZATION regressuser4;
867+
DROP FUNCTION dogrant_ok();
868+
REVOKE regressgroup2 FROM regressuser5;
835869
-- has_sequence_privilege tests
836870
\c -
837871
CREATE SEQUENCE x_seq;

‎src/test/regress/sql/privileges.sql

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ ALTER GROUP regressgroup1 ADD USER regressuser4;
3737

3838
ALTERGROUP regressgroup2 ADD USER regressuser2;-- duplicate
3939
ALTERGROUP regressgroup2 DROP USER regressuser2;
40-
ALTERGROUPregressgroup2ADD USERregressuser4;
40+
GRANTregressgroup2TOregressuser4 WITH ADMIN OPTION;
4141

4242
-- test owner privileges
4343

@@ -472,6 +472,33 @@ SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- false
472472
SELECT has_table_privilege('regressuser1','atest4','SELECT WITH GRANT OPTION');-- true
473473

474474

475+
-- Admin options
476+
477+
SET SESSION AUTHORIZATION regressuser4;
478+
CREATEFUNCTIONdogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINERAS
479+
'GRANT regressgroup2 TO regressuser5';
480+
GRANT regressgroup2 TO regressuser5;-- ok: had ADMIN OPTION
481+
SET ROLE regressgroup2;
482+
GRANT regressgroup2 TO regressuser5;-- fails: SET ROLE suspended privilege
483+
484+
SET SESSION AUTHORIZATION regressuser1;
485+
GRANT regressgroup2 TO regressuser5;-- fails: no ADMIN OPTION
486+
SELECT dogrant_ok();-- ok: SECURITY DEFINER conveys ADMIN
487+
SET ROLE regressgroup2;
488+
GRANT regressgroup2 TO regressuser5;-- fails: SET ROLE did not help
489+
490+
SET SESSION AUTHORIZATION regressgroup2;
491+
GRANT regressgroup2 TO regressuser5;-- ok: a role can self-admin
492+
CREATEFUNCTIONdogrant_fails() RETURNS void LANGUAGE sql SECURITY DEFINERAS
493+
'GRANT regressgroup2 TO regressuser5';
494+
SELECT dogrant_fails();-- fails: no self-admin in SECURITY DEFINER
495+
DROPFUNCTION dogrant_fails();
496+
497+
SET SESSION AUTHORIZATION regressuser4;
498+
DROPFUNCTION dogrant_ok();
499+
REVOKE regressgroup2FROM regressuser5;
500+
501+
475502
-- has_sequence_privilege tests
476503
\c-
477504

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp