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.
From 75ff6fb9c14d761b7b07cfdb007a7d775f943d0c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 18 Jun 2025 23:07:24 +0200
Subject: [PATCH v1] Restrict virtual columns to use built-in functions and
types
Just like selecting from a view is exploitable (CVE-2024-7348),
selecting from a table with virtual generated columns is exploitable.
Users who are concerned about this can avoid selecting from views, but
telling them to avoid selecting from tables is less practical.
To address this, this changes it so that generation expressions for
virtual generated columns are restricted to using built-in functions
and types. We assume that built-in functions and types cannot be
exploited for this purpose.
In the future, this could be expanded by some new mechanism to declare
other functions and types as safe or trusted for this purpose, but
that is to be designed.
(An alternative approach might have been to expand the
restrict_nonsystem_relation_kind GUC to handle this, like the fix for
CVE-2024-7348. But that is kind of an ugly approach. That fix had to
fit in the constraints of fixing an ancient vulnerability in all
branches. Since virtual generated columns are new, we're free from
the constraints of the past, and we can and should use cleaner
options.)
Reported-by: Feike Steenbergen <feikesteenber...@gmail.com>
Discussion:
https://www.postgresql.org/message-id/flat/CAK_s-G2Q7de8Q0qOYUR%3D_CTB5FzzVBm5iZjOp%2BmeVWpMpmfO0w%40mail.gmail.com
---
doc/src/sgml/ddl.sgml | 9 ++
doc/src/sgml/ref/create_table.sgml | 8 ++
src/backend/catalog/heap.c | 84 +++++++++++++++++++
.../regress/expected/generated_virtual.out | 23 ++---
src/test/regress/expected/publication.out | 12 ++-
src/test/regress/sql/generated_virtual.sql | 15 ++--
src/test/regress/sql/publication.sql | 5 +-
7 files changed, 134 insertions(+), 22 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 96936bcd3ae..b13513abb61 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -419,6 +419,15 @@ <title>Generated Columns</title>
<varname>tableoid</varname>.
</para>
</listitem>
+ <listitem>
+ <para>
+ The generation expression of a virtual generated column must not
+ reference user-defined functions or types, that is, it can only
+ reference built-in functions or types. This applies also indirectly,
+ such as for functions or types that underlie operators or casts. (This
+ restriction does not exist for stored generated columns.)
+ </para>
+ </listitem>
<listitem>
<para>
A generated column cannot have a column default or an identity
definition.
diff --git a/doc/src/sgml/ref/create_table.sgml
b/doc/src/sgml/ref/create_table.sgml
index 4a41b2f5530..dbffbf789cc 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -929,6 +929,14 @@ <title>Parameters</title>
not other generated columns. Any functions and operators used must be
immutable. References to other tables are not allowed.
</para>
+
+ <para>
+ The generation expression of a virtual generated column must not
+ reference user-defined functions or types, that is, it can only
+ reference built-in functions or types. This applies also indirectly,
+ such as for functions or types that underlie operators or casts. (This
+ restriction does not exist for stored generated columns.)
+ </para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 10f43c51c5a..906282aae63 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3215,6 +3215,86 @@ check_nested_generated(ParseState *pstate, Node *node)
check_nested_generated_walker(node, pstate);
}
+/*
+ * Check security of virtual generated column expression.
+ *
+ * Just like selecting from a view is exploitable (CVE-2024-7348), selecting
+ * from a table with virtual generated columns is exploitable. Users who are
+ * concerned about this can avoid selecting from views, but telling them to
+ * avoid selecting from tables is less practical.
+ *
+ * To address this, this restricts generation expressions for virtual
+ * generated columns are restricted to using built-in functions and types. We
+ * assume that built-in functions and types cannot be exploited for this
+ * purpose. Note the overall security also requires that all functions in use
+ * a immutable. (For example, there are some built-in non-immutable functions
+ * that can run arbitrary SQL.) The immutability is checked elsewhere, since
+ * that is a property that needs to hold independent of security
+ * considerations.
+ *
+ * In the future, this could be expanded by some new mechanism to declare
+ * other functions and types as safe or trusted for this purpose, but that is
+ * to be designed.
+ */
+
+/*
+ * Callback for check_functions_in_node() that determines whether a function
+ * is user-defined.
+ */
+static bool
+contains_user_functions_checker(Oid func_id, void *context)
+{
+ return (func_id >= FirstUnpinnedObjectId);
+}
+
+/*
+ * Checks for all the things we don't want in the generation expressions of
+ * virtual generated columns for security reasons. Errors out if it finds
+ * one.
+ */
+static bool
+check_virtual_generated_security_walker(Node *node, void *context)
+{
+ ParseState *pstate = context;
+
+ if (node == NULL)
+ return false;
+
+ if (!IsA(node, List))
+ {
+ if (check_functions_in_node(node,
contains_user_functions_checker, NULL))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("generation expression uses
user-defined function"),
+ errdetail("Virtual generated columns
that make use of user-defined functions are not yet supported."),
+ parser_errposition(pstate,
exprLocation(node)));
+
+ /*
+ * check_functions_in_node() doesn't check some node types (see
+ * comment there). We handle CoerceToDomain and MinMaxExpr by
+ * checking for built-in types. The other listed node types
cannot
+ * call user-definable SQL-visible functions.
+ *
+ * We furthermore need this type check to handle built-in,
immutable
+ * polymorphic functions such as array_eq().
+ */
+ if (exprType(node) >= FirstUnpinnedObjectId)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("generation expression uses
user-defined type"),
+ errdetail("Virtual generated columns
that make use of user-defined types are not yet supported."),
+ parser_errposition(pstate,
exprLocation(node)));
+ }
+
+ return expression_tree_walker(node,
check_virtual_generated_security_walker, context);
+}
+
+static void
+check_virtual_generated_security(ParseState *pstate, Node *node)
+{
+ check_virtual_generated_security_walker(node, pstate);
+}
+
/*
* Take a raw default and convert it to a cooked format ready for
* storage.
@@ -3254,6 +3334,10 @@ cookDefault(ParseState *pstate,
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("generation expression is not
immutable")));
+
+ /* Check security of expressions for virtual generated column */
+ if (attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ check_virtual_generated_security(pstate, expr);
}
else
{
diff --git a/src/test/regress/expected/generated_virtual.out
b/src/test/regress/expected/generated_virtual.out
index 6300e7c1d96..0a8c9b3abcb 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -604,9 +604,13 @@ INSERT INTO gtest11 VALUES (1, 10), (2, 20);
GRANT SELECT (a, c) ON gtest11 TO regress_user11;
CREATE FUNCTION gf1(a int) RETURNS int AS $$ SELECT a * 3 $$ IMMUTABLE
LANGUAGE SQL;
REVOKE ALL ON FUNCTION gf1(int) FROM PUBLIC;
-CREATE TABLE gtest12 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS
(gf1(b)) VIRTUAL);
-INSERT INTO gtest12 VALUES (1, 10), (2, 20);
-GRANT SELECT (a, c), INSERT ON gtest12 TO regress_user11;
+CREATE TABLE gtest12 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS
(gf1(b)) VIRTUAL); -- fails, user-defined function
+ERROR: generation expression uses user-defined function
+LINE 1: ...nt PRIMARY KEY, b int, c int GENERATED ALWAYS AS (gf1(b)) VI...
+ ^
+DETAIL: Virtual generated columns that make use of user-defined functions are
not yet supported.
+--INSERT INTO gtest12 VALUES (1, 10), (2, 20);
+--GRANT SELECT (a, c), INSERT ON gtest12 TO regress_user11;
SET ROLE regress_user11;
SELECT a, b FROM gtest11; -- not allowed
ERROR: permission denied for table gtest11
@@ -619,15 +623,12 @@ SELECT a, c FROM gtest11; -- allowed
SELECT gf1(10); -- not allowed
ERROR: permission denied for function gf1
-INSERT INTO gtest12 VALUES (3, 30), (4, 40); -- allowed (does not actually
invoke the function)
-SELECT a, c FROM gtest12; -- currently not allowed because of function
permissions, should arguably be allowed
-ERROR: permission denied for function gf1
+--INSERT INTO gtest12 VALUES (3, 30), (4, 40); -- allowed (does not actually
invoke the function)
+--SELECT a, c FROM gtest12; -- currently not allowed because of function
permissions, should arguably be allowed
RESET ROLE;
-DROP FUNCTION gf1(int); -- fail
-ERROR: cannot drop function gf1(integer) because other objects depend on it
-DETAIL: column c of table gtest12 depends on function gf1(integer)
-HINT: Use DROP ... CASCADE to drop the dependent objects too.
-DROP TABLE gtest11, gtest12;
+--DROP FUNCTION gf1(int); -- fail
+DROP TABLE gtest11;
+--DROP TABLE gtest12;
DROP FUNCTION gf1(int);
DROP USER regress_user11;
-- check constraints
diff --git a/src/test/regress/expected/publication.out
b/src/test/regress/expected/publication.out
index 4de96c04f9d..f1025fc0f19 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -524,10 +524,16 @@ Tables from schemas:
"testpub_rf_schema2"
-- fail - virtual generated column uses user-defined function
+-- (Actually, this already fails at CREATE TABLE rather than at CREATE
+-- PUBLICATION, but let's keep the test in case the former gets
+-- relaxed sometime.)
CREATE TABLE testpub_rf_tbl6 (id int PRIMARY KEY, x int, y int GENERATED
ALWAYS AS (x * testpub_rf_func2()) VIRTUAL);
+ERROR: generation expression uses user-defined function
+LINE 1: ...RIMARY KEY, x int, y int GENERATED ALWAYS AS (x * testpub_rf...
+ ^
+DETAIL: Virtual generated columns that make use of user-defined functions are
not yet supported.
CREATE PUBLICATION testpub7 FOR TABLE testpub_rf_tbl6 WHERE (y > 100);
-ERROR: invalid publication WHERE expression
-DETAIL: User-defined or built-in mutable functions are not allowed.
+ERROR: relation "testpub_rf_tbl6" does not exist
-- test that SET EXPRESSION is rejected, because it could affect a row filter
SET client_min_messages = 'ERROR';
CREATE TABLE testpub_rf_tbl7 (id int PRIMARY KEY, x int, y int GENERATED
ALWAYS AS (x * 111) VIRTUAL);
@@ -541,7 +547,7 @@ DROP TABLE testpub_rf_tbl2;
DROP TABLE testpub_rf_tbl3;
DROP TABLE testpub_rf_tbl4;
DROP TABLE testpub_rf_tbl5;
-DROP TABLE testpub_rf_tbl6;
+--DROP TABLE testpub_rf_tbl6;
DROP TABLE testpub_rf_schema1.testpub_rf_tbl5;
DROP TABLE testpub_rf_schema2.testpub_rf_tbl6;
DROP SCHEMA testpub_rf_schema1;
diff --git a/src/test/regress/sql/generated_virtual.sql
b/src/test/regress/sql/generated_virtual.sql
index b4eedeee2fb..ae5c1c5f8f8 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -290,20 +290,21 @@ CREATE TABLE gtest11 (a int PRIMARY KEY, b int, c int
GENERATED ALWAYS AS (b * 2
CREATE FUNCTION gf1(a int) RETURNS int AS $$ SELECT a * 3 $$ IMMUTABLE
LANGUAGE SQL;
REVOKE ALL ON FUNCTION gf1(int) FROM PUBLIC;
-CREATE TABLE gtest12 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS
(gf1(b)) VIRTUAL);
-INSERT INTO gtest12 VALUES (1, 10), (2, 20);
-GRANT SELECT (a, c), INSERT ON gtest12 TO regress_user11;
+CREATE TABLE gtest12 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS
(gf1(b)) VIRTUAL); -- fails, user-defined function
+--INSERT INTO gtest12 VALUES (1, 10), (2, 20);
+--GRANT SELECT (a, c), INSERT ON gtest12 TO regress_user11;
SET ROLE regress_user11;
SELECT a, b FROM gtest11; -- not allowed
SELECT a, c FROM gtest11; -- allowed
SELECT gf1(10); -- not allowed
-INSERT INTO gtest12 VALUES (3, 30), (4, 40); -- allowed (does not actually
invoke the function)
-SELECT a, c FROM gtest12; -- currently not allowed because of function
permissions, should arguably be allowed
+--INSERT INTO gtest12 VALUES (3, 30), (4, 40); -- allowed (does not actually
invoke the function)
+--SELECT a, c FROM gtest12; -- currently not allowed because of function
permissions, should arguably be allowed
RESET ROLE;
-DROP FUNCTION gf1(int); -- fail
-DROP TABLE gtest11, gtest12;
+--DROP FUNCTION gf1(int); -- fail
+DROP TABLE gtest11;
+--DROP TABLE gtest12;
DROP FUNCTION gf1(int);
DROP USER regress_user11;
diff --git a/src/test/regress/sql/publication.sql
b/src/test/regress/sql/publication.sql
index 68001de4000..c9e309190df 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -262,6 +262,9 @@ CREATE PUBLICATION testpub6 FOR TABLES IN SCHEMA
testpub_rf_schema2;
RESET client_min_messages;
\dRp+ testpub6
-- fail - virtual generated column uses user-defined function
+-- (Actually, this already fails at CREATE TABLE rather than at CREATE
+-- PUBLICATION, but let's keep the test in case the former gets
+-- relaxed sometime.)
CREATE TABLE testpub_rf_tbl6 (id int PRIMARY KEY, x int, y int GENERATED
ALWAYS AS (x * testpub_rf_func2()) VIRTUAL);
CREATE PUBLICATION testpub7 FOR TABLE testpub_rf_tbl6 WHERE (y > 100);
-- test that SET EXPRESSION is rejected, because it could affect a row filter
@@ -276,7 +279,7 @@ CREATE PUBLICATION testpub8 FOR TABLE testpub_rf_tbl7 WHERE
(y > 100);
DROP TABLE testpub_rf_tbl3;
DROP TABLE testpub_rf_tbl4;
DROP TABLE testpub_rf_tbl5;
-DROP TABLE testpub_rf_tbl6;
+--DROP TABLE testpub_rf_tbl6;
DROP TABLE testpub_rf_schema1.testpub_rf_tbl5;
DROP TABLE testpub_rf_schema2.testpub_rf_tbl6;
DROP SCHEMA testpub_rf_schema1;
base-commit: d0d1bcb1e8b2e324bc243d69ccfce55b25a79f8c
--
2.49.0
