Skip to content

Commit 0c3185e

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 8f11369 commit 0c3185e

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
@@ -777,6 +777,10 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
777777

778778
/*
779779
* pgfdw_xact_callback --- cleanup at main-transaction end.
780+
*
781+
* This runs just late enough that it must not enter user-defined code
782+
* locally. (Entering such code on the remote side is fine. Its remote
783+
* COMMIT TRANSACTION may run deferred triggers.)
780784
*/
781785
static void
782786
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
@@ -2094,9 +2094,10 @@ CommitTransaction(void)
20942094

20952095
/*
20962096
* Do pre-commit processing that involves calling user-defined code, such
2097-
* as triggers. Since closing cursors could queue trigger actions,
2098-
* triggers could open cursors, etc, we have to keep looping until there's
2099-
* nothing left to do.
2097+
* as triggers. SECURITY_RESTRICTED_OPERATION contexts must not queue an
2098+
* action that would run here, because that would bypass the sandbox.
2099+
* Since closing cursors could queue trigger actions, triggers could open
2100+
* cursors, etc, we have to keep looping until there's nothing left to do.
21002101
*/
21012102
for (;;)
21022103
{
@@ -2114,16 +2115,16 @@ CommitTransaction(void)
21142115
break;
21152116
}
21162117

2117-
CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
2118-
: XACT_EVENT_PRE_COMMIT);
2119-
21202118
/*
21212119
* The remaining actions cannot call any user-defined code, so it's safe
21222120
* to start shutting down within-transaction services. But note that most
21232121
* of this stuff could still throw an error, which would switch us into
21242122
* the transaction-abort path.
21252123
*/
21262124

2125+
CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
2126+
: XACT_EVENT_PRE_COMMIT);
2127+
21272128
/* If we might have parallel workers, clean them up now. */
21282129
if (IsInParallelMode())
21292130
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"
@@ -65,6 +66,10 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa
6566
*/
6667
if (!(cstmt->options & CURSOR_OPT_HOLD))
6768
RequireTransactionBlock(isTopLevel, "DECLARE CURSOR");
69+
else if (InSecurityRestrictedOperation())
70+
ereport(ERROR,
71+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
72+
errmsg("cannot create a cursor WITH HOLD within security-restricted operation")));
6873

6974
/*
7075
* 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
@@ -4021,6 +4021,7 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
40214021
bool immediate_only)
40224022
{
40234023
bool found = false;
4024+
bool deferred_found = false;
40244025
AfterTriggerEvent event;
40254026
AfterTriggerEventChunk *chunk;
40264027

@@ -4056,13 +4057,24 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
40564057
*/
40574058
if (defer_it && move_list != NULL)
40584059
{
4060+
deferred_found = true;
40594061
/* add it to move_list */
40604062
afterTriggerAddEvent(move_list, event, evtshared);
40614063
/* mark original copy "done" so we don't do it again */
40624064
event->ate_flags |= AFTER_TRIGGER_DONE;
40634065
}
40644066
}
40654067

4068+
/*
4069+
* We could allow deferred triggers if, before the end of the
4070+
* security-restricted operation, we were to verify that a SET CONSTRAINTS
4071+
* ... IMMEDIATE has fired all such triggers. For now, don't bother.
4072+
*/
4073+
if (deferred_found && InSecurityRestrictedOperation())
4074+
ereport(ERROR,
4075+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
4076+
errmsg("cannot fire deferred trigger within security-restricted operation")));
4077+
40664078
return found;
40674079
}
40684080

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

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

1322+
-- security-restricted operations
1323+
\c -
1324+
CREATE ROLE regress_sro_user;
1325+
SET SESSION AUTHORIZATION regress_sro_user;
1326+
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
1327+
'GRANT regress_priv_group2 TO regress_sro_user';
1328+
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1329+
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
1330+
-- REFRESH of this MV will queue a GRANT at end of transaction
1331+
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
1332+
REFRESH MATERIALIZED VIEW sro_mv;
1333+
ERROR: cannot create a cursor WITH HOLD within security-restricted operation
1334+
CONTEXT: SQL function "mv_action" statement 1
1335+
\c -
1336+
REFRESH MATERIALIZED VIEW sro_mv;
1337+
ERROR: cannot create a cursor WITH HOLD within security-restricted operation
1338+
CONTEXT: SQL function "mv_action" statement 1
1339+
SET SESSION AUTHORIZATION regress_sro_user;
1340+
-- INSERT to this table will queue a GRANT at end of transaction
1341+
CREATE TABLE sro_trojan_table ();
1342+
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
1343+
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
1344+
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
1345+
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
1346+
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
1347+
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1348+
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
1349+
REFRESH MATERIALIZED VIEW sro_mv;
1350+
ERROR: cannot fire deferred trigger within security-restricted operation
1351+
CONTEXT: SQL function "mv_action" statement 1
1352+
\c -
1353+
REFRESH MATERIALIZED VIEW sro_mv;
1354+
ERROR: cannot fire deferred trigger within security-restricted operation
1355+
CONTEXT: SQL function "mv_action" statement 1
1356+
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
1357+
ERROR: must have admin option on role "regress_priv_group2"
1358+
CONTEXT: SQL function "unwanted_grant" statement 1
1359+
SQL statement "SELECT unwanted_grant()"
1360+
PL/pgSQL function sro_trojan() line 1 at PERFORM
1361+
SQL function "mv_action" statement 1
1362+
DROP OWNED BY regress_sro_user;
1363+
DROP ROLE regress_sro_user;
13221364
-- Admin options
13231365
SET SESSION AUTHORIZATION regress_priv_user4;
13241366
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
@@ -802,6 +802,40 @@ SELECT has_table_privilege('regress_priv_user3', 'atest4', 'SELECT'); -- false
802802
SELECT has_table_privilege('regress_priv_user1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
803803

804804

805+
-- security-restricted operations
806+
\c -
807+
CREATE ROLE regress_sro_user;
808+
809+
SET SESSION AUTHORIZATION regress_sro_user;
810+
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
811+
'GRANT regress_priv_group2 TO regress_sro_user';
812+
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
813+
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
814+
-- REFRESH of this MV will queue a GRANT at end of transaction
815+
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
816+
REFRESH MATERIALIZED VIEW sro_mv;
817+
\c -
818+
REFRESH MATERIALIZED VIEW sro_mv;
819+
820+
SET SESSION AUTHORIZATION regress_sro_user;
821+
-- INSERT to this table will queue a GRANT at end of transaction
822+
CREATE TABLE sro_trojan_table ();
823+
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
824+
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
825+
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
826+
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
827+
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
828+
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
829+
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
830+
REFRESH MATERIALIZED VIEW sro_mv;
831+
\c -
832+
REFRESH MATERIALIZED VIEW sro_mv;
833+
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
834+
835+
DROP OWNED BY regress_sro_user;
836+
DROP ROLE regress_sro_user;
837+
838+
805839
-- Admin options
806840

807841
SET SESSION AUTHORIZATION regress_priv_user4;

0 commit comments

Comments
 (0)