Skip to content

Commit aefc625

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 7a581f3 commit aefc625

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

668668
/*
669669
* pgfdw_xact_callback --- cleanup at main-transaction end.
670+
*
671+
* This runs just late enough that it must not enter user-defined code
672+
* locally. (Entering such code on the remote side is fine. Its remote
673+
* COMMIT TRANSACTION may run deferred triggers.)
670674
*/
671675
static void
672676
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
@@ -1976,9 +1976,10 @@ CommitTransaction(void)
19761976

19771977
/*
19781978
* Do pre-commit processing that involves calling user-defined code, such
1979-
* as triggers. Since closing cursors could queue trigger actions,
1980-
* triggers could open cursors, etc, we have to keep looping until there's
1981-
* nothing left to do.
1979+
* as triggers. SECURITY_RESTRICTED_OPERATION contexts must not queue an
1980+
* action that would run here, because that would bypass the sandbox.
1981+
* Since closing cursors could queue trigger actions, triggers could open
1982+
* cursors, etc, we have to keep looping until there's nothing left to do.
19821983
*/
19831984
for (;;)
19841985
{
@@ -1996,16 +1997,16 @@ CommitTransaction(void)
19961997
break;
19971998
}
19981999

1999-
CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
2000-
: XACT_EVENT_PRE_COMMIT);
2001-
20022000
/*
20032001
* The remaining actions cannot call any user-defined code, so it's safe
20042002
* to start shutting down within-transaction services. But note that most
20052003
* of this stuff could still throw an error, which would switch us into
20062004
* the transaction-abort path.
20072005
*/
20082006

2007+
CallXactCallbacks(is_parallel_worker ? XACT_EVENT_PARALLEL_PRE_COMMIT
2008+
: XACT_EVENT_PRE_COMMIT);
2009+
20092010
/* If we might have parallel workers, clean them up now. */
20102011
if (IsInParallelMode())
20112012
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 "tcop/pquery.h"
3132
#include "utils/memutils.h"
3233
#include "utils/snapmgr.h"
@@ -67,6 +68,10 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
6768
*/
6869
if (!(cstmt->options & CURSOR_OPT_HOLD))
6970
RequireTransactionChain(isTopLevel, "DECLARE CURSOR");
71+
else if (InSecurityRestrictedOperation())
72+
ereport(ERROR,
73+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
74+
errmsg("cannot create a cursor WITH HOLD within security-restricted operation")));
7075

7176
/*
7277
* Create a portal and copy the plan and queryString into its memory.

‎src/backend/commands/trigger.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3704,6 +3704,7 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
37043704
bool immediate_only)
37053705
{
37063706
bool found = false;
3707+
bool deferred_found = false;
37073708
AfterTriggerEvent event;
37083709
AfterTriggerEventChunk *chunk;
37093710

@@ -3739,13 +3740,24 @@ afterTriggerMarkEvents(AfterTriggerEventList *events,
37393740
*/
37403741
if (defer_it && move_list != NULL)
37413742
{
3743+
deferred_found = true;
37423744
/* add it to move_list */
37433745
afterTriggerAddEvent(move_list, event, evtshared);
37443746
/* mark original copy "done" so we don't do it again */
37453747
event->ate_flags |= AFTER_TRIGGER_DONE;
37463748
}
37473749
}
37483750

3751+
/*
3752+
* We could allow deferred triggers if, before the end of the
3753+
* security-restricted operation, we were to verify that a SET CONSTRAINTS
3754+
* ... IMMEDIATE has fired all such triggers. For now, don't bother.
3755+
*/
3756+
if (deferred_found && InSecurityRestrictedOperation())
3757+
ereport(ERROR,
3758+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
3759+
errmsg("cannot fire deferred trigger within security-restricted operation")));
3760+
37493761
return found;
37503762
}
37513763

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,48 @@ SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION')
12341234
t
12351235
(1 row)
12361236

1237+
-- security-restricted operations
1238+
\c -
1239+
CREATE ROLE regress_sro_user;
1240+
SET SESSION AUTHORIZATION regress_sro_user;
1241+
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
1242+
'GRANT regressgroup2 TO regress_sro_user';
1243+
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1244+
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
1245+
-- REFRESH of this MV will queue a GRANT at end of transaction
1246+
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
1247+
REFRESH MATERIALIZED VIEW sro_mv;
1248+
ERROR: cannot create a cursor WITH HOLD within security-restricted operation
1249+
CONTEXT: SQL function "mv_action" statement 1
1250+
\c -
1251+
REFRESH MATERIALIZED VIEW sro_mv;
1252+
ERROR: cannot create a cursor WITH HOLD within security-restricted operation
1253+
CONTEXT: SQL function "mv_action" statement 1
1254+
SET SESSION AUTHORIZATION regress_sro_user;
1255+
-- INSERT to this table will queue a GRANT at end of transaction
1256+
CREATE TABLE sro_trojan_table ();
1257+
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
1258+
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
1259+
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
1260+
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
1261+
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
1262+
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
1263+
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
1264+
REFRESH MATERIALIZED VIEW sro_mv;
1265+
ERROR: cannot fire deferred trigger within security-restricted operation
1266+
CONTEXT: SQL function "mv_action" statement 1
1267+
\c -
1268+
REFRESH MATERIALIZED VIEW sro_mv;
1269+
ERROR: cannot fire deferred trigger within security-restricted operation
1270+
CONTEXT: SQL function "mv_action" statement 1
1271+
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
1272+
ERROR: must have admin option on role "regressgroup2"
1273+
CONTEXT: SQL function "unwanted_grant" statement 1
1274+
SQL statement "SELECT unwanted_grant()"
1275+
PL/pgSQL function sro_trojan() line 1 at PERFORM
1276+
SQL function "mv_action" statement 1
1277+
DROP OWNED BY regress_sro_user;
1278+
DROP ROLE regress_sro_user;
12371279
-- Admin options
12381280
SET SESSION AUTHORIZATION regressuser4;
12391281
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
@@ -752,6 +752,40 @@ SELECT has_table_privilege('regressuser3', 'atest4', 'SELECT'); -- false
752752
SELECT has_table_privilege('regressuser1', 'atest4', 'SELECT WITH GRANT OPTION'); -- true
753753

754754

755+
-- security-restricted operations
756+
\c -
757+
CREATE ROLE regress_sro_user;
758+
759+
SET SESSION AUTHORIZATION regress_sro_user;
760+
CREATE FUNCTION unwanted_grant() RETURNS void LANGUAGE sql AS
761+
'GRANT regressgroup2 TO regress_sro_user';
762+
CREATE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
763+
'DECLARE c CURSOR WITH HOLD FOR SELECT unwanted_grant(); SELECT true';
764+
-- REFRESH of this MV will queue a GRANT at end of transaction
765+
CREATE MATERIALIZED VIEW sro_mv AS SELECT mv_action() WITH NO DATA;
766+
REFRESH MATERIALIZED VIEW sro_mv;
767+
\c -
768+
REFRESH MATERIALIZED VIEW sro_mv;
769+
770+
SET SESSION AUTHORIZATION regress_sro_user;
771+
-- INSERT to this table will queue a GRANT at end of transaction
772+
CREATE TABLE sro_trojan_table ();
773+
CREATE FUNCTION sro_trojan() RETURNS trigger LANGUAGE plpgsql AS
774+
'BEGIN PERFORM unwanted_grant(); RETURN NULL; END';
775+
CREATE CONSTRAINT TRIGGER t AFTER INSERT ON sro_trojan_table
776+
INITIALLY DEFERRED FOR EACH ROW EXECUTE PROCEDURE sro_trojan();
777+
-- Now, REFRESH will issue such an INSERT, queueing the GRANT
778+
CREATE OR REPLACE FUNCTION mv_action() RETURNS bool LANGUAGE sql AS
779+
'INSERT INTO sro_trojan_table DEFAULT VALUES; SELECT true';
780+
REFRESH MATERIALIZED VIEW sro_mv;
781+
\c -
782+
REFRESH MATERIALIZED VIEW sro_mv;
783+
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
784+
785+
DROP OWNED BY regress_sro_user;
786+
DROP ROLE regress_sro_user;
787+
788+
755789
-- Admin options
756790

757791
SET SESSION AUTHORIZATION regressuser4;

0 commit comments

Comments
 (0)