Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 0abc1a0

Browse files
committed
In REFRESH MATERIALIZED VIEW, set user ID before running user code.
It intended to, but did not, achieve this. Adopt the new standard of setting user ID just after locking the relation. Back-patch to v10 (all supported versions). Reviewed by Simon Riggs. Reported by Alvaro Herrera. Security: CVE-2022-1552
1 parent a117ceb commit 0abc1a0

File tree

3 files changed

+44
-19
lines changed

3 files changed

+44
-19
lines changed

src/backend/commands/matview.c

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,17 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
167167
lockmode, 0,
168168
RangeVarCallbackOwnsTable, NULL);
169169
matviewRel = table_open(matviewOid, NoLock);
170+
relowner = matviewRel->rd_rel->relowner;
171+
172+
/*
173+
* Switch to the owner's userid, so that any functions are run as that
174+
* user. Also lock down security-restricted operations and arrange to
175+
* make GUC variable changes local to this command.
176+
*/
177+
GetUserIdAndSecContext(&save_userid, &save_sec_context);
178+
SetUserIdAndSecContext(relowner,
179+
save_sec_context | SECURITY_RESTRICTED_OPERATION);
180+
save_nestlevel = NewGUCNestLevel();
170181

171182
/* Make sure it is a materialized view. */
172183
if (matviewRel->rd_rel->relkind != RELKIND_MATVIEW)
@@ -269,19 +280,6 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
269280
*/
270281
SetMatViewPopulatedState(matviewRel, !stmt->skipData);
271282

272-
relowner = matviewRel->rd_rel->relowner;
273-
274-
/*
275-
* Switch to the owner's userid, so that any functions are run as that
276-
* user. Also arrange to make GUC variable changes local to this command.
277-
* Don't lock it down too tight to create a temporary table just yet. We
278-
* will switch modes when we are about to execute user code.
279-
*/
280-
GetUserIdAndSecContext(&save_userid, &save_sec_context);
281-
SetUserIdAndSecContext(relowner,
282-
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
283-
save_nestlevel = NewGUCNestLevel();
284-
285283
/* Concurrent refresh builds new data in temp tablespace, and does diff. */
286284
if (concurrent)
287285
{
@@ -305,12 +303,6 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
305303
LockRelationOid(OIDNewHeap, AccessExclusiveLock);
306304
dest = CreateTransientRelDestReceiver(OIDNewHeap);
307305

308-
/*
309-
* Now lock down security-restricted operations.
310-
*/
311-
SetUserIdAndSecContext(relowner,
312-
save_sec_context | SECURITY_RESTRICTED_OPERATION);
313-
314306
/* Generate the data, if wanted. */
315307
if (!stmt->skipData)
316308
processed = refresh_matview_datafill(dest, dataQuery, queryString);

src/test/regress/expected/privileges.out

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,6 +1684,22 @@ CONTEXT: SQL function "unwanted_grant" statement 1
16841684
SQL statement "SELECT unwanted_grant()"
16851685
PL/pgSQL function sro_trojan() line 1 at PERFORM
16861686
SQL function "mv_action" statement 1
1687+
-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
1688+
SET SESSION AUTHORIZATION regress_sro_user;
1689+
CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
1690+
IMMUTABLE LANGUAGE plpgsql AS $$
1691+
BEGIN
1692+
PERFORM unwanted_grant();
1693+
RAISE WARNING 'owned';
1694+
RETURN 1;
1695+
EXCEPTION WHEN OTHERS THEN
1696+
RETURN 2;
1697+
END$$;
1698+
CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
1699+
CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
1700+
\c -
1701+
REFRESH MATERIALIZED VIEW CONCURRENTLY sro_index_mv;
1702+
REFRESH MATERIALIZED VIEW sro_index_mv;
16871703
DROP OWNED BY regress_sro_user;
16881704
DROP ROLE regress_sro_user;
16891705
-- Admin options

src/test/regress/sql/privileges.sql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,6 +1116,23 @@ REFRESH MATERIALIZED VIEW sro_mv;
11161116
REFRESH MATERIALIZED VIEW sro_mv;
11171117
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
11181118

1119+
-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
1120+
SET SESSION AUTHORIZATION regress_sro_user;
1121+
CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
1122+
IMMUTABLE LANGUAGE plpgsql AS $$
1123+
BEGIN
1124+
PERFORM unwanted_grant();
1125+
RAISE WARNING 'owned';
1126+
RETURN 1;
1127+
EXCEPTION WHEN OTHERS THEN
1128+
RETURN 2;
1129+
END$$;
1130+
CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
1131+
CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
1132+
\c -
1133+
REFRESH MATERIALIZED VIEW CONCURRENTLY sro_index_mv;
1134+
REFRESH MATERIALIZED VIEW sro_index_mv;
1135+
11191136
DROP OWNED BY regress_sro_user;
11201137
DROP ROLE regress_sro_user;
11211138

0 commit comments

Comments
 (0)