Hi, On Tue, Jun 03, 2025 at 01:27:29PM -0400, Robert Haas wrote: > On Mon, Jun 2, 2025 at 10:02 AM Bertrand Drouvot > <bertranddrouvot...@gmail.com> wrote: > > So, we currently have 2 patterns: > > > > P1: permission checks on a referenced object is done before we call > > recordMultipleDependencies() > > P2: permission checks on a referenced object is done after we call > > recordMultipleDependencies() > > > > So, if we add locking in recordMultipleDependencies() then I think that P2 > > is worst > > than P1 (there is still the risk that the permissions changed by the time > > we reach recordMultipleDependencies() though). > > Hmm. I don't think I agree.
Thanks for sharing your thoughts! > If I understand correctly, P2 only permits > users to take a lock on an object they shouldn't be able to touch, > permitting them to temporarily interfere with access to it. P1 permits > users to potentially perform a permanent catalog modification that > should have been blocked by the permissions system. To my knowledge, > we've never formally classified the former type of problem as a > security vulnerability, although maybe there's an argument that it is > one. We've filed CVEs for problems of the latter sort. What I meant to say is that P2 is worse "by design" because it's "always" wrong to lock an object we don't have a permission on: so the permission should be checked first. So we have: P1: * check_permissions() * permissions could change here * Lock in recordMultipleDependencies() P2: * Lock in recordMultipleDependencies() * FWIW, permissions could change here too (for example, one could still " revoke usage on schema myschema1 from user1" while there is an AccessShareLock on schema myschema1) * check_permissions() But P2 sequence of events is "wrong" by design (to lock an object we may not have permissions on) that's what I meant. Now if we look at it from a "pure" security angle (as you did) I agree that P1 is the worse because it could allow catalog change that should have been blocked by the permission check. P2 would prevent that. I agree that we should focus on P1 then. Let me try to list the P1 cases (instead of the P2 ones) so that we can focus on /discuss those. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
