On Fri, 26 Jun 2026 at 05:13, Jacob Champion <[email protected]> wrote: > > [moving to -hackers] > > On Wed, Jun 24, 2026 at 2:57 PM Jacob Champion > <[email protected]> wrote: > > TL;DR: The protection in recursive_revoke() against broken GRANT > > OPTION chains doesn't seem to work properly when the grantee also > > holds the privileges of the grantor. > > More accurately: "when an intermediate grantor in the chain only > indirectly holds the ability to grant." > > > I think the issue is in recursive_revoke()'s usage of aclmask(), which > > in turn uses has_privs_of_role(). It doesn't seem like that's what was > > wanted in this particular case... thoughts? > > I propose changing that to aclmask_direct(), as in the attached, and > backpatching all the way down. > > To try to prove to myself that this works, I added tests to pin each > of the three cases that are treated differently by aclmask_direct(): > 1. the grantor has indirect ownership privileges > 2. the grantor has indirect grant options via INHERIT > 3. the grantor has indirect grant options via PUBLIC (which is already > disallowed in practice) > > I also tried to expand the existing comment, both to point out the > pitfall and to explain why the short-circuit works. But I've rewritten > it at least a dozen times, so if anyone can tell me whether I've made > sense and/or used the terminology appropriately, I'd appreciate it. > > > I'm pretty sure the following is unintended behavior. It looks > > potentially related to [1] as well. > > (To fix [1] I suspect we need to make a similar tweak to > check_circularity(), but I haven't looked into that yet.) > > Thanks! > --Jacob > > [1] > https://postgr.es/m/CAM6Zo8wD7RtQNhbQHODc9DobiW+GpT=tnqosmz4+mnza9m0...@mail.gmail.com
Hi! I think to fix [1] you pointing to we can use my patch [0]. I checked that your patch doesn't fix my (and [1]) case and vice-versa: my patch doesn't prevent problems in the revoke case. With your v1 I run CREATE ROLE r1 LOGIN; CREATE ROLE r2 LOGIN; CREATE ROLE r3; GRANT r3 TO r2; GRANT CREATE ON SCHEMA public TO r1; SET ROLE r1; CREATE VIEW v AS SELECT; GRANT SELECT ON v TO r2 WITH GRANT OPTION; -- r2=r*/r1 GRANT SELECT ON v TO r3 WITH GRANT OPTION; -- r3=r*/r SET ROLE r2; GRANT SELECT ON v TO r2 WITH GRANT OPTION; -- r2=r*/r2 -- self grant, already bad and then successfully do REVOKE r3 FROM r2 leaving very bad grants. Also I checked that my patch fixes [1] while patch is this thread not. So I think we need both. Also, another option is to adjust your patch so that it would reject REVOKE r3 FROM r2 in my case, but I think we should fail on GRANT SELECT ON v TO r2 WITH GRANT OPTION; About your patch: LGTM [0] https://www.postgresql.org/message-id/CALdSSPi5JaToi_p0%3DkPUynSTqUdqq8X-axRsrKL1TX0yt1KLiA%40mail.gmail.com [1] https://postgr.es/m/CAM6Zo8wD7RtQNhbQHODc9DobiW+GpT=tnqosmz4+mnza9m0...@mail.gmail.com -- Best regards, Kirill Reshke
