database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-03 09:13:20 +03:00
Code Activity

Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.

Browse Source 
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:
Tom Lane
2020-12-08 17:50:54 -05:00
parent a676386b58
commit 62ee703313
1 changed files with 17 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/backend/optimizer/util/clauses.c
View File

@@ -1121,7 +1121,6 @@ contain_leaked_vars_walker(Node *node, void *context)
case T_ScalarArrayOpExpr: case T_ScalarArrayOpExpr:
case T_CoerceViaIO: case T_CoerceViaIO:
case T_ArrayCoerceExpr: case T_ArrayCoerceExpr:
case T_SubscriptingRef:
/* /*
* If node contains a leaky function call, and there's any Var * If node contains a leaky function call, and there's any Var
@@ -1133,6 +1132,23 @@ contain_leaked_vars_walker(Node *node, void *context)
return true; return true;
break; break;
case T_SubscriptingRef:
{
SubscriptingRef *sbsref = (SubscriptingRef *) node;
/*
* subscripting assignment is leaky, but subscripted fetches
* are not
*/
if (sbsref->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:
{ {
/* /*