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 62ee703

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 a676386 commit 62ee703
Copy full SHA for 62ee703

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
@@ -1121,7 +1121,6 @@ contain_leaked_vars_walker(Node *node, void *context)
11211121
case T_ScalarArrayOpExpr:
11221122
case T_CoerceViaIO:
11231123
case T_ArrayCoerceExpr:
1124-
case T_SubscriptingRef:
11251124

11261125
/*
11271126
* If node contains a leaky function call, and there's any Var
@@ -1133,6 +1132,23 @@ contain_leaked_vars_walker(Node *node, void *context)
11331132
return true;
11341133
break;
11351134

1135+
case T_SubscriptingRef:
1136+
{
1137+
SubscriptingRef *sbsref = (SubscriptingRef *) node;
1138+
1139+
/*
1140+
* subscripting assignment is leaky, but subscripted fetches
1141+
* are not
1142+
*/
1143+
if (sbsref->refassgnexpr != NULL)
1144+
{
1145+
/* Node is leaky, so reject if it contains Vars */
1146+
if (contain_var_clause(node))
1147+
return true;
1148+
}
1149+
}
1150+
break;
1151+
11361152
case T_RowCompareExpr:
11371153
{
11381154
/*

0 commit comments

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