Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context

The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are
correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for
creating the temporary "diff" table, because you cannot create
temporary tables in SRO mode. But creating the temporary "diff" table
is a pretty complex CTAS command that selects from another temporary
table created earlier in the command. If you can cajole that CTAS
command to execute code defined by the table owner, the table owner
can run code with the privileges of the user running the REFRESH
command.

The proof-of-concept reported to the security team relied on CREATE
RULE to convert the internally-built temp table to a view. That's not
possible since commit b23cd185fd, and I was not able to find a
different way to turn the SELECT on the temp table into code
execution, so as far as I know this is only exploitable in v15 and
below. That's a fiddly assumption though, so apply this patch to
master and all stable versions.

Thanks to Pedro Gallegos for the report.

Security: CVE-2023-5869
Reviewed-by: Noah Misch

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 1dcfbe879ba839167d4722eb903dbb00bd8ee46e..b1a94443612e0fc3fbf106c754f99d80f867d7db 100644 (file)
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -656,13 +656,35 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
                           SPI_getvalue(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1))));
    }
 
+   /*
+    * Create the temporary "diff" table.
+    *
+    * Temporarily switch out of the SECURITY_RESTRICTED_OPERATION context,
+    * because you cannot create temp tables in SRO context.  For extra
+    * paranoia, add the composite type column only after switching back to
+    * SRO context.
+    */
    SetUserIdAndSecContext(relowner,
                           save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
+   resetStringInfo(&querybuf);
+   appendStringInfo(&querybuf,
+                    "CREATE TEMP TABLE %s (tid pg_catalog.tid)",
+                    diffname);
+   if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY)
+       elog(ERROR, "SPI_exec failed: %s", querybuf.data);
+   SetUserIdAndSecContext(relowner,
+                          save_sec_context | SECURITY_RESTRICTED_OPERATION);
+   resetStringInfo(&querybuf);
+   appendStringInfo(&querybuf,
+                    "ALTER TABLE %s ADD COLUMN newdata %s",
+                    diffname, tempname);
+   if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY)
+       elog(ERROR, "SPI_exec failed: %s", querybuf.data);
 
-   /* Start building the query for creating the diff table. */
+   /* Start building the query for populating the diff table. */
    resetStringInfo(&querybuf);
    appendStringInfo(&querybuf,
-                    "CREATE TEMP TABLE %s AS "
+                    "INSERT INTO %s "
                     "SELECT mv.ctid AS tid, newdata.*::%s AS newdata "
                     "FROM %s mv FULL JOIN %s newdata ON (",
                     diffname, tempname, matviewname, tempname);
@@ -788,13 +810,10 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
                           "WHERE newdata.* IS NULL OR mv.* IS NULL "
                           "ORDER BY tid");
 
-   /* Create the temporary "diff" table. */
-   if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY)
+   /* Populate the temporary "diff" table. */
+   if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT)
        elog(ERROR, "SPI_exec failed: %s", querybuf.data);
 
-   SetUserIdAndSecContext(relowner,
-                          save_sec_context | SECURITY_RESTRICTED_OPERATION);
-
    /*
     * We have no further use for data from the "full-data" temp table, but we
     * must keep it around because its type is referenced from the diff table.