From 4f3b2a8883c47b6710152a8e157f8a02656d0e68 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Tue, 15 Sep 2015 15:49:31 -0400
Subject: [PATCH] Enforce ALL/SELECT policies in RETURNING for RLS

For the UPDATE/DELETE RETURNING case, filter the records which are not
visible to the user through ALL or SELECT policies from those considered
for the UPDATE or DELETE.  This is similar to how the GRANT system
works, which prevents RETURNING unless the caller has SELECT rights on
the relation.

Per discussion with Robert, Dean, Tom, and Kevin.

Back-patch to 9.5 where RLS was introduced.
---
 src/backend/rewrite/rowsecurity.c         | 47 +++++++++++++++++++++
 src/test/regress/expected/rowsecurity.out | 50 ++++++++++++-----------
 2 files changed, 74 insertions(+), 23 deletions(-)

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index b96c29d8f64..c20a17802b1 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -186,6 +186,33 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 						   securityQuals,
 						   hasSubLinks);
 
+	/*
+	 * For the target relation, when there is a returning list, we need to
+	 * collect up CMD_SELECT policies and add them via add_security_quals.
+	 * This is because, for the RETURNING case, we have to filter any records
+	 * which are not visible through an ALL or SELECT USING policy.
+	 *
+	 * We don't need to worry about the non-target relation case because we are
+	 * checking the ALL and SELECT policies for those relations anyway (see
+	 * above).
+	 */
+	if (root->returningList != NIL &&
+		(commandType == CMD_UPDATE || commandType == CMD_DELETE))
+	{
+		List	   *returning_permissive_policies;
+		List	   *returning_restrictive_policies;
+
+		get_policies_for_relation(rel, CMD_SELECT, user_id,
+								  &returning_permissive_policies,
+								  &returning_restrictive_policies);
+
+		add_security_quals(rt_index,
+						   returning_permissive_policies,
+						   returning_restrictive_policies,
+						   securityQuals,
+						   hasSubLinks);
+	}
+
 	/*
 	 * For INSERT and UPDATE, add withCheckOptions to verify that any new
 	 * records added are consistent with the security policies.  This will use
@@ -233,6 +260,26 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
 								   withCheckOptions,
 								   hasSubLinks);
 
+			/*
+			 * Get and add ALL/SELECT policies, if there is a RETURNING clause,
+			 * also as WCO policies, again, to avoid silently dropping data.
+			 */
+			if (root->returningList != NIL)
+			{
+				List	   *conflict_returning_permissive_policies = NIL;
+				List	   *conflict_returning_restrictive_policies = NIL;
+
+				get_policies_for_relation(rel, CMD_SELECT, user_id,
+									  &conflict_returning_permissive_policies,
+									  &conflict_returning_restrictive_policies);
+				add_with_check_options(rel, rt_index,
+									   WCO_RLS_CONFLICT_CHECK,
+									   conflict_returning_permissive_policies,
+									   conflict_returning_restrictive_policies,
+									   withCheckOptions,
+									   hasSubLinks);
+			}
+
 			/* Enforce the WITH CHECK clauses of the UPDATE policies */
 			add_with_check_options(rel, rt_index,
 								   WCO_RLS_UPDATE_CHECK,
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 6fc80af30ee..7628e52901a 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1246,21 +1246,23 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
 AND f_leak(t2_1.b) AND f_leak(t2_2.b) RETURNING *, t2_1, t2_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t2 t2_1_1
    ->  Nested Loop
          Join Filter: (t2_1.b = t2_2.b)
          ->  Subquery Scan on t2_1
                Filter: f_leak(t2_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t2_1_2
-                           Filter: ((a = 3) AND ((a % 2) = 1))
+               ->  Subquery Scan on t2_1_2
+                     Filter: ((t2_1_2.a % 2) = 1)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t2_1_3
+                                 Filter: ((a = 3) AND ((a % 2) = 1))
          ->  Subquery Scan on t2_2
                Filter: f_leak(t2_2.b)
                ->  Seq Scan on t2 t2_2_1
                      Filter: ((a = 3) AND ((a % 2) = 1))
-(12 rows)
+(14 rows)
 
 UPDATE t2 t2_1 SET b = t2_2.b FROM t2 t2_2
 WHERE t2_1.a = 3 AND t2_2.a = t2_1.a AND t2_2.b = t2_1.b
@@ -1275,8 +1277,8 @@ NOTICE:  f_leak => cde
 EXPLAIN (COSTS OFF) UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
 AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
-                          QUERY PLAN                           
----------------------------------------------------------------
+                             QUERY PLAN                              
+---------------------------------------------------------------------
  Update on t1 t1_1_3
    Update on t1 t1_1_3
    Update on t2 t1_1
@@ -1285,9 +1287,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1.b = t1_2.b)
          ->  Subquery Scan on t1_1
                Filter: f_leak(t1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t1 t1_1_4
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_4
+                     Filter: ((t1_1_4.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t1 t1_1_5
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2
                Filter: f_leak(t1_2.b)
                ->  Append
@@ -1301,9 +1305,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_1.b = t1_2_1.b)
          ->  Subquery Scan on t1_1_1
                Filter: f_leak(t1_1_1.b)
-               ->  LockRows
-                     ->  Seq Scan on t2 t1_1_5
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_6
+                     Filter: ((t1_1_6.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t2 t1_1_7
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_1
                Filter: f_leak(t1_2_1.b)
                ->  Append
@@ -1317,9 +1323,11 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
          Join Filter: (t1_1_2.b = t1_2_2.b)
          ->  Subquery Scan on t1_1_2
                Filter: f_leak(t1_1_2.b)
-               ->  LockRows
-                     ->  Seq Scan on t3 t1_1_6
-                           Filter: ((a = 4) AND ((a % 2) = 0))
+               ->  Subquery Scan on t1_1_8
+                     Filter: ((t1_1_8.a % 2) = 0)
+                     ->  LockRows
+                           ->  Seq Scan on t3 t1_1_9
+                                 Filter: ((a = 4) AND ((a % 2) = 0))
          ->  Subquery Scan on t1_2_2
                Filter: f_leak(t1_2_2.b)
                ->  Append
@@ -1329,7 +1337,7 @@ AND f_leak(t1_1.b) AND f_leak(t1_2.b) RETURNING *, t1_1, t1_2;
                            Filter: ((a = 4) AND ((a % 2) = 0))
                      ->  Seq Scan on t3 t1_2_11
                            Filter: ((a = 4) AND ((a % 2) = 0))
-(52 rows)
+(58 rows)
 
 UPDATE t1 t1_1 SET b = t1_2.b FROM t1 t1_2
 WHERE t1_1.a = 4 AND t1_2.a = t1_1.a AND t1_2.b = t1_1.b
@@ -1960,8 +1968,6 @@ NOTICE:  f_leak => fgh_updt
 (6 rows)
 
 DELETE FROM x1 WHERE f_leak(b) RETURNING *;
-NOTICE:  f_leak => abc_updt
-NOTICE:  f_leak => efg_updt
 NOTICE:  f_leak => cde_updt
 NOTICE:  f_leak => fgh_updt
 NOTICE:  f_leak => bcd_updt_updt
@@ -1970,15 +1976,13 @@ NOTICE:  f_leak => fgh_updt_updt
 NOTICE:  f_leak => fgh_updt_updt
  a |       b       |         c         
 ---+---------------+-------------------
- 1 | abc_updt      | rls_regress_user1
- 5 | efg_updt      | rls_regress_user1
  3 | cde_updt      | rls_regress_user2
  7 | fgh_updt      | rls_regress_user2
  2 | bcd_updt_updt | rls_regress_user1
  4 | def_updt_updt | rls_regress_user2
  6 | fgh_updt_updt | rls_regress_user1
  8 | fgh_updt_updt | rls_regress_user2
-(8 rows)
+(6 rows)
 
 --
 -- Duplicate Policy Names
-- 
GitLab