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

Commit 88743d5

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 35edcc0 commit 88743d5

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
{
@@ -304,12 +302,6 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
304302
LockRelationOid(OIDNewHeap, AccessExclusiveLock);
305303
dest = CreateTransientRelDestReceiver(OIDNewHeap);
306304

307-
/*
308-
* Now lock down security-restricted operations.
309-
*/
310-
SetUserIdAndSecContext(relowner,
311-
save_sec_context | SECURITY_RESTRICTED_OPERATION);
312-
313305
/* Generate the data, if wanted. */
314306
if (!stmt->skipData)
315307
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
@@ -1511,6 +1511,22 @@ CONTEXT: SQL function "unwanted_grant" statement 1
15111511
SQL statement "SELECT unwanted_grant()"
15121512
PL/pgSQL function sro_trojan() line 1 at PERFORM
15131513
SQL function "mv_action" statement 1
1514+
-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
1515+
SET SESSION AUTHORIZATION regress_sro_user;
1516+
CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
1517+
IMMUTABLE LANGUAGE plpgsql AS $$
1518+
BEGIN
1519+
PERFORM unwanted_grant();
1520+
RAISE WARNING 'owned';
1521+
RETURN 1;
1522+
EXCEPTION WHEN OTHERS THEN
1523+
RETURN 2;
1524+
END$$;
1525+
CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
1526+
CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
1527+
\c -
1528+
REFRESH MATERIALIZED VIEW CONCURRENTLY sro_index_mv;
1529+
REFRESH MATERIALIZED VIEW sro_index_mv;
15141530
DROP OWNED BY regress_sro_user;
15151531
DROP ROLE regress_sro_user;
15161532
-- Admin options

src/test/regress/sql/privileges.sql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,23 @@ REFRESH MATERIALIZED VIEW sro_mv;
945945
REFRESH MATERIALIZED VIEW sro_mv;
946946
BEGIN; SET CONSTRAINTS ALL IMMEDIATE; REFRESH MATERIALIZED VIEW sro_mv; COMMIT;
947947

948+
-- REFRESH MATERIALIZED VIEW CONCURRENTLY use of eval_const_expressions()
949+
SET SESSION AUTHORIZATION regress_sro_user;
950+
CREATE FUNCTION unwanted_grant_nofail(int) RETURNS int
951+
IMMUTABLE LANGUAGE plpgsql AS $$
952+
BEGIN
953+
PERFORM unwanted_grant();
954+
RAISE WARNING 'owned';
955+
RETURN 1;
956+
EXCEPTION WHEN OTHERS THEN
957+
RETURN 2;
958+
END$$;
959+
CREATE MATERIALIZED VIEW sro_index_mv AS SELECT 1 AS c;
960+
CREATE UNIQUE INDEX ON sro_index_mv (c) WHERE unwanted_grant_nofail(1) > 0;
961+
\c -
962+
REFRESH MATERIALIZED VIEW CONCURRENTLY sro_index_mv;
963+
REFRESH MATERIALIZED VIEW sro_index_mv;
964+
948965
DROP OWNED BY regress_sro_user;
949966
DROP ROLE regress_sro_user;
950967

0 commit comments

Comments
 (0)