Fix RLS policy usage in MERGE.

If MERGE executes an UPDATE action on a table with row-level security,
the code incorrectly applied the WITH CHECK clauses from the target
table's INSERT policies to new rows, instead of the clauses from the
table's UPDATE policies. In addition, it failed to check new rows
against the target table's SELECT policies, if SELECT permissions were
required (likely to always be the case).

In addition, if MERGE executes a DO NOTHING action for matched rows,
the code incorrectly applied the USING clauses from the target table's
DELETE policies to existing target tuples. These policies were applied
as checks that would throw an error, if they did not pass.

Fix this, so that a MERGE UPDATE action applies the same RLS policies
as a plain UPDATE query with a WHERE clause, and a DO NOTHING action
does not apply any RLS checks (other than adding clauses from SELECT
policies to the join).

Back-patch to v15, where MERGE was introduced.

Dean Rasheed, reviewed by Stephen Frost.

Security: CVE-2023-39418

1 files changed, 60 insertions, 25 deletions

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index 5c3fe4eda28..b1620e4625d 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -394,7 +394,11 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 	 * and set them up so that we can enforce the appropriate policy depending
 	 * on the final action we take.
 	 *
-	 * We already fetched the SELECT policies above.
+	 * We already fetched the SELECT policies above, to check existing rows,
+	 * but we must also check that new rows created by UPDATE actions are
+	 * visible, if SELECT rights are required for this relation. We don't do
+	 * this for INSERT actions, since an INSERT command would only do this
+	 * check if it had a RETURNING list, and MERGE does not support RETURNING.
 	 *
 	 * We don't push the UPDATE/DELETE USING quals to the RTE because we don't
 	 * really want to apply them while scanning the relation since we don't
@@ -410,16 +414,20 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 	 */
 	if (commandType == CMD_MERGE)
 	{
-		List	   *merge_permissive_policies;
-		List	   *merge_restrictive_policies;
+		List	   *merge_update_permissive_policies;
+		List	   *merge_update_restrictive_policies;
+		List	   *merge_delete_permissive_policies;
+		List	   *merge_delete_restrictive_policies;
+		List	   *merge_insert_permissive_policies;
+		List	   *merge_insert_restrictive_policies;
 
 		/*
 		 * Fetch the UPDATE policies and set them up to execute on the
 		 * existing target row before doing UPDATE.
 		 */
 		get_policies_for_relation(rel, CMD_UPDATE, user_id,
-								  &merge_permissive_policies,
-								  &merge_restrictive_policies);
+								  &merge_update_permissive_policies,
+								  &merge_update_restrictive_policies);
 
 		/*
 		 * WCO_RLS_MERGE_UPDATE_CHECK is used to check UPDATE USING quals on
@@ -427,23 +435,59 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 		 */
 		add_with_check_options(rel, rt_index,
 							   WCO_RLS_MERGE_UPDATE_CHECK,
-							   merge_permissive_policies,
-							   merge_restrictive_policies,
+							   merge_update_permissive_policies,
+							   merge_update_restrictive_policies,
 							   withCheckOptions,
 							   hasSubLinks,
 							   true);
 
+		/* Enforce the WITH CHECK clauses of the UPDATE policies */
+		add_with_check_options(rel, rt_index,
+							   WCO_RLS_UPDATE_CHECK,
+							   merge_update_permissive_policies,
+							   merge_update_restrictive_policies,
+							   withCheckOptions,
+							   hasSubLinks,
+							   false);
+
+		/*
+		 * Add ALL/SELECT policies as WCO_RLS_UPDATE_CHECK WCOs, to ensure
+		 * that the updated row is visible when executing an UPDATE action, if
+		 * SELECT rights are required for this relation.
+		 */
+		if (perminfo->requiredPerms & ACL_SELECT)
+		{
+			List	   *merge_select_permissive_policies;
+			List	   *merge_select_restrictive_policies;
+
+			get_policies_for_relation(rel, CMD_SELECT, user_id,
+									  &merge_select_permissive_policies,
+									  &merge_select_restrictive_policies);
+			add_with_check_options(rel, rt_index,
+								   WCO_RLS_UPDATE_CHECK,
+								   merge_select_permissive_policies,
+								   merge_select_restrictive_policies,
+								   withCheckOptions,
+								   hasSubLinks,
+								   true);
+		}
+
 		/*
-		 * Same with DELETE policies.
+		 * Fetch the DELETE policies and set them up to execute on the
+		 * existing target row before doing DELETE.
 		 */
 		get_policies_for_relation(rel, CMD_DELETE, user_id,
-								  &merge_permissive_policies,
-								  &merge_restrictive_policies);
+								  &merge_delete_permissive_policies,
+								  &merge_delete_restrictive_policies);
 
+		/*
+		 * WCO_RLS_MERGE_DELETE_CHECK is used to check DELETE USING quals on
+		 * the existing target row.
+		 */
 		add_with_check_options(rel, rt_index,
 							   WCO_RLS_MERGE_DELETE_CHECK,
-							   merge_permissive_policies,
-							   merge_restrictive_policies,
+							   merge_delete_permissive_policies,
+							   merge_delete_restrictive_policies,
 							   withCheckOptions,
 							   hasSubLinks,
 							   true);
@@ -454,22 +498,13 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 		 * withCheckOptions.
 		 */
 		get_policies_for_relation(rel, CMD_INSERT, user_id,
-								  &merge_permissive_policies,
-								  &merge_restrictive_policies);
+								  &merge_insert_permissive_policies,
+								  &merge_insert_restrictive_policies);
 
 		add_with_check_options(rel, rt_index,
 							   WCO_RLS_INSERT_CHECK,
-							   merge_permissive_policies,
-							   merge_restrictive_policies,
-							   withCheckOptions,
-							   hasSubLinks,
-							   false);
-
-		/* Enforce the WITH CHECK clauses of the UPDATE policies */
-		add_with_check_options(rel, rt_index,
-							   WCO_RLS_UPDATE_CHECK,
-							   merge_permissive_policies,
-							   merge_restrictive_policies,
+							   merge_insert_permissive_policies,
+							   merge_insert_restrictive_policies,
 							   withCheckOptions,
 							   hasSubLinks,
 							   false);