🌐 AI搜索 & 代理 主页
Skip to content

Commit ac8f624

Browse files
nmischnmisch
committed
In security-restricted operations, block enqueue of at-commit user code.
Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred triggers within index expressions and materialized view queries. 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. One can work around the vulnerability by disabling autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW. (Don't restore from pg_dump, since it runs some of those commands.) Plain VACUUM (without FULL) is safe, and all commands are fine when a trusted user owns the target object. Performance may degrade quickly under this workaround, however. Back-patch to 9.5 (all supported versions). Reviewed by Robert Haas. Reported by Etienne Stalmans. Security: CVE-2020-25695
1 parent a98b461 commit ac8f624

File tree

6 files changed

+104
-6
lines changed

6 files changed

+104
-6
lines changed

‎contrib/postgres_fdw/connection.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,10 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
646646

647647
/*
648648
* pgfdw_xact_callback --- cleanup at main-transaction end.
649+
*
650+
* This runs just late enough that it must not enter user-defined code
651+
* locally. (Entering such code on the remote side is fine. Its remote
652+
* COMMIT TRANSACTION may run deferred triggers.)
649653
*/
650654
static void
651655
pgfdw_xact_callback(XactEvent event, void *arg)

‎src/backend/access/transam/xact.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2065,9 +2065,10 @@ CommitTransaction(void)
20652065

20662066
/*
20672067
* Do pre-commit processing that involves calling user-defined code, such
2068-
* as triggers. Since closing cursors could queue trigger actions,
2069-
* triggers could open cursors, etc, we have to keep looping until there's
2070-
* nothing left to do.
2068+
* as triggers. SECURITY_RESTRICTED_OPERATION contexts must not queue an
2069+
* action that would run here, because that would bypass the sandbox.
2070+
* Since closing cursors could queue trigger actions, triggers could open
2071+
* cursors, etc, we have to keep looping until there's nothing left to do.
20712072
*/
20722073
for (;;)
20732074
{
@@ -2085,16 +2086,16 @@ CommitTransaction(void)
20852086
break;
20862087
}
20872088

2088-
CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
2089-
: XACT_EVENT_PRE_COMMIT);
2090-
20912089
/*
20922090
* The remaining actions cannot call any user-defined code, so it's safe
20932091
* to start shutting down within-transaction services. But note that most
20942092
* of this stuff could still throw an error, which would switch us into
20952093
* the transaction-abort path.
20962094
*/
20972095

2096+
CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
2097+
: XACT_EVENT_PRE_COMMIT);
2098+
20982099
/* If we might have parallel workers, clean them up now. */
20992100
if (IsInParallelMode())
21002101
AtEOXact_Parallel(true);

‎src/backend/commands/portalcmds.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "commands/portalcmds.h"
2828
#include "executor/executor.h"
2929
#include "executor/tstoreReceiver.h"
30+
#include "miscadmin.h"
3031
#include "rewrite/rewriteHandler.h"
3132
#include "tcop/pquery.h"
3233
#include "tcop/tcopprot.h"
@@ -64,6 +65,10 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params,
6465
*/
6566
if (!(cstmt->options & CURSOR_OPT_HOLD))
6667
RequireTransactionBlock(isTopLevel, "DECLARE CURSOR");
68+
else if (InSecurityRestrictedOperation())
69+
ereport(ERROR,
70+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
71+
errmsg("cannot create a cursor WITH HOLD within security-restricted operation")));
6772

6873
/*
6974
* Parse analysis was done already, but we still have to run the rule

‎src/backend/commands/trigger.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4383,6 +4383,7 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
43834383
bool immediate_only)
43844384
{
43854385
bool found = false;
4386+
bool deferred_found = false;
43864387
AfterTriggerEvent event;
43874388
AfterTriggerEventChunk *chunk;
43884389

@@ -4418,13 +4419,24 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
44184419
*/
44194420
if (defer_it && move_list != NULL)
44204421
{
4422+
deferred_found = true;
44214423
/* add it to move_list */
44224424
afterTriggerAddEvent(move_list, event, evtshared);
44234425
/* mark original copy "done" so we don't do it again */
44244426
event->ate_flags |= AFTER_TRIGGER_DONE;
44254427
}
44264428
}
44274429

4430+
/*
4431+
* We could allow deferred triggers if, before the end of the
4432+
* security-restricted operation, we were to verify that a SET CONSTRAINTS
4433+
* ... IMMEDIATE has fired all such triggers. For now, don't bother.
4434+
*/
4435+
if (deferred_found && InSecurityRestrictedOperation())
4436+
ereport(ERROR,
4437+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
4438+
errmsg("cannot fire deferred trigger within security-restricted operation")));
4439+
44284440
return found;
44294441
}
44304442

‎src/test/regress/expected/privileges.out‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,48 @@ SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OP
12871287
t
12881288
(1 row)
12891289

1290+
-- security-restricted operations
1291+
\c -
1292+
CREATE ROLE regress_sro_user;
1293+
SET SESSION AUTHORIZATION regress_sro_user;
1294+
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
1295+
'GRANT regress_priv_group2 TO regress_sro_user';
1296+
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1297+
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
1298+
-- REFRESH of this MV will queue a GRANT at end of transaction
1299+
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
1300+
REFRESH MATERIALIZED VIEW sro_mv;
1301+
ERROR: cannot create a cursor WITH HOLD within security-restricted operation
1302+
CONTEXT: SQL function "mv_action" statement 1
1303+
\c -
1304+
REFRESH MATERIALIZED VIEW sro_mv;
1305+
ERROR: cannot create a cursor WITH HOLD within security-restricted operation
1306+
CONTEXT: SQL function "mv_action" statement 1
1307+
SET SESSION AUTHORIZATION regress_sro_user;
1308+
-- INSERT to this table will queue a GRANT at end of transaction
1309+
CREATE TABLE sro_trojan_table ();
1310+
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
1311+
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
1312+
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
1313+
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
1314+
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
1315+
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1316+
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
1317+
REFRESH MATERIALIZED VIEW sro_mv;
1318+
ERROR: cannot fire deferred trigger within security-restricted operation
1319+
CONTEXT: SQL function "mv_action" statement 1
1320+
\c -
1321+
REFRESH MATERIALIZED VIEW sro_mv;
1322+
ERROR: cannot fire deferred trigger within security-restricted operation
1323+
CONTEXT: SQL function "mv_action" statement 1
1324+
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
1325+
ERROR: must have admin option on role "regress_priv_group2"
1326+
CONTEXT: SQL function "unwanted_grant" statement 1
1327+
SQL statement "SELECT unwanted_grant()"
1328+
PL/pgSQL function sro_trojan() line 1 at PERFORM
1329+
SQL function "mv_action" statement 1
1330+
DROP OWNED BY regress_sro_user;
1331+
DROP ROLE regress_sro_user;
12901332
-- Admin options
12911333
SET SESSION AUTHORIZATION regress_priv_user4;
12921334
CREATE FUNCTION dogrant_ok() RETURNS void LANGUAGE sql SECURITY DEFINER AS

‎src/test/regress/sql/privileges.sql‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,40 @@ SELECT has_table_privilege('regress_priv_user3', 'atest4', 'SELECT'); -- false
778778
SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
779779

780780

781+
-- security-restricted operations
782+
\c -
783+
CREATE ROLE regress_sro_user;
784+
785+
SET SESSION AUTHORIZATION regress_sro_user;
786+
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
787+
'GRANT regress_priv_group2 TO regress_sro_user';
788+
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
789+
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
790+
-- REFRESH of this MV will queue a GRANT at end of transaction
791+
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
792+
REFRESH MATERIALIZED VIEW sro_mv;
793+
\c -
794+
REFRESH MATERIALIZED VIEW sro_mv;
795+
796+
SET SESSION AUTHORIZATION regress_sro_user;
797+
-- INSERT to this table will queue a GRANT at end of transaction
798+
CREATE TABLE sro_trojan_table ();
799+
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
800+
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
801+
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
802+
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
803+
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
804+
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
805+
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
806+
REFRESH MATERIALIZED VIEW sro_mv;
807+
\c -
808+
REFRESH MATERIALIZED VIEW sro_mv;
809+
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
810+
811+
DROP OWNED BY regress_sro_user;
812+
DROP ROLE regress_sro_user;
813+
814+
781815
-- Admin options
782816

783817
SET SESSION AUTHORIZATION regress_priv_user4;

0 commit comments

Comments
 (0)