database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-27 12:41:57 +03:00
Code Activity

Make relation-enumerating operations be security-restricted operations.

Browse Source 
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
This commit is contained in:
Noah Misch
2022-05-09 08:35:08 -07:00
parent e5b5a21356
commit ab49ce7c34
10 changed files with 378 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
contrib/amcheck/expected/check_btree.out
View File

@ -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;

21
contrib/amcheck/sql/check_btree.sql
View File

@ -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;

27
contrib/amcheck/verify_nbtree.c
View File

@ -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