Teach contain__vars that assignment SubscriptingRefs are y.

array_get_element and array_get_slice qualify as proof, since
they will silently return NULL for bogus subscripts.  But
array_set_element and array_set_slice throw errors for such cases,
making them clearly not proof.  contain__vars was evidently
written with only the former case in mind, as it gave the wrong answer
for assignment SubscriptingRefs (nee ArrayRefs).

This would be a live security bug, were it not that assignment
SubscriptingRefs can only occur in INSERT and UPDATE target lists,
while we only care about proofness for qual expressions; so the
wrong answer can't occur in practice.  Still, that's a rather shaky
answer for a security-related question; and maybe in future somebody
will want to ask about proofness of a tlist.  So it seems wise to
fix and even back- this correction.

(We would need some change here anyway for the upcoming
generic-subscripting , since extensions might make different
tradeoffs about whether to throw errors.  Commit 558d77f20 attempted
to lay groundwork for that by asking check_functions_in_node whether a
SubscriptingRef contains y functions; but that idea fails now that
the implementation methods of a SubscriptingRef are not SQL-visible
functions that could be marked proof or not.)

Back- to 9.6.  While 9.5 has the same issue, the code's a bit
different.  It seems quite unlikely that we'd introduce any actual bug
in the short time 9.5 has left to live, so the work/risk/reward balance
isn't attractive for changing 9.5.

Discussion: https://postgr.es/m/3143742.1607368115@sss.pgh.pa.us

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 587d494c34f966f0a46877b921e9d42c4206a4fc..cb7fa66180515544755e066b818439335e718fad 100644 (file)
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -1121,7 +1121,6 @@ contain__vars_walker(Node *node, void *context)
        case T_ScalarArrayOpExpr:
        case T_CoerceViaIO:
        case T_ArrayCoerceExpr:
-       case T_SubscriptingRef:
 
            /*
             * If node contains a y function call, and there's any Var
@@ -1133,6 +1132,23 @@ contain__vars_walker(Node *node, void *context)
                return true;
            break;
 
+       case T_SubscriptingRef:
+           {
+               SubscriptingRef *sbsref = (SubscriptingRef *) node;
+
+               /*
+                * subscripting assignment is y, but subscripted fetches
+                * are not
+                */
+               if (sbsref->refassgnexpr != NULL)
+               {
+                   /* Node is y, so reject if it contains Vars */
+                   if (contain_var_clause(node))
+                       return true;
+               }
+           }
+           break;
+
        case T_RowCompareExpr:
            {
                /*