mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-03 08:27:19 +02:00
Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.
array_get_element and array_get_slice qualify as leakproof, 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 leakproof. contain_leaked_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 leakproofness 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 leakproofness of a tlist. So it seems wise to
fix and even back-patch this correction.
(We would need some change here anyway for the upcoming
generic-subscripting patch, 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 leaky functions; but that idea fails now that
the implementation methods of a SubscriptingRef are not SQL-visible
functions that could be marked leakproof or not.)
Back-patch 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
This commit is contained in:
parent
5303706b32
commit
1f229f4fdc
@ -1602,7 +1602,6 @@ contain_leaked_vars_walker(Node *node, void *context)
|
|||||||
case T_Var:
|
case T_Var:
|
||||||
case T_Const:
|
case T_Const:
|
||||||
case T_Param:
|
case T_Param:
|
||||||
case T_ArrayRef:
|
|
||||||
case T_ArrayExpr:
|
case T_ArrayExpr:
|
||||||
case T_FieldSelect:
|
case T_FieldSelect:
|
||||||
case T_FieldStore:
|
case T_FieldStore:
|
||||||
@ -1643,6 +1642,23 @@ contain_leaked_vars_walker(Node *node, void *context)
|
|||||||
return true;
|
return true;
|
||||||
break;
|
break;
|
||||||
|
|
||||||
|
case T_ArrayRef:
|
||||||
|
{
|
||||||
|
ArrayRef *aref = (ArrayRef *) node;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* array assignment is leaky, but subscripted fetches
|
||||||
|
* are not
|
||||||
|
*/
|
||||||
|
if (aref->refassgnexpr != NULL)
|
||||||
|
{
|
||||||
|
/* Node is leaky, so reject if it contains Vars */
|
||||||
|
if (contain_var_clause(node))
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
|
||||||
case T_RowCompareExpr:
|
case T_RowCompareExpr:
|
||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
|
Loading…
Reference in New Issue
Block a user