diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
index 973db7435bc..cb5c8fccae9 100644
--- a/doc/src/sgml/rules.sgml
+++ b/doc/src/sgml/rules.sgml
@@ -2136,7 +2136,7 @@ SELECT * FROM phone_number WHERE tricky(person, phone);
When it is necessary for a view to provide row level security, the
<literal>security_barrier</literal> attribute should be applied to
the view. This prevents maliciously-chosen functions and operators from
- being invoked on rows until after the view has done its work. For
+ being passed values from rows until after the view has done its work. For
example, if the view shown above had been created like this, it would
be secure:
<programlisting>
@@ -2157,9 +2157,12 @@ CREATE VIEW phone_number WITH (security_barrier) AS
operators. The query planner can safely allow such functions to be evaluated
at any point in the query execution process, since invoking them on rows
invisible to the user will not leak any information about the unseen rows.
- In contrast, a function that might throw an error depending on the values
- received as arguments (such as one that throws an error in the event of
- overflow or division by zero) are not leak-proof, and could provide
+ Further, functions which do not take arguments or which are not passed any
+ arguments from the security barrier view do not have to be marked as
+ <literal>LEAKPROOF</literal> to be pushed down, as they never receive data
+ from the view. In contrast, a function that might throw an error depending
+ on the values received as arguments (such as one that throws an error in the
+ event of overflow or division by zero) are not leak-proof, and could provide
significant information about the unseen rows if applied before the security
view's row filters.
</para>
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index c4b0c79fb17..9caca94f64b 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1982,7 +1982,9 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query)
* 2. If unsafeVolatile is set, the qual must not contain any volatile
* functions.
*
- * 3. If unsafeLeaky is set, the qual must not contain any leaky functions.
+ * 3. If unsafeLeaky is set, the qual must not contain any leaky functions
+ * that are passed Var nodes, and therefore might reveal values from the
+ * subquery as side effects.
*
* 4. The qual must not refer to the whole-row output of the subquery
* (since there is no easy way to name that within the subquery itself).
@@ -2009,7 +2011,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
/* Refuse leaky quals if told to (point 3) */
if (safetyInfo->unsafeLeaky &&
- contain_leaky_functions(qual))
+ contain_leaked_vars(qual))
return false;
/*
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 84d58ae595e..480114d92b6 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -97,7 +97,7 @@ static bool contain_mutable_functions_walker(Node *node, void *context);
static bool contain_volatile_functions_walker(Node *node, void *context);
static bool contain_volatile_functions_not_nextval_walker(Node *node, void *context);
static bool contain_nonstrict_functions_walker(Node *node, void *context);
-static bool contain_leaky_functions_walker(Node *node, void *context);
+static bool contain_leaked_vars_walker(Node *node, void *context);
static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
@@ -1318,26 +1318,30 @@ contain_nonstrict_functions_walker(Node *node, void *context)
}
/*****************************************************************************
- * Check clauses for non-leakproof functions
+ * Check clauses for Vars passed to non-leakproof functions
*****************************************************************************/
/*
- * contain_leaky_functions
- * Recursively search for leaky functions within a clause.
+ * contain_leaked_vars
+ * Recursively scan a clause to discover whether it contains any Var
+ * nodes (of the current query level) that are passed as arguments to
+ * leaky functions.
*
- * Returns true if any function call with side-effect may be present in the
- * clause. Qualifiers from outside the a security_barrier view should not
- * be pushed down into the view, lest the contents of tuples intended to be
- * filtered out be revealed via side effects.
+ * Returns true if the clause contains any non-leakproof functions that are
+ * passed Var nodes of the current query level, and which might therefore leak
+ * data. Qualifiers from outside a security_barrier view that might leak data
+ * in this way should not be pushed down into the view in case the contents of
+ * tuples intended to be filtered out by the view are revealed by the leaky
+ * functions.
*/
bool
-contain_leaky_functions(Node *clause)
+contain_leaked_vars(Node *clause)
{
- return contain_leaky_functions_walker(clause, NULL);
+ return contain_leaked_vars_walker(clause, NULL);
}
static bool
-contain_leaky_functions_walker(Node *node, void *context)
+contain_leaked_vars_walker(Node *node, void *context)
{
if (node == NULL)
return false;
@@ -1369,7 +1373,8 @@ contain_leaky_functions_walker(Node *node, void *context)
{
FuncExpr *expr = (FuncExpr *) node;
- if (!get_func_leakproof(expr->funcid))
+ if (!get_func_leakproof(expr->funcid) &&
+ contain_var_clause((Node *) expr->args))
return true;
}
break;
@@ -1381,7 +1386,8 @@ contain_leaky_functions_walker(Node *node, void *context)
OpExpr *expr = (OpExpr *) node;
set_opfuncid(expr);
- if (!get_func_leakproof(expr->opfuncid))
+ if (!get_func_leakproof(expr->opfuncid) &&
+ contain_var_clause((Node *) expr->args))
return true;
}
break;
@@ -1391,7 +1397,8 @@ contain_leaky_functions_walker(Node *node, void *context)
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node;
set_sa_opfuncid(expr);
- if (!get_func_leakproof(expr->opfuncid))
+ if (!get_func_leakproof(expr->opfuncid) &&
+ contain_var_clause((Node *) expr->args))
return true;
}
break;
@@ -1401,15 +1408,29 @@ contain_leaky_functions_walker(Node *node, void *context)
CoerceViaIO *expr = (CoerceViaIO *) node;
Oid funcid;
Oid ioparam;
+ bool leakproof;
bool varlena;
+ /*
+ * Data may be leaked if either the input or the output
+ * function is leaky.
+ */
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
- if (!get_func_leakproof(funcid))
- return true;
+ leakproof = get_func_leakproof(funcid);
- getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
- if (!get_func_leakproof(funcid))
+ /*
+ * If the input function is leakproof, then check the output
+ * function.
+ */
+ if (leakproof)
+ {
+ getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
+ leakproof = get_func_leakproof(funcid);
+ }
+
+ if (!leakproof &&
+ contain_var_clause((Node *) expr->arg))
return true;
}
break;
@@ -1419,14 +1440,29 @@ contain_leaky_functions_walker(Node *node, void *context)
ArrayCoerceExpr *expr = (ArrayCoerceExpr *) node;
Oid funcid;
Oid ioparam;
+ bool leakproof;
bool varlena;
+ /*
+ * Data may be leaked if either the input or the output
+ * function is leaky.
+ */
getTypeInputInfo(exprType((Node *) expr->arg),
&funcid, &ioparam);
- if (!get_func_leakproof(funcid))
- return true;
- getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
- if (!get_func_leakproof(funcid))
+ leakproof = get_func_leakproof(funcid);
+
+ /*
+ * If the input function is leakproof, then check the output
+ * function.
+ */
+ if (leakproof)
+ {
+ getTypeOutputInfo(expr->resulttype, &funcid, &varlena);
+ leakproof = get_func_leakproof(funcid);
+ }
+
+ if (!leakproof &&
+ contain_var_clause((Node *) expr->arg))
return true;
}
break;
@@ -1435,12 +1471,22 @@ contain_leaky_functions_walker(Node *node, void *context)
{
RowCompareExpr *rcexpr = (RowCompareExpr *) node;
ListCell *opid;
+ ListCell *larg;
+ ListCell *rarg;
- foreach(opid, rcexpr->opnos)
+ /*
+ * Check the comparison function and arguments passed to it for
+ * each pair of row elements.
+ */
+ forthree(opid, rcexpr->opnos,
+ larg, rcexpr->largs,
+ rarg, rcexpr->rargs)
{
Oid funcid = get_opcode(lfirst_oid(opid));
- if (!get_func_leakproof(funcid))
+ if (!get_func_leakproof(funcid) &&
+ (contain_var_clause((Node *) lfirst(larg)) ||
+ contain_var_clause((Node *) lfirst(rarg))))
return true;
}
}
@@ -1455,7 +1501,7 @@ contain_leaky_functions_walker(Node *node, void *context)
*/
return true;
}
- return expression_tree_walker(node, contain_leaky_functions_walker,
+ return expression_tree_walker(node, contain_leaked_vars_walker,
context);
}
diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h
index 160045cf417..3d04ac2a98d 100644
--- a/src/include/optimizer/clauses.h
+++ b/src/include/optimizer/clauses.h
@@ -63,7 +63,7 @@ extern bool contain_mutable_functions(Node *clause);
extern bool contain_volatile_functions(Node *clause);
extern bool contain_volatile_functions_not_nextval(Node *clause);
extern bool contain_nonstrict_functions(Node *clause);
-extern bool contain_leaky_functions(Node *clause);
+extern bool contain_leaked_vars(Node *clause);
extern Relids find_nonnullable_rels(Node *clause);
extern List *find_nonnullable_vars(Node *clause);
diff --git a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
index 9427a6fae80..3a7a4c329f3 100644
--- a/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
+++ b/src/test/modules/test_rls_hooks/expected/test_rls_hooks.out
@@ -85,13 +85,11 @@ SET ROLE s1;
-- restrictive hook's policy is current_user = superuser
-- combined with AND, results in nothing being allowed
EXPLAIN (costs off) SELECT * FROM rls_test_both;
- QUERY PLAN
--------------------------------------------------------
- Subquery Scan on rls_test_both
- Filter: ("current_user"() = rls_test_both.username)
- -> Seq Scan on rls_test_both rls_test_both_1
- Filter: ("current_user"() = supervisor)
-(4 rows)
+ QUERY PLAN
+-------------------------------------------------------------------------------
+ Seq Scan on rls_test_both
+ Filter: ((supervisor = "current_user"()) AND (username = "current_user"()))
+(2 rows)
SELECT * FROM rls_test_both;
username | supervisor | data
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 1ea65a7d8a1..ad936321749 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -1913,6 +1913,112 @@ EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak(b);
Filter: (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0))
(4 rows)
+--
+-- Qual push-down of leaky functions, when not referring to table
+--
+SELECT * FROM y2 WHERE f_leak('abc');
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+NOTICE: f_leak => abc
+ a | b
+----+----------------------------------
+ 0 | cfcd208495d565ef66e7dff9f98764da
+ 2 | c81e728d9d4c2f636f067f89cc14862c
+ 3 | eccbc87e4b5ce2fe28308fd9f2a7baf3
+ 4 | a87ff679a2f3e71d9181a67b7542122c
+ 6 | 1679091c5a880faf6fb5e6087eb1b2dc
+ 8 | c9f0f895fb98ab9159f51fd0297e236d
+ 9 | 45c48cce2e2d7fbdea1afc51c7c6ad26
+ 10 | d3d9446802a44259755d38e6d163e820
+ 12 | c20ad4d76fe97759aa27a0c99bff6710
+ 14 | aab3238922bcc25a6f606eb525ffdc56
+ 15 | 9bf31c7ff062936a96d3c8bd1f8f2ff3
+ 16 | c74d97b01eae257e44aa9d5bade97baf
+ 18 | 6f4922f45568161a8cdf4ad2299f6d23
+ 20 | 98f13708210194c475687be6106a3b84
+(14 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak('abc');
+ QUERY PLAN
+---------------------------------------------------------------------------------------
+ Seq Scan on y2
+ Filter: (f_leak('abc'::text) AND (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0)))
+(2 rows)
+
+CREATE TABLE test_qual_pushdown (
+ abc text
+);
+INSERT INTO test_qual_pushdown VALUES ('abc'),('def');
+SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc);
+NOTICE: f_leak => abc
+NOTICE: f_leak => def
+ a | b | abc
+---+---+-----
+(0 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc);
+ QUERY PLAN
+-------------------------------------------------------------------------
+ Hash Join
+ Hash Cond: (test_qual_pushdown.abc = y2.b)
+ -> Seq Scan on test_qual_pushdown
+ Filter: f_leak(abc)
+ -> Hash
+ -> Seq Scan on y2
+ Filter: (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0))
+(7 rows)
+
+SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b);
+NOTICE: f_leak => cfcd208495d565ef66e7dff9f98764da
+NOTICE: f_leak => c81e728d9d4c2f636f067f89cc14862c
+NOTICE: f_leak => eccbc87e4b5ce2fe28308fd9f2a7baf3
+NOTICE: f_leak => a87ff679a2f3e71d9181a67b7542122c
+NOTICE: f_leak => 1679091c5a880faf6fb5e6087eb1b2dc
+NOTICE: f_leak => c9f0f895fb98ab9159f51fd0297e236d
+NOTICE: f_leak => 45c48cce2e2d7fbdea1afc51c7c6ad26
+NOTICE: f_leak => d3d9446802a44259755d38e6d163e820
+NOTICE: f_leak => c20ad4d76fe97759aa27a0c99bff6710
+NOTICE: f_leak => aab3238922bcc25a6f606eb525ffdc56
+NOTICE: f_leak => 9bf31c7ff062936a96d3c8bd1f8f2ff3
+NOTICE: f_leak => c74d97b01eae257e44aa9d5bade97baf
+NOTICE: f_leak => 6f4922f45568161a8cdf4ad2299f6d23
+NOTICE: f_leak => 98f13708210194c475687be6106a3b84
+ a | b | abc
+---+---+-----
+(0 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b);
+ QUERY PLAN
+-------------------------------------------------------------------------------
+ Hash Join
+ Hash Cond: (test_qual_pushdown.abc = y2.b)
+ -> Seq Scan on test_qual_pushdown
+ -> Hash
+ -> Subquery Scan on y2
+ Filter: f_leak(y2.b)
+ -> Seq Scan on y2 y2_1
+ Filter: (((a % 4) = 0) OR ((a % 3) = 0) OR ((a % 2) = 0))
+(8 rows)
+
+DROP TABLE test_qual_pushdown;
--
-- Plancache invalidate on user change.
--
diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out
index 82d510de806..7f575266c1b 100644
--- a/src/test/regress/expected/select_views.out
+++ b/src/test/regress/expected/select_views.out
@@ -1348,6 +1348,52 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd);
Filter: (name = ("current_user"())::text)
(4 rows)
+--
+-- scenario: qualifiers can be pushed down if they contain leaky functions,
+-- provided they aren't passed data from inside the view.
+--
+SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+NOTICE: f_leak => passwd
+NOTICE: f_leak => passwd123
+NOTICE: f_leak => passwd
+NOTICE: f_leak => beafsteak
+NOTICE: f_leak => passwd
+NOTICE: f_leak => hamburger
+ cid | name | tel | passwd
+-----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+(1 row)
+
+EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+---------------------------------------------------------------------------------------------
+ Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND f_leak(passwd) AND (name = ("current_user"())::text))
+(2 rows)
+
+SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+NOTICE: f_leak => passwd
+NOTICE: f_leak => passwd123
+NOTICE: f_leak => passwd
+NOTICE: f_leak => passwd
+ cid | name | tel | passwd
+-----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+(1 row)
+
+EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+--------------------------------------------------------------------------------
+ Subquery Scan on v
+ Filter: f_leak(v.passwd)
+ -> Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND (name = ("current_user"())::text))
+(4 rows)
+
--
-- scenario: if a qualifier references only one-side of a particular join-
-- tree, it shall be distributed to the most deep scan plan as
diff --git a/src/test/regress/expected/select_views_1.out b/src/test/regress/expected/select_views_1.out
index ce22bfabeca..5275ef0b2de 100644
--- a/src/test/regress/expected/select_views_1.out
+++ b/src/test/regress/expected/select_views_1.out
@@ -1348,6 +1348,52 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd);
Filter: (name = ("current_user"())::text)
(4 rows)
+--
+-- scenario: qualifiers can be pushed down if they contain leaky functions,
+-- provided they aren't passed data from inside the view.
+--
+SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+NOTICE: f_leak => passwd
+NOTICE: f_leak => passwd123
+NOTICE: f_leak => passwd
+NOTICE: f_leak => beafsteak
+NOTICE: f_leak => passwd
+NOTICE: f_leak => hamburger
+ cid | name | tel | passwd
+-----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+(1 row)
+
+EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+---------------------------------------------------------------------------------------------
+ Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND f_leak(passwd) AND (name = ("current_user"())::text))
+(2 rows)
+
+SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+NOTICE: f_leak => passwd
+NOTICE: f_leak => passwd123
+NOTICE: f_leak => passwd
+NOTICE: f_leak => passwd
+ cid | name | tel | passwd
+-----+---------------+------------------+-----------
+ 101 | regress_alice | +81-12-3456-7890 | passwd123
+(1 row)
+
+EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+ QUERY PLAN
+--------------------------------------------------------------------------------
+ Subquery Scan on v
+ Filter: f_leak(v.passwd)
+ -> Seq Scan on customer
+ Filter: (f_leak('passwd'::text) AND (name = ("current_user"())::text))
+(4 rows)
+
--
-- scenario: if a qualifier references only one-side of a particular join-
-- tree, it shall be distributed to the most deep scan plan as
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index f38b4438fdf..7d12dd00a2f 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -679,6 +679,26 @@ SET SESSION AUTHORIZATION rls_regress_user1;
SELECT * FROM y2 WHERE f_leak(b);
EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak(b);
+--
+-- Qual push-down of leaky functions, when not referring to table
+--
+SELECT * FROM y2 WHERE f_leak('abc');
+EXPLAIN (COSTS OFF) SELECT * FROM y2 WHERE f_leak('abc');
+
+CREATE TABLE test_qual_pushdown (
+ abc text
+);
+
+INSERT INTO test_qual_pushdown VALUES ('abc'),('def');
+
+SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc);
+EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(abc);
+
+SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b);
+EXPLAIN (COSTS OFF) SELECT * FROM y2 JOIN test_qual_pushdown ON (b = abc) WHERE f_leak(b);
+
+DROP TABLE test_qual_pushdown;
+
--
-- Plancache invalidate on user change.
--
diff --git a/src/test/regress/sql/select_views.sql b/src/test/regress/sql/select_views.sql
index da356237eba..3b74ab9d80f 100644
--- a/src/test/regress/sql/select_views.sql
+++ b/src/test/regress/sql/select_views.sql
@@ -95,6 +95,20 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal WHERE f_leak(passwd);
SELECT * FROM my_property_secure WHERE f_leak(passwd);
EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd);
+--
+-- scenario: qualifiers can be pushed down if they contain leaky functions,
+-- provided they aren't passed data from inside the view.
+--
+SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+
+SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure v
+ WHERE f_leak('passwd') AND f_leak(passwd);
+
--
-- scenario: if a qualifier references only one-side of a particular join-
-- tree, it shall be distributed to the most deep scan plan as