From 1ed6f1b9116c9a478a973040319395b58b6648ad Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 19 Nov 2022 12:00:27 -0500 Subject: [PATCH] pg_dump: avoid unsafe function calls in getPolicies(). getPolicies() had the same disease I fixed in other places in commit e3fcbbd62, i.e., it was calling pg_get_expr() for expressions on tables that we don't necessarily have lock on. To fix, restrict the query to only collect interesting rows, rather than doing the filtering on the client side. Back-patch of commit 3e6e86abc. That's been in v15/HEAD long enough to have some confidence about it, so now let's fix the problem in older branches. Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc Discussion: https://postgr.es/m/45c93d57-9973-248e-d2df-e02ca9af48d4@darold.net --- src/bin/pg_dump/pg_dump.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 91b26f5136..db015cf10d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -3549,6 +3549,7 @@ void getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) { PQExpBuffer query; + PQExpBuffer tbloids; PGresult *res; PolicyInfo *polinfo; int i_oid; @@ -3564,15 +3565,17 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) j, ntups; + /* No policies before 9.5 */ if (fout->remoteVersion < 90500) return; query = createPQExpBuffer(); + tbloids = createPQExpBuffer(); /* - * First, check which tables have RLS enabled. We represent RLS being - * enabled on a table by creating a PolicyInfo object with null polname. + * Identify tables of interest, and check which ones have RLS enabled. */ + appendPQExpBufferChar(tbloids, '{'); for (i = 0; i < numTables; i++) { TableInfo *tbinfo = &tblinfo[i]; @@ -3581,9 +3584,23 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) if (!(tbinfo->dobj.dump & DUMP_COMPONENT_POLICY)) continue; + /* It can't have RLS or policies if it's not a table */ + if (tbinfo->relkind != RELKIND_RELATION && + tbinfo->relkind != RELKIND_PARTITIONED_TABLE) + continue; + + /* Add it to the list of table OIDs to be probed below */ + if (tbloids->len > 1) /* do we have more than the '{'? */ + appendPQExpBufferChar(tbloids, ','); + appendPQExpBuffer(tbloids, "%u", tbinfo->dobj.catId.oid); + + /* Is RLS enabled? (That's separate from whether it has policies) */ if (tbinfo->rowsec) { /* + * We represent RLS being enabled on a table by creating a + * PolicyInfo object with null polname. + * * Note: use tableoid 0 so that this object won't be mistaken for * something that pg_depend entries apply to. */ @@ -3603,15 +3620,18 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) polinfo->polwithcheck = NULL; } } + appendPQExpBufferChar(tbloids, '}'); /* - * Now, read all RLS policies, and create PolicyInfo objects for all those - * that are of interest. + * Now, read all RLS policies belonging to the tables of interest, and + * create PolicyInfo objects for them. (Note that we must filter the + * results server-side not locally, because we dare not apply pg_get_expr + * to tables we don't have lock on.) */ pg_log_info("reading row-level security policies"); printfPQExpBuffer(query, - "SELECT oid, tableoid, pol.polrelid, pol.polname, pol.polcmd, "); + "SELECT pol.oid, pol.tableoid, pol.polrelid, pol.polname, pol.polcmd, "); if (fout->remoteVersion >= 100000) appendPQExpBuffer(query, "pol.polpermissive, "); else @@ -3621,7 +3641,9 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from pg_catalog.pg_roles WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, " "pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, " "pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwithcheck " - "FROM pg_catalog.pg_policy pol"); + "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" + "JOIN pg_catalog.pg_policy pol ON (src.tbloid = pol.polrelid)", + tbloids->data); res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); @@ -3645,13 +3667,6 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) Oid polrelid = atooid(PQgetvalue(res, j, i_polrelid)); TableInfo *tbinfo = findTableByOid(polrelid); - /* - * Ignore row security on tables not to be dumped. (This will - * result in some harmless wasted slots in polinfo[].) - */ - if (!(tbinfo->dobj.dump & DUMP_COMPONENT_POLICY)) - continue; - polinfo[j].dobj.objType = DO_POLICY; polinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid)); @@ -3686,6 +3701,7 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) PQclear(res); destroyPQExpBuffer(query); + destroyPQExpBuffer(tbloids); } /* -- 2.11.4.GIT