Skip to content

Commit 2951987

Browse files
hvitvedhvitved
committed
Ruby: Handle captured yield calls
1 parent 55be4c3 commit 2951987

File tree

8 files changed

+70
-13
lines changed

8 files changed

+70
-13
lines changed

‎ruby/ql/consistency-queries/DataFlowConsistency.ql

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ private module Input implements InputSig<RubyDataFlow> {
4343
arg.asExpr().getASuccessor(any(SuccessorTypes::ConditionalSuccessor c)).getASuccessor*() = n and
4444
n.getASplit() instanceof Split::ConditionalCompletionSplit
4545
)
46-
or
47-
// Synthetic block parameter nodes are passed directly as lambda-self reference
48-
// arguments to all `yield` calls
49-
arg instanceof ArgumentNodes::BlockParameterArgumentNode
5046
}
5147
}
5248

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,9 @@ private predicate moduleFlowsToMethodCallReceiver(RelevantCall call, Module m, s
208208
flowsToMethodCallReceiver(call, trackModuleAccess(m), method)
209209
}
210210

211-
private Block blockCall(RelevantCall call) { lambdaSourceCall(call, _, trackBlock(result)) }
211+
private Block blockCall(RelevantCall call) {
212+
lambdaSourceCall(call, _, trackBlock(result).(DataFlow::LocalSourceNode).getALocalUse())
213+
}
212214

213215
pragma[nomagic]
214216
private predicate superCall(RelevantCall call, Module cls, string method) {

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ module LocalFlow {
230230
or
231231
p.(KeywordParameter).getDefaultValue() = nodeFrom.asExpr().getExpr()
232232
)
233+
or
234+
nodeTo.(BlockArgumentNode).getParameterNode(true) = nodeFrom
233235
}
234236
}
235237

@@ -497,6 +499,9 @@ private module Cached {
497499
TSelfParameterNode(MethodBase m) or
498500
TLambdaSelfReferenceNode(Callable c) { lambdaCreationExpr(_, _, c) } or
499501
TBlockParameterNode(MethodBase m) or
502+
TBlockArgumentNode(CfgNodes::ExprNodes::CallCfgNode yield) {
503+
yield = any(BlockParameterNode b).getAYieldCall()
504+
} or
500505
TSynthHashSplatParameterNode(DataFlowCallable c) {
501506
isParameterNode(_, c, any(ParameterPosition p | p.isKeyword(_)))
502507
} or
@@ -645,6 +650,8 @@ private module Cached {
645650
isStoreTargetNode(n)
646651
or
647652
TypeTrackingInput::loadStep(_, n, _)
653+
or
654+
n instanceof BlockArgumentNode
648655
}
649656

650657
cached
@@ -770,6 +777,8 @@ predicate nodeIsHidden(Node n) {
770777
n instanceof LambdaSelfReferenceNode
771778
or
772779
n instanceof CaptureNode
780+
or
781+
n instanceof BlockArgumentNode
773782
}
774783

775784
/** An SSA definition, viewed as a node in a data flow graph. */
@@ -1277,18 +1286,36 @@ module ArgumentNodes {
12771286
}
12781287
}
12791288

1280-
class BlockParameterArgumentNode extends BlockParameterNode, ArgumentNode {
1281-
BlockParameterArgumentNode() { exists(this.getAYieldCall()) }
1289+
class BlockArgumentNode extends NodeImpl, ArgumentNode, TBlockArgumentNode {
1290+
CfgNodes::ExprNodes::CallCfgNode yield;
1291+
1292+
BlockArgumentNode() { this = TBlockArgumentNode(yield) }
1293+
1294+
CfgNodes::ExprNodes::CallCfgNode getYieldCall() { result = yield }
1295+
1296+
pragma[nomagic]
1297+
BlockParameterNode getParameterNode(boolean inSameScope) {
1298+
result.getAYieldCall() = yield and
1299+
if nodeGetEnclosingCallable(this) = nodeGetEnclosingCallable(result)
1300+
then inSameScope = true
1301+
else inSameScope = false
1302+
}
12821303

12831304
// needed for variable capture flow
12841305
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
1285-
call = this.getAYieldCall() and
1306+
call = yield and
12861307
pos.isLambdaSelf()
12871308
}
12881309

12891310
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
12901311
this.sourceArgumentOf(call.asCall(), pos)
12911312
}
1313+
1314+
override CfgScope getCfgScope() { result = yield.getScope() }
1315+
1316+
override Location getLocationImpl() { result = yield.getLocation() }
1317+
1318+
override string toStringImpl() { result = "yield block argument" }
12921319
}
12931320

12941321
private class SummaryArgumentNode extends FlowSummaryNode, ArgumentNode {
@@ -1699,6 +1726,8 @@ predicate jumpStep(Node pred, Node succ) {
16991726
succ.(FlowSummaryNode).getSummaryNode())
17001727
or
17011728
any(AdditionalJumpStep s).step(pred, succ)
1729+
or
1730+
succ.(BlockArgumentNode).getParameterNode(false) = pred
17021731
}
17031732

17041733
private ContentSet getArrayContent(int n) {
@@ -2037,7 +2066,7 @@ private predicate lambdaCallExpr(
20372066
*/
20382067
predicate lambdaSourceCall(CfgNodes::ExprNodes::CallCfgNode call, LambdaCallKind kind, Node receiver) {
20392068
kind = TYieldCallKind() and
2040-
call = receiver.(BlockParameterNode).getAYieldCall()
2069+
call = receiver.(BlockArgumentNode).getYieldCall()
20412070
or
20422071
kind = TLambdaCallKind() and
20432072
lambdaCallExpr(call, receiver.asExpr())

‎ruby/ql/test/library-tests/dataflow/call-sensitivity/call-sensitivity.expected

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
testFailures
2-
| call_sensitivity.rb:200:10:200:28 | # $ hasValueFlow=37 | Missing result:hasValueFlow=37 |
32
edges
43
| call_sensitivity.rb:9:7:9:13 | call to taint | call_sensitivity.rb:9:6:9:14 | ( ... ) |
54
| call_sensitivity.rb:11:13:11:13 | x | call_sensitivity.rb:12:11:12:11 | x |
@@ -82,6 +81,17 @@ edges
8281
| call_sensitivity.rb:178:11:178:19 | call to taint | call_sensitivity.rb:174:19:174:19 | x |
8382
| call_sensitivity.rb:187:11:187:20 | ( ... ) | call_sensitivity.rb:104:18:104:18 | x |
8483
| call_sensitivity.rb:187:12:187:19 | call to taint | call_sensitivity.rb:187:11:187:20 | ( ... ) |
84+
| call_sensitivity.rb:189:19:189:19 | x | call_sensitivity.rb:190:9:190:9 | x |
85+
| call_sensitivity.rb:190:9:190:9 | x | call_sensitivity.rb:194:23:194:23 | x |
86+
| call_sensitivity.rb:193:19:193:19 | x | call_sensitivity.rb:194:17:194:17 | x |
87+
| call_sensitivity.rb:194:17:194:17 | x | call_sensitivity.rb:189:19:189:19 | x |
88+
| call_sensitivity.rb:194:23:194:23 | x | call_sensitivity.rb:195:11:195:11 | x |
89+
| call_sensitivity.rb:195:11:195:11 | x | call_sensitivity.rb:199:30:199:30 | x |
90+
| call_sensitivity.rb:195:11:195:11 | x | call_sensitivity.rb:203:26:203:26 | x |
91+
| call_sensitivity.rb:199:15:199:24 | ( ... ) | call_sensitivity.rb:193:19:193:19 | x |
92+
| call_sensitivity.rb:199:16:199:23 | call to taint | call_sensitivity.rb:199:15:199:24 | ( ... ) |
93+
| call_sensitivity.rb:199:30:199:30 | x | call_sensitivity.rb:200:8:200:8 | x |
94+
| call_sensitivity.rb:203:26:203:26 | x | call_sensitivity.rb:204:8:204:8 | x |
8595
nodes
8696
| call_sensitivity.rb:9:6:9:14 | ( ... ) | semmle.label | ( ... ) |
8797
| call_sensitivity.rb:9:7:9:13 | call to taint | semmle.label | call to taint |
@@ -169,6 +179,18 @@ nodes
169179
| call_sensitivity.rb:178:11:178:19 | call to taint | semmle.label | call to taint |
170180
| call_sensitivity.rb:187:11:187:20 | ( ... ) | semmle.label | ( ... ) |
171181
| call_sensitivity.rb:187:12:187:19 | call to taint | semmle.label | call to taint |
182+
| call_sensitivity.rb:189:19:189:19 | x | semmle.label | x |
183+
| call_sensitivity.rb:190:9:190:9 | x | semmle.label | x |
184+
| call_sensitivity.rb:193:19:193:19 | x | semmle.label | x |
185+
| call_sensitivity.rb:194:17:194:17 | x | semmle.label | x |
186+
| call_sensitivity.rb:194:23:194:23 | x | semmle.label | x |
187+
| call_sensitivity.rb:195:11:195:11 | x | semmle.label | x |
188+
| call_sensitivity.rb:199:15:199:24 | ( ... ) | semmle.label | ( ... ) |
189+
| call_sensitivity.rb:199:16:199:23 | call to taint | semmle.label | call to taint |
190+
| call_sensitivity.rb:199:30:199:30 | x | semmle.label | x |
191+
| call_sensitivity.rb:200:8:200:8 | x | semmle.label | x |
192+
| call_sensitivity.rb:203:26:203:26 | x | semmle.label | x |
193+
| call_sensitivity.rb:204:8:204:8 | x | semmle.label | x |
172194
subpaths
173195
#select
174196
| call_sensitivity.rb:9:6:9:14 | ( ... ) | call_sensitivity.rb:9:7:9:13 | call to taint | call_sensitivity.rb:9:6:9:14 | ( ... ) | $@ | call_sensitivity.rb:9:7:9:13 | call to taint | call to taint |
@@ -194,6 +216,8 @@ subpaths
194216
| call_sensitivity.rb:105:10:105:10 | x | call_sensitivity.rb:125:12:125:19 | call to taint | call_sensitivity.rb:105:10:105:10 | x | $@ | call_sensitivity.rb:125:12:125:19 | call to taint | call to taint |
195217
| call_sensitivity.rb:105:10:105:10 | x | call_sensitivity.rb:178:11:178:19 | call to taint | call_sensitivity.rb:105:10:105:10 | x | $@ | call_sensitivity.rb:178:11:178:19 | call to taint | call to taint |
196218
| call_sensitivity.rb:105:10:105:10 | x | call_sensitivity.rb:187:12:187:19 | call to taint | call_sensitivity.rb:105:10:105:10 | x | $@ | call_sensitivity.rb:187:12:187:19 | call to taint | call to taint |
219+
| call_sensitivity.rb:200:8:200:8 | x | call_sensitivity.rb:199:16:199:23 | call to taint | call_sensitivity.rb:200:8:200:8 | x | $@ | call_sensitivity.rb:199:16:199:23 | call to taint | call to taint |
220+
| call_sensitivity.rb:204:8:204:8 | x | call_sensitivity.rb:199:16:199:23 | call to taint | call_sensitivity.rb:204:8:204:8 | x | $@ | call_sensitivity.rb:199:16:199:23 | call to taint | call to taint |
197221
mayBenefitFromCallContext
198222
| call_sensitivity.rb:51:5:51:10 | call to sink |
199223
| call_sensitivity.rb:55:5:55:13 | call to method1 |

‎ruby/ql/test/library-tests/dataflow/call-sensitivity/call_sensitivity.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,5 +201,5 @@ def invoke_block2 x
201201
end
202202

203203
invoke_block2 "safe" do |x|
204-
sink x
204+
sink x # $ SPURIOUS hasValueFlow=37
205205
end

‎ruby/ql/test/library-tests/dataflow/global/TypeTrackingInlineTest.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
testFailures
2+
| blocks.rb:4:10:4:10 | r | Fixed missing result:hasValueFlow=1 |
23
| captured_variables.rb:50:10:50:10 | x | Fixed missing result:hasValueFlow=2 |
34
| captured_variables.rb:68:25:68:68 | # $ hasValueFlow=3 $ MISSING: hasValueFlow=4 | Missing result:hasValueFlow=3 |
45
| captured_variables.rb:72:21:72:66 | # $ hasValueFlow=4 $ SPURIOUS: hasValueFlow=3 | Fixed spurious result:hasValueFlow=3 |

‎ruby/ql/test/library-tests/dataflow/global/blocks.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
class A
22
def m1(&block)
3-
r = block.call() # $ MISSING: hasValueFlow=1
4-
sink r
3+
r = block.call()
4+
sink r # $ MISSING: hasValueFlow=1
55
end
66

77
def m2

‎shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,4 +319,9 @@ module MakeConsistency<
319319
strictcount(DataFlowCall call0 | multipleArgumentCallInclude(arg, call0)) > 1 and
320320
msg = "Multiple calls for argument node."
321321
}
322+
323+
query predicate lambdaCallEnclosingCallableMismatch(DataFlowCall call, Node receiver) {
324+
lambdaCall(call, _, receiver) and
325+
not nodeGetEnclosingCallable(receiver) = call.getEnclosingCallable()
326+
}
322327
}

0 commit comments

Comments
 (0)