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

Peter Eisentraut <peter@eisentraut.org>

From: Peter Eisentraut <peter@eisentraut.org>
To: Feike Steenbergen <feikesteenbergen@gmail.com>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2025-06-18T21:11:39Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Restrict virtual columns to use built-in functions and types

  2. Fix virtual generated column type checking for ALTER TABLE

  3. Expand virtual generated columns in the planner

Attachments

On 05.06.25 12:49, Peter Eisentraut wrote:
> On 23.05.25 10:43, Feike Steenbergen wrote:
>> Attached is a sample exploit, that achieves this, key components:
>>
>> - the GENERATED column uses a user defined immutable function
>> - this immutable function cannot ALTER ROLE (needs volatile)
>> - therefore this immutable function calls a volatile function
>> - the volatile function can contain any security exploit
> 
> I propose to address this by not allowing the use of user-defined 
> functions in generation expressions for now.  The attached patch 
> implements this.  This assumes that all built-in functions are 
> trustworthy, for this purpose, which seems likely true and likely 
> desirable.
> 
> I think the feature is still useful like that, and this approach 
> provides a path to add new functionality in the future that grows this 
> set of allowed functions, for example by allowing some configurable set 
> of "trusted" functions or whatever.

Here is a new patch.

My previous patch was a bit too simple.  I had thought that 
check_functions_in_node() does the node walking itself, but that was 
wrong, so the patch only worked at the top-level of the expression.  So 
I had to build some node-walking scaffolding around it to make it work. 
Also, check_functions_in_node() has some comments about what node type 
it doesn't check, so I had to add some code to handle those.  This also 
requires that in addition to requiring built-in functions, we require 
built-in types.  This shouldn't move the needle, since non-builtin types 
can't do much without non-builtin functions.  Finally, it seems that 
most code actually uses FirstUnpinnedObjectId, not FirstNormalObjectId, 
to check for "built-in" status, so I changed to that, to be on the safe 
side.