Subject: Re: pgsql: Fix search_path to a safe value during maintenance operations.

From: Tom Lane <[email-masked]>
Date: Tue, 13 Jun 2023 16:23:24 -0400

Noah Misch <[email-masked]> writes:
> Best argument for shipping without $SUBJECT: we already have REFERENCES and
> TRIGGER privilege that tend to let the grantee hijack the table owner's
> account.  Adding MAINTAIN to the list, while sad, is defensible.  I still
> prefer to ship with $SUBJECT, not without.

What I'm concerned about is making such a fundamental semantics change
post-beta1.  It'll basically invalidate any application compatibility
testing anybody might have done against beta1.  I think this ship has
sailed as far as v16 is concerned, although we could reconsider it
in v17.

Also, I fail to see any connection to the MAINTAIN privilege: the
committed-and-reverted patch would break things whether the user
was making any use of that privilege or not.  Thus, I do not accept
the idea that we're fixing something that's new in 16.

			regards, tom lane

---

From: Jeff Davis <[email-masked]>
Date: Wed, 14 Jun 2023 21:59:40 -0700

On Tue, 2023-06-13 at 16:23 -0400, Tom Lane wrote:
> post-beta1.

I have added the patch to the July CF for v17.

If someone does feel like something should be done for v16, David G.
Johnston posted one possibility here:

https://www.postgresql.org/message-id/[email-masked]

But as Noah pointed out, there are other privileges that can be abused,
so a workaround for 16 might not be important if we have a likely fix
for MAINTAIN coming in 17.

Regards,
	Jeff Davis

---

From: Robert Haas <[email-masked]>
Date: Mon, 19 Jun 2023 16:03:36 -0400

> for MAINTAIN coming in 17.

Rather than is_superuser(userid) || userid == ownerid, I think that
the test should be has_privs_of_role(userid, ownerid).

I'm inclined to think that this is a real security issue and am not
very sanguine about waiting another year to fix it, but at the same
time, I'm somewhat worried that the proposed fix might be too narrow
or wrongly-shaped. I'm not too convinced that we've properly
understood what all of the problems in this area are. :-(

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Jeff Davis <[email-masked]>
Date: Mon, 19 Jun 2023 15:58:55 -0700

> I'm inclined to think that this is a real security issue and am not

Can you expand on that a bit? You mean a practical security issue for
the intended use cases?

> understood what all of the problems in this area are. :-(

Would it be acceptable to document that the MAINTAIN privilege (along
with TRIGGER and, if I understand correctly, REFERENCES) carries
privilege escalation risk for the grantor?

Regards,
	Jeff Davis

---

From: Robert Haas <[email-masked]>
Date: Thu, 29 Jun 2023 11:19:38 -0400

[ emerges from hibernation ]

> Can you expand on that a bit? You mean a practical security issue for

Yeah. I mean, as things stand, it seems like giving someone the
MAINTAIN privilege will be sufficient to allow them to escalate to the
table owner if there are any expression indexes involved. That seems
like a real problem. We shouldn't ship a new feature with a built-in
security hole like that.

I was pretty outraged when I realized that we'd been shipping releases
for years where CREATEROLE let you grab superuser because you could
just grant yourself pg_execute_server_program and then go to town.
Ideally, that hazard should have been identified and fixed in some way
before introducing pg_execute_server_program. I don't know whether the
hazard wasn't realized at the time or whether somebody somehow
convinced themselves that that was OK, but it clearly isn't.

Now we're proposing to ship a brand-new feature with a hole that we
definitely already know exists. I can't understand that at all. Should
we just go file the CVE against ourselves right now, then? Seriously,
what are we doing?

If we're not going to fix the feature so that it doesn't break the
security model, we should probably just revert it. I don't understand
at all the idea of shipping something that we 100% know is broken.

> privilege escalation risk for the grantor?

That's clearly better than nothing, but also seems like it's pretty
clearly the wrong approach. If somebody electrocutes themselves on the
toaster in the break room, you don't stick a sign on the side of it
that says "this toaster will electrocute you if you try to use it" and
then call it good. You either fix or replace the toaster, or at the
very least throw it out, or at the VERY least unplug it. I am failing
to understand how this situation is any different.