Hi, On 2023-02-06 14:07:39 -0500, Robert Haas wrote: > On Fri, Feb 3, 2023 at 3:47 AM Andres Freund <and...@anarazel.de> wrote: > > On 2023-02-02 09:28:03 -0500, Robert Haas wrote: > > > I don't know what you mean by this. DML doesn't confer privileges. If > > > code gets executed and runs with the replication user's credentials, > > > that could lead to privilege escalation, but just moving rows around > > > doesn't, at least not in the database sense. > > > > Executing DML ends up executing code. Think predicated/expression > > indexes, triggers, default expressions etc. If a badly written trigger > > etc can be tricked to do arbitrary code exec, an attack will be able to > > run with the privs of the run-as user. How bad that is is influenced to > > some degree by the amount of privileges that user has. > > I spent some time studying this today. I think you're right. What I'm > confused about is: why do we consider this situation even vaguely > acceptable? Isn't this basically an admission that our logical > replication security model is completely and totally broken and we > need to fix it somehow and file for a CVE number? Like, in released > branches, you can't even have a subscription owned by a non-superuser. > But any non-superuser can set a default expression or create an enable > always trigger and sure enough, if that table is replicated, the > system will run that trigger as the subscription owner, who is a > superuser. Which AFAICS means that if a non-superuser owns a table > that is part of a subscription, they can instantly hack superuser. > Which seems, uh, extremely bad. Am I missing something?
It's decidedly not great, yes. I don't know if it's quite a CVE type issue, after all, the same is true for any other type of query the superuser executes. But at the very least the documentation needs to be better, with a big red box making sure the admin is aware of the problem. I think we need some more fundamental ways to deal with this issue, including but not restricted to the replication context. Some potentially relevant discussion is in this thread: https://postgr.es/m/75b0dbb55e9febea54c441efff8012a6d2cb5bd7.camel%40j-davis.com I don't agree with Jeff's proposal, but I think there's some worthwhile ideas in the idea + followups. > And then, in master, where there's some provision for non-superuser > subscription owners, we maybe need to re-think the privileges required > to replicate into a table in the first place. I don't think that > having I/U/D permissions on a table is really sufficient to justify > performing those operations *as the table owner*; perhaps the check > ought to be whether you have the privileges of the table owner. Yes, I think we ought to check role membership, including non-inherited memberships. Greetings, Andres Freund