Skip to content

Commit 61e35dd

Browse files
hvitvedhvitved
committed
Data flow: Call-sensitive resolution of lambda/block calls
1 parent 77146e4 commit 61e35dd

6 files changed

Lines changed: 136 additions & 6 deletions

File tree

‎ql/src/codeql_ruby/controlflow/CfgNodes.qll‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ module ExprNodes {
249249
class CallCfgNode extends ExprCfgNode {
250250
override CallExprChildMapping e;
251251

252-
final override Call getExpr() { result = ExprCfgNode.super.getExpr() }
252+
override Call getExpr() { result = super.getExpr() }
253253

254254
/** Gets the `n`th argument of this call. */
255255
final ExprCfgNode getArgument(int n) { e.hasCfgChild(e.getArgument(n), this, result) }
@@ -270,7 +270,9 @@ module ExprNodes {
270270

271271
/** A control-flow node that wraps a `MethodCall` AST expression. */
272272
class MethodCallCfgNode extends CallCfgNode {
273-
MethodCallCfgNode() { this.getExpr() instanceof MethodCall }
273+
MethodCallCfgNode() { super.getExpr() instanceof MethodCall }
274+
275+
final override MethodCall getExpr() { result = super.getExpr() }
274276
}
275277

276278
/** A control-flow node that wraps a `CaseExpr` AST expression. */

‎ql/src/codeql_ruby/dataflow/internal/DataFlowDispatch.qll‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,10 @@ private DataFlow::LocalSourceNode trackModule(Module tp) {
282282
}
283283

284284
/** Gets a viable run-time target for the call `call`. */
285-
DataFlowCallable viableCallable(DataFlowCall call) { result = call.getTarget() }
285+
DataFlowCallable viableCallable(DataFlowCall call) {
286+
result = call.getTarget() and
287+
not call.getExpr() instanceof YieldCall // handled by `lambdaCreation`/`lambdaCall`
288+
}
286289

287290
/**
288291
* Holds if the set of viable implementations that can be called by `call`

‎ql/src/codeql_ruby/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,13 +553,39 @@ class BarrierGuard extends Expr {
553553
Node getAGuardedNode() { none() }
554554
}
555555

556-
class LambdaCallKind = Unit;
556+
newtype LambdaCallKind =
557+
TYieldCallKind() or
558+
TLambdaCallKind()
557559

558560
/** Holds if `creation` is an expression that creates a lambda of kind `kind` for `c`. */
559-
predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) { none() }
561+
predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) {
562+
kind = TYieldCallKind() and
563+
creation.asExpr().getExpr() = c.(Block)
564+
or
565+
kind = TLambdaCallKind() and
566+
(
567+
creation.asExpr().getExpr() = c.(Lambda)
568+
or
569+
creation.asExpr() =
570+
any(CfgNodes::ExprNodes::MethodCallCfgNode mc |
571+
c = mc.getBlock().getExpr() and
572+
mc.getExpr().getMethodName() = "lambda"
573+
)
574+
)
575+
}
560576

561577
/** Holds if `call` is a lambda call of kind `kind` where `receiver` is the lambda expression. */
562-
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { none() }
578+
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) {
579+
kind = TYieldCallKind() and
580+
receiver.(BlockParameterNode).getMethod() = call.getExpr().(YieldCall).getEnclosingMethod()
581+
or
582+
kind = TLambdaCallKind() and
583+
call =
584+
any(CfgNodes::ExprNodes::MethodCallCfgNode mc |
585+
receiver.asExpr() = mc.getReceiver() and
586+
mc.getExpr().getMethodName() = "call"
587+
)
588+
}
563589

564590
/** Extra data-flow steps needed for lambda flow analysis. */
565591
predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() }

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
edges
2+
| call_sensitivity.rb:7:13:7:13 | x : | call_sensitivity.rb:8:11:8:11 | x : |
3+
| call_sensitivity.rb:8:11:8:11 | x : | call_sensitivity.rb:15:20:15:20 | x : |
4+
| call_sensitivity.rb:15:9:15:15 | "taint" : | call_sensitivity.rb:7:13:7:13 | x : |
5+
| call_sensitivity.rb:15:20:15:20 | x : | call_sensitivity.rb:15:28:15:28 | x |
6+
| call_sensitivity.rb:17:27:17:27 | x : | call_sensitivity.rb:18:17:18:17 | x : |
7+
| call_sensitivity.rb:17:27:17:27 | x : | call_sensitivity.rb:18:17:18:17 | x : |
8+
| call_sensitivity.rb:18:17:18:17 | x : | call_sensitivity.rb:27:17:27:17 | x : |
9+
| call_sensitivity.rb:18:17:18:17 | x : | call_sensitivity.rb:36:23:36:23 | x : |
10+
| call_sensitivity.rb:27:17:27:17 | x : | call_sensitivity.rb:27:27:27:27 | x |
11+
| call_sensitivity.rb:28:25:28:31 | "taint" : | call_sensitivity.rb:17:27:17:27 | x : |
12+
| call_sensitivity.rb:36:23:36:23 | x : | call_sensitivity.rb:36:31:36:31 | x |
13+
| call_sensitivity.rb:37:25:37:31 | "taint" : | call_sensitivity.rb:17:27:17:27 | x : |
14+
nodes
15+
| call_sensitivity.rb:5:6:5:12 | "taint" | semmle.label | "taint" |
16+
| call_sensitivity.rb:7:13:7:13 | x : | semmle.label | x : |
17+
| call_sensitivity.rb:8:11:8:11 | x : | semmle.label | x : |
18+
| call_sensitivity.rb:15:9:15:15 | "taint" : | semmle.label | "taint" : |
19+
| call_sensitivity.rb:15:20:15:20 | x : | semmle.label | x : |
20+
| call_sensitivity.rb:15:28:15:28 | x | semmle.label | x |
21+
| call_sensitivity.rb:17:27:17:27 | x : | semmle.label | x : |
22+
| call_sensitivity.rb:17:27:17:27 | x : | semmle.label | x : |
23+
| call_sensitivity.rb:18:17:18:17 | x : | semmle.label | x : |
24+
| call_sensitivity.rb:18:17:18:17 | x : | semmle.label | x : |
25+
| call_sensitivity.rb:27:17:27:17 | x : | semmle.label | x : |
26+
| call_sensitivity.rb:27:27:27:27 | x | semmle.label | x |
27+
| call_sensitivity.rb:28:25:28:31 | "taint" : | semmle.label | "taint" : |
28+
| call_sensitivity.rb:36:23:36:23 | x : | semmle.label | x : |
29+
| call_sensitivity.rb:36:31:36:31 | x | semmle.label | x |
30+
| call_sensitivity.rb:37:25:37:31 | "taint" : | semmle.label | "taint" : |
31+
#select
32+
| call_sensitivity.rb:5:6:5:12 | "taint" | call_sensitivity.rb:5:6:5:12 | "taint" | call_sensitivity.rb:5:6:5:12 | "taint" | $@ | call_sensitivity.rb:5:6:5:12 | "taint" | "taint" |
33+
| call_sensitivity.rb:15:28:15:28 | x | call_sensitivity.rb:15:9:15:15 | "taint" : | call_sensitivity.rb:15:28:15:28 | x | $@ | call_sensitivity.rb:15:9:15:15 | "taint" : | "taint" : |
34+
| call_sensitivity.rb:27:27:27:27 | x | call_sensitivity.rb:28:25:28:31 | "taint" : | call_sensitivity.rb:27:27:27:27 | x | $@ | call_sensitivity.rb:28:25:28:31 | "taint" : | "taint" : |
35+
| call_sensitivity.rb:36:31:36:31 | x | call_sensitivity.rb:37:25:37:31 | "taint" : | call_sensitivity.rb:36:31:36:31 | x | $@ | call_sensitivity.rb:37:25:37:31 | "taint" : | "taint" : |

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import ruby
6+
import codeql_ruby.DataFlow
7+
import DataFlow::PathGraph
8+
9+
class Conf extends DataFlow::Configuration {
10+
Conf() { this = "Conf" }
11+
12+
override predicate isSource(DataFlow::Node src) {
13+
src.asExpr().getExpr().(StringLiteral).getValueText() = "taint"
14+
}
15+
16+
override predicate isSink(DataFlow::Node sink) {
17+
exists(MethodCall mc |
18+
mc.getMethodName() = "sink" and
19+
mc.getAnArgument() = sink.asExpr().getExpr()
20+
)
21+
}
22+
}
23+
24+
from DataFlow::PathNode source, DataFlow::PathNode sink, Conf conf
25+
where conf.hasFlowPath(source, sink)
26+
select sink, source, sink, "$@", source, source.toString()

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
def sink s
2+
puts s
3+
end
4+
5+
sink "taint"
6+
7+
def yielder x
8+
yield x
9+
end
10+
11+
yielder "no taint" { |x| sink x } # no flow
12+
13+
yielder "taint" { |x| puts x } # no flow
14+
15+
yielder "taint" { |x| sink x } # flow
16+
17+
def apply_lambda (lambda, x)
18+
lambda.call(x)
19+
end
20+
21+
my_lambda = -> (x) { sink x }
22+
apply_lambda(my_lambda, "no taint") # no flow
23+
24+
my_lambda = -> (x) { puts x }
25+
apply_lambda(my_lambda, "taint") # no flow
26+
27+
my_lambda = -> (x) { sink x }
28+
apply_lambda(my_lambda, "taint") # flow
29+
30+
my_lambda = lambda { |x| sink x }
31+
apply_lambda(my_lambda, "no taint") # no flow
32+
33+
my_lambda = lambda { |x| puts x }
34+
apply_lambda(my_lambda, "taint") # no flow
35+
36+
my_lambda = lambda { |x| sink x }
37+
apply_lambda(my_lambda, "taint") # flow
38+

0 commit comments

Comments
 (0)