diff options
author | Noah Misch <noah@leadboat.com> | 2022-05-09 08:35:08 -0700 |
---|---|---|
committer | Noah Misch <noah@leadboat.com> | 2022-05-09 08:35:12 -0700 |
commit | ab49ce7c3414ac19e4afb386d7843ce2d2fb8bda (patch) | |
tree | 87f4681e87f0061a44b3db6f3bd6b7b80ecfc85f /contrib | |
parent | e5b5a21356233739a552063fa70d4f5b245edb9a (diff) | |
download | postgresql-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.out | 23 | ||||
-rw-r--r-- | contrib/amcheck/sql/check_btree.sql | 21 | ||||
-rw-r--r-- | contrib/amcheck/verify_nbtree.c | 27 |
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 |