aboutsummaryrefslogtreecommitdiff
path: root/contrib
diff options
context:
space:
mode:
authorNoah Misch <noah@leadboat.com>2022-05-09 08:35:08 -0700
committerNoah Misch <noah@leadboat.com>2022-05-09 08:35:12 -0700
commitab49ce7c3414ac19e4afb386d7843ce2d2fb8bda (patch)
tree87f4681e87f0061a44b3db6f3bd6b7b80ecfc85f /contrib
parente5b5a21356233739a552063fa70d4f5b245edb9a (diff)
downloadpostgresql-ab49ce7c3414ac19e4afb386d7843ce2d2fb8bda.tar.gz
postgresql-ab49ce7c3414ac19e4afb386d7843ce2d2fb8bda.zip
Make relation-enumerating operations be security-restricted operations.
When a feature enumerates relations and runs functions associated with all found relations, the feature's user shall not need to trust every user having permission to create objects. BRIN-specific functionality in autovacuum neglected to account for this, as did pg_amcheck and CLUSTER. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. CREATE INDEX (not a relation-enumerating operation) and REINDEX protected themselves too late. This change extends to the non-enumerating amcheck interface. Back-patch to v10 (all supported versions). Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2022-1552
Diffstat (limited to 'contrib')
-rw-r--r--contrib/amcheck/expected/check_btree.out23
-rw-r--r--contrib/amcheck/sql/check_btree.sql21
-rw-r--r--contrib/amcheck/verify_nbtree.c27
3 files changed, 71 insertions, 0 deletions
diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out
index 5a3f1ef737c..38791bbc1f4 100644
--- a/contrib/amcheck/expected/check_btree.out
+++ b/contrib/amcheck/expected/check_btree.out
@@ -177,11 +177,34 @@ SELECT bt_index_check('toasty', true);
(1 row)
+--
+-- Check that index expressions and predicates are run as the table's owner
+--
+TRUNCATE bttest_a;
+INSERT INTO bttest_a SELECT * FROM generate_series(1, 1000);
+ALTER TABLE bttest_a OWNER TO regress_bttest_role;
+-- A dummy index function checking current_user
+CREATE FUNCTION ifun(int8) RETURNS int8 AS $$
+BEGIN
+ ASSERT current_user = 'regress_bttest_role',
+ format('ifun(%s) called by %s', $1, current_user);
+ RETURN $1;
+END;
+$$ LANGUAGE plpgsql IMMUTABLE;
+CREATE INDEX bttest_a_expr_idx ON bttest_a ((ifun(id) + ifun(0)))
+ WHERE ifun(id + 10) > ifun(10);
+SELECT bt_index_check('bttest_a_expr_idx', true);
+ bt_index_check
+----------------
+
+(1 row)
+
-- cleanup
DROP TABLE bttest_a;
DROP TABLE bttest_b;
DROP TABLE bttest_multi;
DROP TABLE delete_test_table;
DROP TABLE toast_bug;
+DROP FUNCTION ifun(int8);
DROP OWNED BY regress_bttest_role; -- permissions
DROP ROLE regress_bttest_role;
diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql
index 97a3e1a20d5..033c04b4d05 100644
--- a/contrib/amcheck/sql/check_btree.sql
+++ b/contrib/amcheck/sql/check_btree.sql
@@ -115,11 +115,32 @@ INSERT INTO toast_bug SELECT repeat('a', 2200);
-- Should not get false positive report of corruption:
SELECT bt_index_check('toasty', true);
+--
+-- Check that index expressions and predicates are run as the table's owner
+--
+TRUNCATE bttest_a;
+INSERT INTO bttest_a SELECT * FROM generate_series(1, 1000);
+ALTER TABLE bttest_a OWNER TO regress_bttest_role;
+-- A dummy index function checking current_user
+CREATE FUNCTION ifun(int8) RETURNS int8 AS $$
+BEGIN
+ ASSERT current_user = 'regress_bttest_role',
+ format('ifun(%s) called by %s', $1, current_user);
+ RETURN $1;
+END;
+$$ LANGUAGE plpgsql IMMUTABLE;
+
+CREATE INDEX bttest_a_expr_idx ON bttest_a ((ifun(id) + ifun(0)))
+ WHERE ifun(id + 10) > ifun(10);
+
+SELECT bt_index_check('bttest_a_expr_idx', true);
+
-- cleanup
DROP TABLE bttest_a;
DROP TABLE bttest_b;
DROP TABLE bttest_multi;
DROP TABLE delete_test_table;
DROP TABLE toast_bug;
+DROP FUNCTION ifun(int8);
DROP OWNED BY regress_bttest_role; -- permissions
DROP ROLE regress_bttest_role;
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index c85de7c2b01..f23bc1473b0 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -248,6 +248,9 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
Relation indrel;
Relation heaprel;
LOCKMODE lockmode;
+ Oid save_userid;
+ int save_sec_context;
+ int save_nestlevel;
if (parentcheck)
lockmode = ShareLock;
@@ -264,9 +267,27 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
*/
heapid = IndexGetRelation(indrelid, true);
if (OidIsValid(heapid))
+ {
heaprel = table_open(heapid, lockmode);
+
+ /*
+ * Switch to the table owner's userid, so that any index functions are
+ * run as that user. Also lock down security-restricted operations
+ * and arrange to make GUC variable changes local to this command.
+ */
+ GetUserIdAndSecContext(&save_userid, &save_sec_context);
+ SetUserIdAndSecContext(heaprel->rd_rel->relowner,
+ save_sec_context | SECURITY_RESTRICTED_OPERATION);
+ save_nestlevel = NewGUCNestLevel();
+ }
else
+ {
heaprel = NULL;
+ /* for "gcc -Og" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78394 */
+ save_userid = InvalidOid;
+ save_sec_context = -1;
+ save_nestlevel = -1;
+ }
/*
* Open the target index relations separately (like relation_openrv(), but
@@ -326,6 +347,12 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
heapallindexed, rootdescend);
}
+ /* Roll back any GUC changes executed by index functions */
+ AtEOXact_GUC(false, save_nestlevel);
+
+ /* Restore userid and security context */
+ SetUserIdAndSecContext(save_userid, save_sec_context);
+
/*
* Release locks early. That's ok here because nothing in the called
* routines will trigger shared cache invalidations to be sent, so we can