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

Commit a9d0f1c

Browse files
committed
Fix superuser concurrent refresh of matview owned by another.
Use SECURITY_LOCAL_USERID_CHANGE while building temporary tables; only escalate to SECURITY_RESTRICTED_OPERATION while potentially running user-supplied code. The more secure mode was preventing temp table creation. Add regression tests to cover this problem. This fixes Bug #11208 reported by Bruno Emanuel de Andrade Silva. Backpatch to 9.4, where the bug was introduced.
1 parent 5569d75 commit a9d0f1c

File tree

3 files changed

+70
-48
lines changed

3 files changed

+70
-48
lines changed

src/backend/commands/matview.c

+45-48
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,13 @@ static void transientrel_receive(TupleTableSlot *slot, DestReceiver *self);
5959
static void transientrel_shutdown(DestReceiver *self);
6060
static void transientrel_destroy(DestReceiver *self);
6161
static void refresh_matview_datafill(DestReceiver *dest, Query *query,
62-
const char *queryString, Oid relowner);
62+
const char *queryString);
6363

6464
static char *make_temptable_name_n(char *tempname, int n);
6565
static void mv_GenerateOper(StringInfo buf, Oid opoid);
6666

67-
static void refresh_by_match_merge(Oid matviewOid, Oid tempOid);
67+
static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
68+
int save_sec_context);
6869
static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap);
6970

7071
static void OpenMatViewIncrementalMaintenance(void);
@@ -142,12 +143,15 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
142143
List *actions;
143144
Query *dataQuery;
144145
Oid tableSpace;
145-
Oid owner;
146+
Oid relowner;
146147
Oid OIDNewHeap;
147148
DestReceiver *dest;
148149
bool concurrent;
149150
LOCKMODE lockmode;
150151
char relpersistence;
152+
Oid save_userid;
153+
int save_sec_context;
154+
int save_nestlevel;
151155

152156
/* Determine strength of lock needed. */
153157
concurrent = stmt->concurrent;
@@ -232,6 +236,19 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
232236
*/
233237
SetMatViewPopulatedState(matviewRel, !stmt->skipData);
234238

239+
relowner = matviewRel->rd_rel->relowner;
240+
241+
/*
242+
* Switch to the owner's userid, so that any functions are run as that
243+
* user. Also arrange to make GUC variable changes local to this command.
244+
* Don't lock it down too tight to create a temporary table just yet. We
245+
* will switch modes when we are about to execute user code.
246+
*/
247+
GetUserIdAndSecContext(&save_userid, &save_sec_context);
248+
SetUserIdAndSecContext(relowner,
249+
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
250+
save_nestlevel = NewGUCNestLevel();
251+
235252
/* Concurrent refresh builds new data in temp tablespace, and does diff. */
236253
if (concurrent)
237254
{
@@ -244,8 +261,6 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
244261
relpersistence = matviewRel->rd_rel->relpersistence;
245262
}
246263

247-
owner = matviewRel->rd_rel->relowner;
248-
249264
/*
250265
* Create the transient table that will receive the regenerated data. Lock
251266
* it against access by any other process until commit (by which time it
@@ -256,9 +271,15 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
256271
LockRelationOid(OIDNewHeap, AccessExclusiveLock);
257272
dest = CreateTransientRelDestReceiver(OIDNewHeap);
258273

274+
/*
275+
* Now lock down security-restricted operations.
276+
*/
277+
SetUserIdAndSecContext(relowner,
278+
save_sec_context | SECURITY_RESTRICTED_OPERATION);
279+
259280
/* Generate the data, if wanted. */
260281
if (!stmt->skipData)
261-
refresh_matview_datafill(dest, dataQuery, queryString, owner);
282+
refresh_matview_datafill(dest, dataQuery, queryString);
262283

263284
heap_close(matviewRel, NoLock);
264285

@@ -269,7 +290,8 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
269290

270291
PG_TRY();
271292
{
272-
refresh_by_match_merge(matviewOid, OIDNewHeap);
293+
refresh_by_match_merge(matviewOid, OIDNewHeap, relowner,
294+
save_sec_context);
273295
}
274296
PG_CATCH();
275297
{
@@ -282,6 +304,12 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
282304
else
283305
refresh_by_heap_swap(matviewOid, OIDNewHeap);
284306

307+
/* Roll back any GUC changes */
308+
AtEOXact_GUC(false, save_nestlevel);
309+
310+
/* Restore userid and security context */
311+
SetUserIdAndSecContext(save_userid, save_sec_context);
312+
285313
return matviewOid;
286314
}
287315

@@ -290,26 +318,13 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
290318
*/
291319
static void
292320
refresh_matview_datafill(DestReceiver *dest, Query *query,
293-
const char *queryString, Oid relowner)
321+
const char *queryString)
294322
{
295323
List *rewritten;
296324
PlannedStmt *plan;
297325
QueryDesc *queryDesc;
298-
Oid save_userid;
299-
int save_sec_context;
300-
int save_nestlevel;
301326
Query *copied_query;
302327

303-
/*
304-
* Switch to the owner's userid, so that any functions are run as that
305-
* user. Also lock down security-restricted operations and arrange to
306-
* make GUC variable changes local to this command.
307-
*/
308-
GetUserIdAndSecContext(&save_userid, &save_sec_context);
309-
SetUserIdAndSecContext(relowner,
310-
save_sec_context | SECURITY_RESTRICTED_OPERATION);
311-
save_nestlevel = NewGUCNestLevel();
312-
313328
/* Lock and rewrite, using a copy to preserve the original query. */
314329
copied_query = copyObject(query);
315330
AcquireRewriteLocks(copied_query, true, false);
@@ -353,12 +368,6 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
353368
FreeQueryDesc(queryDesc);
354369

355370
PopActiveSnapshot();
356-
357-
/* Roll back any GUC changes */
358-
AtEOXact_GUC(false, save_nestlevel);
359-
360-
/* Restore userid and security context */
361-
SetUserIdAndSecContext(save_userid, save_sec_context);
362371
}
363372

364373
DestReceiver *
@@ -529,7 +538,8 @@ mv_GenerateOper(StringInfo buf, Oid opoid)
529538
* this command.
530539
*/
531540
static void
532-
refresh_by_match_merge(Oid matviewOid, Oid tempOid)
541+
refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
542+
int save_sec_context)
533543
{
534544
StringInfoData querybuf;
535545
Relation matviewRel;
@@ -543,9 +553,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid)
543553
ListCell *indexoidscan;
544554
int16 relnatts;
545555
bool *usedForQual;
546-
Oid save_userid;
547-
int save_sec_context;
548-
int save_nestlevel;
549556

550557
initStringInfo(&querybuf);
551558
matviewRel = heap_open(matviewOid, NoLock);
@@ -596,6 +603,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid)
596603
SPI_getvalue(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1))));
597604
}
598605

606+
SetUserIdAndSecContext(relowner,
607+
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
608+
599609
/* Start building the query for creating the diff table. */
600610
resetStringInfo(&querybuf);
601611
appendStringInfo(&querybuf,
@@ -690,9 +700,12 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid)
690700
if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY)
691701
elog(ERROR, "SPI_exec failed: %s", querybuf.data);
692702

703+
SetUserIdAndSecContext(relowner,
704+
save_sec_context | SECURITY_RESTRICTED_OPERATION);
705+
693706
/*
694707
* We have no further use for data from the "full-data" temp table, but we
695-
* must keep it around because its type is reference from the diff table.
708+
* must keep it around because its type is referenced from the diff table.
696709
*/
697710

698711
/* Analyze the diff table. */
@@ -703,16 +716,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid)
703716

704717
OpenMatViewIncrementalMaintenance();
705718

706-
/*
707-
* Switch to the owner's userid, so that any functions are run as that
708-
* user. Also lock down security-restricted operations and arrange to
709-
* make GUC variable changes local to this command.
710-
*/
711-
GetUserIdAndSecContext(&save_userid, &save_sec_context);
712-
SetUserIdAndSecContext(matviewRel->rd_rel->relowner,
713-
save_sec_context | SECURITY_RESTRICTED_OPERATION);
714-
save_nestlevel = NewGUCNestLevel();
715-
716719
/* Deletes must come before inserts; do them first. */
717720
resetStringInfo(&querybuf);
718721
appendStringInfo(&querybuf,
@@ -733,12 +736,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid)
733736
if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT)
734737
elog(ERROR, "SPI_exec failed: %s", querybuf.data);
735738

736-
/* Roll back any GUC changes */
737-
AtEOXact_GUC(false, save_nestlevel);
738-
739-
/* Restore userid and security context */
740-
SetUserIdAndSecContext(save_userid, save_sec_context);
741-
742739
/* We're done maintaining the materialized view. */
743740
CloseMatViewIncrementalMaintenance();
744741
heap_close(tempRel, NoLock);

src/test/regress/expected/matview.out

+12
Original file line numberDiff line numberDiff line change
@@ -502,3 +502,15 @@ SELECT * FROM mv_v;
502502

503503
DROP TABLE v CASCADE;
504504
NOTICE: drop cascades to materialized view mv_v
505+
-- make sure running as superuser works when MV owned by another role (bug #11208)
506+
CREATE ROLE user_dw;
507+
SET ROLE user_dw;
508+
CREATE TABLE foo_data AS SELECT i, md5(random()::text)
509+
FROM generate_series(1, 10) i;
510+
CREATE MATERIALIZED VIEW mv_foo AS SELECT * FROM foo_data;
511+
CREATE UNIQUE INDEX ON mv_foo (i);
512+
RESET ROLE;
513+
REFRESH MATERIALIZED VIEW mv_foo;
514+
REFRESH MATERIALIZED VIEW CONCURRENTLY mv_foo;
515+
DROP OWNED BY user_dw CASCADE;
516+
DROP ROLE user_dw;

src/test/regress/sql/matview.sql

+13
Original file line numberDiff line numberDiff line change
@@ -194,3 +194,16 @@ DELETE FROM v WHERE EXISTS ( SELECT * FROM mv_v WHERE mv_v.a = v.a );
194194
SELECT * FROM v;
195195
SELECT * FROM mv_v;
196196
DROP TABLE v CASCADE;
197+
198+
-- make sure running as superuser works when MV owned by another role (bug #11208)
199+
CREATE ROLE user_dw;
200+
SET ROLE user_dw;
201+
CREATE TABLE foo_data AS SELECT i, md5(random()::text)
202+
FROM generate_series(1, 10) i;
203+
CREATE MATERIALIZED VIEW mv_foo AS SELECT * FROM foo_data;
204+
CREATE UNIQUE INDEX ON mv_foo (i);
205+
RESET ROLE;
206+
REFRESH MATERIALIZED VIEW mv_foo;
207+
REFRESH MATERIALIZED VIEW CONCURRENTLY mv_foo;
208+
DROP OWNED BY user_dw CASCADE;
209+
DROP ROLE user_dw;

0 commit comments

Comments
 (0)