Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 17c77c8

Browse filesBrowse files
tglsfdctglsfdc
committed
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 558d77f 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
1 parent 2bf5d1a commit 17c77c8
Copy full SHA for 17c77c8

File tree

Expand file treeCollapse file tree

1 file changed

+17
-1
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+17
-1
lines changed

‎src/backend/optimizer/util/clauses.c

Copy file name to clipboardExpand all lines: src/backend/optimizer/util/clauses.c
+17-1Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1505,7 +1505,6 @@ contain_leaked_vars_walker(Node *node, void *context)
15051505
case T_Var:
15061506
case T_Const:
15071507
case T_Param:
1508-
case T_ArrayRef:
15091508
case T_ArrayExpr:
15101509
case T_FieldSelect:
15111510
case T_FieldStore:
@@ -1544,6 +1543,23 @@ contain_leaked_vars_walker(Node *node, void *context)
15441543
return true;
15451544
break;
15461545

1546+
case T_ArrayRef:
1547+
{
1548+
ArrayRef *aref = (ArrayRef *) node;
1549+
1550+
/*
1551+
* array assignment is leaky, but subscripted fetches
1552+
* are not
1553+
*/
1554+
if (aref->refassgnexpr != NULL)
1555+
{
1556+
/* Node is leaky, so reject if it contains Vars */
1557+
if (contain_var_clause(node))
1558+
return true;
1559+
}
1560+
}
1561+
break;
1562+
15471563
case T_RowCompareExpr:
15481564
{
15491565
/*

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.