Merge #147348

147348: sql/rls: prevent leak of hidden rows in RLS due to predicate reordering r=spilchen a=spilchen

RLS policies are applied as filters to scan operations before any user-defined predicates. Previously, the optimizer could reorder these predicates freely, which could result in information leakage: users could infer the existence of hidden rows based on query behavior.

This change wraps the RLS filter in a Barrier operator, which prevents it from being reordered across non-leak-proof expressions. This ensures that evaluation order is preserved and RLS protections remain intact.

 The Barrier is marked as permeable, allowing optgen rules to push the Barrier up the plan tree for expressions that are leakproof. Only optgen rules for the Select operator were added in this change. Subsequent changes will handle joins and projections.

Informs #146952

Epic: CRDB-48807
Release note (bug fix): Fixed a security issue where optimizer predicate reordering could leak information about hidden rows protected by RLS policies.

Co-authored-by: Matt Spilchen <[email protected]>

Author: craig[bot]

pkg/sql/logictest/testdata/logic_test/row_level_security

@@ -5522,3 +5522,116 @@ statement ok
 DROP ROLE can_createdb_global;
 
 subtest end
+
+subtest filter_pushdown_leaks
+
+statement ok
+CREATE ROLE alice;
+
+statement ok
+CREATE TABLE t (x INT PRIMARY KEY, y INT, alice_has_access BOOL);
+
+statement ok
+CREATE INDEX ON t(y);
+
+statement ok
+INSERT INTO t VALUES (1, 10, true), (2, 20, false);
+
+statement ok
+GRANT SELECT, INSERT, UPDATE, DELETE ON t TO alice;
+
+statement ok
+ALTER TABLE t ENABLE ROW LEVEL SECURITY;
+
+statement ok
+CREATE POLICY select_policy_alice
+ON t
+FOR SELECT
+TO alice
+USING (alice_has_access);
+
+statement ok
+SET ROLE alice;
+
+# Attempt various runtime errors to test whether the system leaks the existence
+# of a row with y = 20, which is hidden from the user by the RLS policy.
+
+# Division by zero
+query IIB
+SELECT * FROM t WHERE y = 20 AND y/0 = 0;
+----
+
+# Out of range error
+query IIB
+SELECT * FROM t WHERE y = 20 AND ARRAY[1,2,3][10] = 1;
+----
+
+# Out of range error
+query IIB
+UPDATE t SET y = y WHERE y = 20 AND ARRAY[1,2,3][10] = 1 RETURNING *;
+----
+
+# Custom function that raise an exception.
+statement ok
+CREATE FUNCTION fail() RETURNS INT AS $$ BEGIN RAISE EXCEPTION 'fail'; END; $$ LANGUAGE plpgsql;
+
+query IIB
+DELETE FROM t WHERE y = 20 AND fail() = 1 RETURNING *;
+----
+
+# Custom function that is designated as leakproof
+statement ok
+CREATE FUNCTION divbyzero() RETURNS INT IMMUTABLE LEAKPROOF
+AS $$
+ BEGIN
+   SELECT 20 / 0;
+   RETURN 1;
+ END;
+$$ LANGUAGE plpgsql;
+
+# Show that queries on rows we can see will cause the query to fail
+statement error pq: division by zero
+SELECT * FROM t WHERE y = 10 AND divbyzero() = 1;
+
+# But queries on rows we cannot see won't see any error.
+query IIB
+SELECT * FROM t WHERE y = 20 AND divbyzero() = 1;
+----
+
+# Invalid regular expression
+query IIB
+SELECT * FROM t WHERE y = 20 AND 'a' ~ '(';
+----
+
+# Unicode errors
+query IIB
+UPDATE t SET y = y WHERE y = 20 AND power(1e400, 1) > 0 RETURNING *;
+----
+
+# CTEs
+query IIB
+WITH rows AS (
+  SELECT x FROM t WHERE y = 20 AND y/0 = 0
+)
+DELETE FROM t WHERE x IN (SELECT x FROM rows) RETURNING *;
+----
+
+# INSERT w/ ON CONFLICT. This leaks information because the update attempts
+# to modify a row that is filtered out by RLS, and fails. Postgres behaves the
+# same, so this is left here as an example of that behaviour.
+statement error pq: new row violates row-level security policy for table "t"
+INSERT INTO t VALUES (2, 20, false) ON CONFLICT(x) DO UPDATE SET y = 20;
+
+statement ok
+RESET ROLE;
+
+statement ok
+DROP TABLE t;
+
+statement ok
+DROP FUNCTION fail, divbyzero;
+
+statement ok
+DROP ROLE alice;
+
+subtest end

pkg/sql/opt/exec/explain/testdata/row_level_security

@@ -174,10 +174,13 @@ select t1.c1 from t1, t2 where t1.c1 = t2.c1 and t1.c1 = 1;
 ----
 • cross join
 │
-├── • scan
-│     table: t1@t1_idx
-│     spans: 1+ spans
-│     policies: policy 1, p2, p3, r1, r2
+├── • index join
+│   │ table: t1@t1_pkey
+│   │
+│   └── • scan
+│         table: t1@t1_idx
+│         spans: 1+ spans
+│         policies: policy 1, p2, p3, r1, r2
 │
 └── • filter
     │ filter: c1 = _
@@ -303,6 +306,9 @@ exec-ddl
 CREATE POLICY p_update2 ON writer FOR UPDATE WITH CHECK (value IN ('updater: a', 'updater: b', 'updater: c'))
 ----
 
+# TODO(mgartner): We used to record the locking strength in this plan, but it
+# appears that optimization no longer applies since introducing the barrier for
+# the RLS expression.
 plan
 UPDATE writer SET key = key * 11;
 ----
@@ -317,7 +323,6 @@ UPDATE writer SET key = key * 11;
     └── • scan
           table: writer@writer_pkey
           spans: 1+ spans
-          locking strength: for update
           policies: p_select, p_update1
 
 plan
@@ -353,7 +358,6 @@ UPDATE writer SET value = 'updated' WHERE key between 1 and 100;
     └── • scan
           table: writer@writer_pkey
           spans: 1+ spans
-          locking strength: for update
           policies: p_select, p_update1
 
 plan
@@ -370,7 +374,6 @@ UPDATE writer SET value = value WHERE key = 5;
     └── • scan
           table: writer@writer_pkey
           spans: 1+ spans
-          locking strength: for update
           policies: p_select, p_update1
 
 # Should not apply select policy for the new row because columns weren't
@@ -389,7 +392,6 @@ UPDATE writer SET key = 10 WHERE true;
     └── • scan
           table: writer@writer_pkey
           spans: 1+ spans
-          locking strength: for update
           policies: p_select, p_update1
 
 
@@ -434,11 +436,14 @@ CREATE POLICY p_delete ON writer FOR DELETE USING (key = 1);
 plan
 DELETE FROM writer WHERE key = 1;
 ----
-• delete range
-  from: writer
-  auto commit
-  spans: 1+ spans
-  policies: p_select, p_delete
+• delete
+│ from: writer
+│ auto commit
+│
+└── • scan
+      table: writer@writer_pkey
+      spans: 1+ spans
+      policies: p_select, p_delete
 
 plan
 DELETE FROM writer WHERE value = 'some-val' or value = 'other-val';

pkg/sql/opt/memo/testdata/logprops/tail-calls

@@ -444,14 +444,13 @@ values
                                     │              └── const: '00000'
                                     └── project
                                          ├── barrier
-                                         │    └── barrier
-                                         │         └── values
-                                         │              └── tuple
-                                         │                   └── udf: nested
-                                         │                        └── body
-                                         │                             └── values
-                                         │                                  └── tuple
-                                         │                                       └── const: 1
+                                         │    └── values
+                                         │         └── tuple
+                                         │              └── udf: nested
+                                         │                   └── body
+                                         │                        └── values
+                                         │                             └── tuple
+                                         │                                  └── const: 1
                                          └── projections
                                               └── udf: _stmt_raise_3
                                                    ├── tail-call

pkg/sql/opt/norm/BUILD.bazel

@@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 go_library(
     name = "norm",
     srcs = [
+        "barrier_funcs.go",
         "bool_funcs.go",
         "comp_funcs.go",
         "decorrelate_funcs.go",

pkg/sql/opt/norm/barrier_funcs.go

@@ -0,0 +1,16 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package norm
+
+import "github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
+
+// BarriersEqual returns true if the two BarrierPrivate structs are the same.
+func (c *CustomFuncs) BarriersEqual(left, right *memo.BarrierPrivate) bool {
+	if left == nil || right == nil {
+		return left == right // true if both nil, false otherwise
+	}
+	return *left == *right
+}

pkg/sql/opt/norm/general_funcs.go

@@ -1560,3 +1560,34 @@ func (c *CustomFuncs) DuplicateJoinPrivate(jp *memo.JoinPrivate) *memo.JoinPriva
 		SkipReorderJoins: jp.SkipReorderJoins,
 	}
 }
+
+// IsBarrierLeakproofPermeable returns true if the barrier allows for leakproof
+// filters to permeate through it.
+func (c *CustomFuncs) IsBarrierLeakproofPermeable(priv *memo.BarrierPrivate) bool {
+	return priv.LeakproofPermeable
+}
+
+// SplitLeakproofFilters separates a list of filters into two groups: those that
+// are leakproof and those that are not. Leakproof filters are expressions that
+// do not reveal information about underlying data through their evaluation
+// behavior.
+//
+// This function is typically used to determine which filters can be safely
+// reordered or pushed past a Barrier marked as LeakproofPermeable. It returns
+// the leakproof filters, the remaining filters, and a boolean indicating
+// whether any leakproof filters were found.
+func (c *CustomFuncs) SplitLeakproofFilters(
+	filters memo.FiltersExpr,
+) (leakproofFilters, remainingFilters memo.FiltersExpr, hasLeakproofFilters bool) {
+	for i := range filters {
+		if filters[i].ScalarProps().VolatilitySet.IsLeakproof() {
+			leakproofFilters = append(leakproofFilters, filters[i])
+		} else {
+			remainingFilters = append(remainingFilters, filters[i])
+		}
+	}
+	if len(leakproofFilters) > 0 {
+		return leakproofFilters, remainingFilters, true
+	}
+	return nil, filters, false
+}

pkg/sql/opt/norm/rules/barrier.opt

@@ -0,0 +1,16 @@
+# =============================================================================
+# barrier.opt contains normalization rules for Barrier operators.
+# =============================================================================
+
+# EliminateRedundantBarrier removes a Barrier operator when it wraps another
+# identical Barrier. This deduplication avoids unnecessary nesting of
+# equivalent Barriers. The rule applies only when both Barrier operators have
+# the same configuration.
+[EliminateRedundantBarrier, Normalize]
+(Barrier
+    (Barrier $input:* $innerBarrierPrivate:*)
+    $outerBarrierPrivate:* &
+        (BarriersEqual $innerBarrierPrivate $outerBarrierPrivate)
+)
+=>
+(Barrier $input $outerBarrierPrivate)

pkg/sql/opt/norm/rules/select.opt

@@ -448,3 +448,32 @@ $input
     )
     (RemoveFiltersItem $filter $item)
 )
+
+# PushLeakproofFiltersIntoPermeableBarrier splits filter expressions based on
+# leakproofness and pushes only the leakproof filters beneath a permeable
+# Barrier. The remaining filters stay above the Barrier.
+#
+# This allows safe reordering of leakproof expressions while preserving the
+# Barrier to block unsafe transformations involving non-leakproof filters.
+# The Barrier must be marked as LeakproofPermeable to allow this behavior.
+[PushLeakproofFiltersIntoPermeableBarrier, Normalize]
+(Select
+    (Barrier
+        $input:*
+        $private:* & (IsBarrierLeakproofPermeable $private)
+    )
+    $filters:* &
+        (Let
+            (
+                $leakproofFilters
+                $remainingFilters
+                $ok
+            ):(SplitLeakproofFilters $filters)
+            $ok
+        )
+)
+=>
+(Select
+    (Barrier (Select $input $leakproofFilters) $private)
+    $remainingFilters
+)