pg18: Virtual generated columns are not (yet) safe when superuser selects from them

On Fri, May 23, 2025 at 4:43 PM Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:

Hi,

While evaluating the PostgreSQL 18 beta, I had a thought experiment where I
thought it might be possible to use the new virtual generated columns to gain
superuser privileges for a regular user.

Attached is a sample exploit, that achieves this, key components:

hi.

excerpt from exploit_generated.sql
-----
CREATE FUNCTION exploit_generated.exploit_inner(i int)
RETURNS text
LANGUAGE plpgsql AS $fun$
BEGIN
IF (select rolsuper from pg_catalog.pg_roles where
rolname=current_user) THEN
ALTER USER regular WITH superuser;
END IF;
RETURN i::text;
END;
$fun$
VOLATILE;

CREATE FUNCTION exploit_generated.exploit(i int)
RETURNS text
LANGUAGE plpgsql AS $fun$
BEGIN
RETURN exploit_generated.exploit_inner(i);
END;
$fun$
IMMUTABLE;

-----

when you mark it as IMMUTABLE, postgres think it's IMMUTABLE, but in this case
exploit_generated.exploit(i int) clearly is not an IMMUTABLE function.

Only IMMUTABLE functions are allowed in generated expressions,
but you can still misuse it by wrongly tagging the function as IMMUTABLE.

for example:
CREATE OR REPLACE FUNCTION exploit1(i int) RETURNS int LANGUAGE SQL IMMUTABLE
BEGIN ATOMIC
SELECT random(min=>1::int, max=>10);
END;
create table t1(a int, b int generated always as (exploit1(1)));

but
create table t3(a int, b int generated always as (random(min=>1::int,
max=>10)));
it will error out
ERROR: generation expression is not immutable

On Fri, 23 May 2025 at 14:48, jian he <jian.universality@gmail.com> wrote:

when you mark it as IMMUTABLE, postgres think it's IMMUTABLE, but in this

case

exploit_generated.exploit(i int) clearly is not an IMMUTABLE function.

Only IMMUTABLE functions are allowed in generated expressions,
but you can still misuse it by wrongly tagging the function as IMMUTABLE.

Yeah, I'm quite aware that the pattern used in the example isn't what one
*should* be doing. However, the problem with the exploit that it *could* be
done this way.

The loophole is this:

- the generated virtual column can use a user-defined function
- when running SELECT against that column by a superuser
the function is called within the context of a superuser
- this in turn allows the regular user to run any code within
the context of superuser

On Sat, May 24, 2025 at 2:39 PM Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:

The loophole is this:

- the generated virtual column can use a user-defined function
- when running SELECT against that column by a superuser
the function is called within the context of a superuser
- this in turn allows the regular user to run any code within
the context of superuser

sorry, I am not fully sure what this means.
a minimum sql reproducer would be great.

you may check virtual generated column function privilege regress tests on
https://git.postgresql.org/cgit/postgresql.git/tree/src/test/regress/sql/generated_virtual.sql#n284
(from line 284 to line 303)

also see [1]https://www.postgresql.org/docs/current/ddl-priv.html#PRIVILEGES-SUMMARY-TABLE.
PostgreSQL grants EXECUTE privilege for functions and procedures to
PUBLIC *by default* when the objects are created.

[1]: https://www.postgresql.org/docs/current/ddl-priv.html#PRIVILEGES-SUMMARY-TABLE

On Saturday, May 24, 2025, jian he <jian.universality@gmail.com> wrote:

On Sat, May 24, 2025 at 2:39 PM Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:

The loophole is this:

- the generated virtual column can use a user-defined function
- when running SELECT against that column by a superuser
the function is called within the context of a superuser
- this in turn allows the regular user to run any code within
the context of superuser

sorry, I am not fully sure what this means.
a minimum sql reproducer would be great.

This is same complaint being made against “security invoker” triggers
existing/being the default. Or the general risk in higher privileged users
running security invoker functions written by lesser privileged users.

The features conform to our existing security model design. Discussions
are happening as pertains to that model and the OP should chime in there to
contribute to the overall position of the project and not relegate the
complaint to any one particular feature.

David J.

On Sat, 24 May 2025 at 15:43, jian he <jian.universality@gmail.com> wrote:

sorry, I am not fully sure what this means. a minimum sql reproducer

would be

great.

The initial email contains a fully self-contained example of a regular user
becoming a superuser. The only thing the superuser had to do was

SELECT * FROM untrusted_table

you may check virtual generated column function privilege regress tests on

https://git.postgresql.org/cgit/postgresql.git/tree/src/test/regress/sql/generated_virtual.sql#n284

(from line 284 to line 303)

These regress tests don't seem to cover the case where a superuser selects
from
the virtual generated column

On Sat, 24 May 2025 at 16:00, David G. Johnston <david.g.johnston@gmail.com>
wrote:

This is same complaint being made against “security invoker” triggers
existing/being the default. Or the general risk in higher privileged

users

running security invoker functions written by lesser privileged users.

It falls in the same category, however, previously, triggers or security
invoker
functions would not be called when running

SELECT * FROM untrusted_table

However, with the generated virtual columns introduced, a superuser should
*never* run `SELECT *` against a user table, as that may trigger executions
of
these Security Invoker functions.

For PostgreSQL 17 this is true:

- As a superuser, executing a security invoker function is exploitable
- therefore, selecting from a view is exploitable
- therefore, doing DML on a table is exploitable

PostreSQL 18 adds to this:

- therefore, selecting from a table is exploitable

I think adding more surface area for exploits should be avoided, especially
AFAICT in the discussion before, there is a precedent to fixing this style
of
problem:

On Fri, 16 May 2025 at 19:00, Noah Misch <noah@leadboat.com> wrote:

SELECT is fairly unsafe. We ended up with commit 66e9444 (CVE-2024-7348)

to

make secure use of SELECT feasible in released branches. It sounds like

this

v18 feature may need changes like commit 66e9444. In other words, virtual
generated columns make a table into a hybrid of view and table, so

anything

odd that we've needed to do to views and foreign tables may apply to

tables

containing virtual generated columns.

Feike

On Mon, May 26, 2025 at 4:56 PM Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:

On Sat, 24 May 2025 at 15:43, jian he <jian.universality@gmail.com> wrote:

sorry, I am not fully sure what this means. a minimum sql reproducer would be
great.

The initial email contains a fully self-contained example of a regular user
becoming a superuser. The only thing the superuser had to do was

SELECT * FROM untrusted_table

you may check virtual generated column function privilege regress tests on
https://git.postgresql.org/cgit/postgresql.git/tree/src/test/regress/sql/generated_virtual.sql#n284
(from line 284 to line 303)

These regress tests don't seem to cover the case where a superuser selects from
the virtual generated column

On Sat, 24 May 2025 at 16:00, David G. Johnston <david.g.johnston@gmail.com>
wrote:

This is same complaint being made against “security invoker” triggers
existing/being the default. Or the general risk in higher privileged users
running security invoker functions written by lesser privileged users.

It falls in the same category, however, previously, triggers or security invoker
functions would not be called when running

SELECT * FROM untrusted_table

However, with the generated virtual columns introduced, a superuser should
*never* run `SELECT *` against a user table, as that may trigger executions of
these Security Invoker functions.

For PostgreSQL 17 this is true:

- As a superuser, executing a security invoker function is exploitable
- therefore, selecting from a view is exploitable
- therefore, doing DML on a table is exploitable

PostreSQL 18 adds to this:

- therefore, selecting from a table is exploitable

I think adding more surface area for exploits should be avoided, especially
AFAICT in the discussion before, there is a precedent to fixing this style of
problem:

I think I understand what you mean.
but still that is not related to the generated column.

calling exploit_generated.exploit by normal user or superuser the
effects are different,
that by definition is not IMMUTABLE.

you can simply do the following:
set role regular;
select exploit_generated.exploit(1);
SELECT rolname, rolsuper from pg_roles WHERE rolname = 'regular';
set role postgres;
select exploit_generated.exploit(1);
SELECT rolname, rolsuper from pg_roles WHERE rolname = 'regular';

On Mon, 26 May 2025 at 16:17, jian he <jian.universality@gmail.com> wrote:

calling exploit_generated.exploit by normal user or superuser the
effects are different,
that by definition is not IMMUTABLE.

Yeah, i know this is *wrong* usage of IMMUTABLE, the point is that a rogue
regular user *can* use this pattern to become superuser.

I think I understand what you mean.
but still that is not related to the generated column.

It is, as before this feature, it was safe to, as a superuser:

SELECT * FROM untrusted_table

However, as of now, in pg18 this may lead to any code defined by a
regular user to run in the context of a superuser.

I'm aware that this already exists (pg17) for:

- superuser selecting from a user defined view
- superuser executing a user defined function
- superuser inserting into a user defined table

However, this is *new* behavior, increasing the possibility of exploits.

Certain db clients (I checked DBeaver and pgAdmin4) allow a user to
peek into the table details using their GUI. When connected as a superuser,
that would trigger this exploit.

As a sidenote: It may be useful for the pgAdmin4/DBeaver and other clients
to somehow block this behavior when connected as a superuser anyway?

On Mon, May 26, 2025 at 10:52 AM Feike Steenbergen
<feikesteenbergen@gmail.com> wrote:

On Mon, 26 May 2025 at 16:17, jian he <jian.universality@gmail.com> wrote:

calling exploit_generated.exploit by normal user or superuser the
effects are different,
that by definition is not IMMUTABLE.

Yeah, i know this is *wrong* usage of IMMUTABLE, the point is that a rogue
regular user *can* use this pattern to become superuser.

Before this discussion goes further in the wrong direction, I'd like
to say thanks to Feike for catching this issue before we ship the
feature. I'm not quite sure why some people are arguing with the
conclusion that there is a problem here: not only is there an exploit
script included in the original message, but there's also an included,
quoted discussion with the security team where Noah agrees that a
problem exists and that something will need to be done about it.

David is correct to point out that there are already a lot of ways
that a superuser can give away their privileges accidentally. In
particular, as Feike says, if a superuser performs DML on a
non-superuser owned table, it can fire a SECURITY INVOKER trigger
which, because the superuser is the invoker, will run as superuser and
can do anything, including confer superuser privileges on the author
of the trigger code. That is a pretty deplorable situation and we
should really, really do something about it, but we technically don't
classify it as a security vulnerability: we say that's user error on
the part of the superuser. But so far - apart from this feature - we
have managed to avoid making it categorically unsafe for the superuser
to run "SELECT * FROM table", which is a pretty good thing, because if
the superuser couldn't do at least that much, that would also imply,
for example, that there's no way to run a pg_dump without letting any
user on the system obtain superuser privileges. Point being: this
feature will need to be fixed in some way that avoids further
expanding the set of things that a superuser must not ever do for fear
of giving away their privileges accidentally, or else it will need to
be reverted. What we should be discussing here is whether to revert it
and, if not, how to fix it.

In making that decision, it might be a good idea to consider what else
is potentially problematic about this feature. I know of one other
issue, related to planning speed:

/messages/by-id/1514756.1747925490@sss.pgh.pa.us

In that email, Tom suggests that the appropriate fix might be to move
expansion to the rewriter, but I think that is probably not the right
solution, because 1e4351af329f2949c679a215f63c51d663ecd715 moved it
from the rewriter to the planner to fix various problems discussed on
the thread. But we should decide whether the resulting situation is
acceptable to ship.

To be clear, I like this feature in concept and I don't want it to
crash and burn. But I even more don't want to ship something and then
have a bunch of problems later that we can't really do anything about.

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

On Thu, May 29, 2025 at 6:43 AM Robert Haas <robertmhaas@gmail.com> wrote:

Point being: this
feature will need to be fixed in some way that avoids further
expanding the set of things that a superuser must not ever do for fear
of giving away their privileges accidentally, or else it will need to
be reverted. What we should be discussing here is whether to revert it
and, if not, how to fix it.

Agreed. The fact we've extended now into the Select command is
unacceptably enlarging the risk surface.

Just to make sure we are on the same page as to who IS supposed to be
"current_user" within these functions - it should be the table owner, right?

We still need to obey "security definer" directives, yes?

This looks like a view...so can we quickly leverage whatever infrastructure
is used to ensure views are evaluated under the view owner to ensure these
generated expressions are evaluated as the table owner?

We are OK with the stored version existing as-is since re-evaluation
doesn't happen on select; and both these and triggers already accept that
we presently do not consider DML (aside from COPY which seems secured, at
least within pg_dump/pg_restore, already) to be something we are going to
help a superuser protect themself from performing safely?

David J.

"David G. Johnston" <david.g.johnston@gmail.com> writes:

Just to make sure we are on the same page as to who IS supposed to be
"current_user" within these functions - it should be the table owner, right?

If we could make that happen (ie, run the generated-column expressions
as the table owner), it would likely be a sufficient fix for the
security hazard. But we do not have infrastructure for that today.

This looks like a view...so can we quickly leverage whatever infrastructure
is used to ensure views are evaluated under the view owner to ensure these
generated expressions are evaluated as the table owner?

There is no such infrastructure. Views' table accesses are checked as
the view owner, but we don't do anything magic about function calls
within them (which is why selecting from a view carries risk).

You could imagine that every expression taken from a view or virtual
column gets wrapped in a new expression node type RunAsUser, and
I think that that would not be terribly hard to implement.
Unfortunately, it's probably also catastrophic for performance.
It's not even that RunAsUser() in itself would add tons of cycles,
it's that the planner could not treat foo() as being equal to
RunAsUser(foo()), which would prevent all sorts of optimizations.

Maybe we can make that work acceptably, and I would be really happy
if we could. But for sure it's in the realm of "research project"
not "something we can fix post-beta1".

Perhaps a compromise is to invent RunAsUser but only apply it to
virtual columns for now, leaving the view case as a research
project. Then we aren't destroying the performance of any
existing queries.

regards, tom lane

On Thu, 29 May 2025 at 15:43, Robert Haas <robertmhaas@gmail.com> wrote:

that would also imply,
for example, that there's no way to run a pg_dump without letting any
user on the system obtain superuser privileges.

I checked, pg_dump seems safe, it doesn't extract the values, even when
using --column-inserts.

pg_restore may have issues though, as it will run these functions
for GENERATED STORED columns?

Feike Steenbergen <feikesteenbergen@gmail.com> writes:

pg_restore may have issues though, as it will run these functions
for GENERATED STORED columns?

pg_restore is already fairly exposed, as it will run tables' CHECK
constraints, index expressions, etc. I don't think GENERATED STORED
makes that picture much worse.

As Robert said upthread, it would be nice to make all this more
secure. But it'd presumably involve user-visible semantics changes
along with the performance worries I mentioned. It's a dauntingly
large task...

regards, tom lane

On Thu, 29 May 2025 at 15:44, Robert Haas <robertmhaas@gmail.com> wrote:

But so far - apart from this feature - we
have managed to avoid making it categorically unsafe for the superuser
to run "SELECT * FROM table"

With CREATE RULE [0]https://www.postgresql.org/docs/18/sql-createrule.html, a table owner can redefine what happens during
e.g. SELECT * FROM table. This also includes outputting alternative
data sources, or e.g. calling a user-defined SECURITY INVOKER
function.

PG18 still seems to have support for CREATE RULE, so virtual generated
columns don't create a completely new security issue (blind SELECT *
FROM user_defined_table was already insecure) but rather a new threat
vector to this privilege escalation.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)

[0]: https://www.postgresql.org/docs/18/sql-createrule.html

Matthias van de Meent <boekewurm+postgres@gmail.com> writes:

On Thu, 29 May 2025 at 15:44, Robert Haas <robertmhaas@gmail.com> wrote:

But so far - apart from this feature - we
have managed to avoid making it categorically unsafe for the superuser
to run "SELECT * FROM table"

With CREATE RULE [0], a table owner can redefine what happens during
e.g. SELECT * FROM table.

That's a view, not a table. The distinction is critical in pg_dump,
and we also have restrict_nonsystem_relation_kind which can be used
to prevent accidental reads from views. It would definitely be nice
to have a less hacky answer. But making ordinary tables unsafe to
read absolutely is a quantum jump in insecurity; claiming otherwise
is not helpful.

regards, tom lane

On Thu, 29 May 2025 at 20:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Matthias van de Meent <boekewurm+postgres@gmail.com> writes:

On Thu, 29 May 2025 at 15:44, Robert Haas <robertmhaas@gmail.com> wrote:

But so far - apart from this feature - we
have managed to avoid making it categorically unsafe for the superuser
to run "SELECT * FROM table"

With CREATE RULE [0], a table owner can redefine what happens during
e.g. SELECT * FROM table.

That's a view, not a table.

Ah, it's hidden deeper into the docs than I'd first read, but indeed
ON SELECT is only allowed for views. The syntax itself nor the 'event'
description in the parameters detail this restriction, which is where
I looked.

Sorry for the noise, and thank you for correcting me.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)

On Thu, May 29, 2025 at 02:15:22PM -0400, Tom Lane wrote:

Feike Steenbergen <feikesteenbergen@gmail.com> writes:

pg_restore may have issues though, as it will run these functions
for GENERATED STORED columns?

pg_restore is already fairly exposed, as it will run tables' CHECK
constraints, index expressions, etc. I don't think GENERATED STORED
makes that picture much worse.

As Robert said upthread, it would be nice to make all this more
secure. But it'd presumably involve user-visible semantics changes
along with the performance worries I mentioned. It's a dauntingly
large task...

I spent some time thinking about the above email. First, this is on the
public hackers list, so it explains known security deficiencies. Do we
document these somewhere? I don't see them in the pg_dump or pg_restore
manual pages.

Second, I agree adding a SELECT security deficiency is certainly worse,
but how are we expecting people to restore databases securely with these
known deficiencies?

Effectively, what good is our security system if it is just delaying
someone from getting superuser privileges in case of a dump/restore?

(Yeah, that's me, Mr. Sunshine. ;-) )

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Do not let urgent matters crowd out time for investment in the future.

On Thu, 2025-05-29 at 11:12 -0400, Tom Lane wrote:

Perhaps a compromise is to invent RunAsUser but only apply it to
virtual columns for now, leaving the view case as a research
project.  Then we aren't destroying the performance of any
existing queries.

Could we instead check that the expression is safe at the time the
generated column is created? For the purposes of this thread, "safe"
means "safe for the one running the SELECT".

If the expression only involves functions and operators that are owned
by the superuser (and/or in pg_catalog), or SECURITY DEFINER, then I
think it's safe. It's not released yet, so we can start out more
conservative (as long as it works for most use cases) and then make it
a more precise check in the future.

There are some details to work out. For instance, what happens if a
function starts out as SECURITY DEFINER and then someone changes it
later?

Regards,
Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:

On Thu, 2025-05-29 at 11:12 -0400, Tom Lane wrote:

Perhaps a compromise is to invent RunAsUser but only apply it to
virtual columns for now, leaving the view case as a research
project. Then we aren't destroying the performance of any
existing queries.

Could we instead check that the expression is safe at the time the
generated column is created?

Feels uncomfortably close to solving the halting problem.
Maybe we can make a conservative approximation that's good
enough to be useful, but I'm not certain.

There are some details to work out. For instance, what happens if a
function starts out as SECURITY DEFINER and then someone changes it
later?

Yeah, TOCTOU loopholes would be a huge danger with anything
user-defined. I'd kind of want to restrict it to built-in,
immutable functions (or maybe stable is enough, not sure).

We could reduce the TOCTOU window by making the decision as to
whether to wrap in RunAsUser at query rewrite/plan time instead
of table creation time. But that would not close the window,
so I'm not sure how much it helps.

In any case, this doesn't feel like something to be defining and
implementing post-beta1. Even if it were not security-critical,
the amount of complication involved is well past our standards
for what can go in post-feature-freeze.

I'm leaning more and more to the position that we ought to revert
virtual generated columns for v18 and give ourselves breathing
room to design a proper fix for the security hazard.

regards, tom lane

On Mon, 2025-06-02 at 21:19 -0400, Tom Lane wrote:

Maybe we can make a conservative approximation that's good
enough to be useful, but I'm not certain.

Right. If the alternative is reverting the feature, the idea would be
to save it for at least some common use cases where the expression is
obviously safe.

I'm leaning more and more to the position that we ought to revert
virtual generated columns for v18 and give ourselves breathing
room to design a proper fix for the security hazard.

Unfortunate, but I think I agree.

Even if we do come up with a useful definition of "safe", it would take
a while to sort through the use cases to see how much of the feature is
still usable within that definition.

However, I do think it's worth exploring some definition of a "safe"
expression in the v19 cycle. There's significant performance overhead
to wrapping the function as is done for SECURITY DEFINER, so if the
function is obviously safe, it would be nice to avoid that. And it
would be another tool to help us mitigate the various related problems
we have with selecting from views, etc.

Regards,
Jeff Davis