Skip to content

Commit 4607adc

Browse files
hvitvedhvitved
committed
Ruby: Call sensitive instance method resolution
1 parent d177ba3 commit 4607adc

File tree

4 files changed

+173
-100
lines changed

4 files changed

+173
-100
lines changed

‎ruby/ql/lib/codeql/ruby/ast/Module.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ class Module extends TModule {
1313
/** Gets the super class of this module, if any. */
1414
Module getSuperClass() { result = getSuperClass(this) }
1515

16+
/** Gets a sub class of this module, if any. */
17+
Module getASubClass() { this = getSuperClass(result) }
18+
1619
/** Gets a `prepend`ed module. */
1720
Module getAPrependedModule() { result = getAPrependedModule(this) }
1821

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

Lines changed: 151 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -453,87 +453,100 @@ private DataFlow::LocalSourceNode trackModuleAccess(Module m) {
453453
result = trackModuleAccess(m, TypeTracker::end())
454454
}
455455

456-
pragma[nomagic]
457-
private DataFlow::LocalSourceNode trackInstance(Module tp, boolean exact, TypeTracker t) {
458-
t.start() and
459-
(
460-
result.asExpr().getExpr() instanceof NilLiteral and
461-
tp = TResolved("NilClass") and
462-
exact = true
463-
or
464-
result.asExpr().getExpr().(BooleanLiteral).isFalse() and
465-
tp = TResolved("FalseClass") and
466-
exact = true
467-
or
468-
result.asExpr().getExpr().(BooleanLiteral).isTrue() and
469-
tp = TResolved("TrueClass") and
470-
exact = true
471-
or
472-
result.asExpr().getExpr() instanceof IntegerLiteral and
473-
tp = TResolved("Integer") and
474-
exact = true
475-
or
476-
result.asExpr().getExpr() instanceof FloatLiteral and
477-
tp = TResolved("Float") and
478-
exact = true
479-
or
480-
result.asExpr().getExpr() instanceof RationalLiteral and
481-
tp = TResolved("Rational") and
482-
exact = true
483-
or
484-
result.asExpr().getExpr() instanceof ComplexLiteral and
485-
tp = TResolved("Complex") and
486-
exact = true
487-
or
488-
result.asExpr().getExpr() instanceof StringlikeLiteral and
489-
tp = TResolved("String") and
490-
exact = true
491-
or
492-
result.asExpr() instanceof CfgNodes::ExprNodes::ArrayLiteralCfgNode and
493-
tp = TResolved("Array") and
494-
exact = true
495-
or
496-
result.asExpr() instanceof CfgNodes::ExprNodes::HashLiteralCfgNode and
497-
tp = TResolved("Hash") and
498-
exact = true
499-
or
500-
result.asExpr().getExpr() instanceof MethodBase and
501-
tp = TResolved("Symbol") and
502-
exact = true
503-
or
504-
result.asParameter() instanceof BlockParameter and
505-
tp = TResolved("Proc") and
506-
exact = true
456+
/** Holds if `n` is an instance of type `tp`. */
457+
private predicate isInstance(DataFlow::LocalSourceNode n, Module tp, boolean exact) {
458+
n.asExpr().getExpr() instanceof NilLiteral and
459+
tp = TResolved("NilClass") and
460+
exact = true
461+
or
462+
n.asExpr().getExpr().(BooleanLiteral).isFalse() and
463+
tp = TResolved("FalseClass") and
464+
exact = true
465+
or
466+
n.asExpr().getExpr().(BooleanLiteral).isTrue() and
467+
tp = TResolved("TrueClass") and
468+
exact = true
469+
or
470+
n.asExpr().getExpr() instanceof IntegerLiteral and
471+
tp = TResolved("Integer") and
472+
exact = true
473+
or
474+
n.asExpr().getExpr() instanceof FloatLiteral and
475+
tp = TResolved("Float") and
476+
exact = true
477+
or
478+
n.asExpr().getExpr() instanceof RationalLiteral and
479+
tp = TResolved("Rational") and
480+
exact = true
481+
or
482+
n.asExpr().getExpr() instanceof ComplexLiteral and
483+
tp = TResolved("Complex") and
484+
exact = true
485+
or
486+
n.asExpr().getExpr() instanceof StringlikeLiteral and
487+
tp = TResolved("String") and
488+
exact = true
489+
or
490+
n.asExpr() instanceof CfgNodes::ExprNodes::ArrayLiteralCfgNode and
491+
tp = TResolved("Array") and
492+
exact = true
493+
or
494+
n.asExpr() instanceof CfgNodes::ExprNodes::HashLiteralCfgNode and
495+
tp = TResolved("Hash") and
496+
exact = true
497+
or
498+
n.asExpr().getExpr() instanceof MethodBase and
499+
tp = TResolved("Symbol") and
500+
exact = true
501+
or
502+
n.asParameter() instanceof BlockParameter and
503+
tp = TResolved("Proc") and
504+
exact = true
505+
or
506+
n.asExpr().getExpr() instanceof Lambda and
507+
tp = TResolved("Proc") and
508+
exact = true
509+
or
510+
exists(CfgNodes::ExprNodes::CallCfgNode call, DataFlow::LocalSourceNode sourceNode |
511+
methodCall(call, sourceNode, "new") and
512+
exact = true and
513+
n.asExpr() = call
514+
|
515+
// `C.new`
516+
sourceNode = trackModuleAccess(tp)
507517
or
508-
result.asExpr().getExpr() instanceof Lambda and
509-
tp = TResolved("Proc") and
510-
exact = true
518+
// `self.new` inside a module
519+
selfInModule(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
511520
or
512-
exists(CfgNodes::ExprNodes::CallCfgNode call, DataFlow::LocalSourceNode sourceNode |
513-
methodCall(call, sourceNode, "new") and
514-
exact = true and
515-
result.asExpr() = call
516-
|
517-
// `C.new`
518-
sourceNode = trackModuleAccess(tp)
519-
or
520-
// `self.new` inside a module
521-
selfInModule(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
521+
// `self.new` inside a (singleton) method
522+
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
523+
)
524+
or
525+
// `self` reference in method or top-level (but not in module, where instance
526+
// methods cannot be called; only singleton methods)
527+
n =
528+
any(SsaSelfDefinitionNode self |
529+
selfInMethod(self.getVariable(), tp) and
530+
exact = false
522531
or
523-
// `self.new` inside a (singleton) method
524-
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
532+
selfInToplevel(self.getVariable(), tp) and
533+
exact = true
525534
)
526-
or
527-
// `self` reference in method or top-level (but not in module, where instance
528-
// methods cannot be called; only singleton methods)
529-
result =
530-
any(SsaSelfDefinitionNode self |
531-
selfInMethod(self.getVariable(), tp) and
532-
exact = false
533-
or
534-
selfInToplevel(self.getVariable(), tp) and
535-
exact = true
536-
)
535+
or
536+
// `in C => c then c.foo`
537+
asModulePattern(n, tp) and
538+
exact = false
539+
or
540+
// `case object when C then object.foo`
541+
hasAdjacentTypeCheckedReads(_, _, n.asExpr(), tp) and
542+
exact = false
543+
}
544+
545+
pragma[nomagic]
546+
private DataFlow::LocalSourceNode trackInstance(Module tp, boolean exact, TypeTracker t) {
547+
t.start() and
548+
(
549+
isInstance(result, tp, exact)
537550
or
538551
exists(Module m |
539552
(if m.isClass() then tp = TResolved("Class") else tp = TResolved("Module")) and
@@ -548,14 +561,6 @@ private DataFlow::LocalSourceNode trackInstance(Module tp, boolean exact, TypeTr
548561
// needed for e.g. `self.puts`
549562
selfInMethod(result.(SsaSelfDefinitionNode).getVariable(), m)
550563
)
551-
or
552-
// `in C => c then c.foo`
553-
asModulePattern(result, tp) and
554-
exact = false
555-
or
556-
// `case object when C then object.foo`
557-
hasAdjacentTypeCheckedReads(_, _, result.asExpr(), tp) and
558-
exact = false
559564
)
560565
or
561566
exists(TypeTracker t2, StepSummary summary |
@@ -729,19 +734,77 @@ private DataFlow::LocalSourceNode trackSingletonMethodOnInstance(MethodBase meth
729734
result = trackSingletonMethodOnInstance(method, name, TypeTracker::end())
730735
}
731736

737+
/** Same as `isInstance`, but includes local must-flow through SSA definitions. */
738+
private predicate isInstanceLocalFlow(DataFlow::Node n, Module tp, boolean exact) {
739+
isInstance(n, tp, exact)
740+
or
741+
exists(DataFlow::Node mid | isInstanceLocalFlow(mid, tp, exact) |
742+
n.asExpr() = mid.(SsaDefinitionNode).getDefinition().getARead()
743+
or
744+
n.(SsaDefinitionNode).getDefinition().(Ssa::WriteDefinition).assigns(mid.asExpr())
745+
)
746+
}
747+
748+
/**
749+
* Holds if `ctx` targets `c`, which is the enclosing callable of `call`, and
750+
* the receiver of `call` is a parameter access, where the corresponding argument
751+
* of `ctx` has type `tp`.
752+
*
753+
* `name` is the name of the method being called by `call`, and `exact` is pertaining
754+
* to the type of the argument.
755+
*/
756+
pragma[nomagic]
757+
private predicate mayBenefitFromCallContext0(
758+
CfgNodes::ExprNodes::CallCfgNode ctx, CfgNodes::ExprNodes::CallCfgNode call, Callable c,
759+
Module tp, boolean exact, string name
760+
) {
761+
exists(
762+
ParameterNodeImpl p, DataFlow::Node ssaNode, ParameterPosition ppos, ArgumentPosition apos,
763+
ArgumentNode arg
764+
|
765+
ssaNode = trackInstance(_, _) and
766+
LocalFlow::localFlowSsaParamInput(p, ssaNode) and
767+
methodCall(pragma[only_bind_into](call), pragma[only_bind_into](ssaNode),
768+
pragma[only_bind_into](name)) and
769+
c = call.getScope() and
770+
p.isParameterOf(TCfgScope(c), ppos) and
771+
getTarget(ctx) = c and
772+
arg.sourceArgumentOf(ctx, apos) and
773+
parameterMatch(ppos, apos) and
774+
isInstanceLocalFlow(arg, pragma[only_bind_out](tp), exact) and
775+
exists(lookupMethod(tp, pragma[only_bind_into](name)))
776+
)
777+
}
778+
732779
/**
733780
* Holds if the set of viable implementations that can be called by `call`
734781
* might be improved by knowing the call context. This is the case if the
735-
* qualifier accesses a parameter of the enclosing callable `c` (including
782+
* receiver accesses a parameter of the enclosing callable `c` (including
736783
* the implicit `self` parameter).
737784
*/
738-
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) { none() }
785+
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
786+
call.asCall() =
787+
any(CfgNodes::ExprNodes::CallCfgNode callNode |
788+
c.asCallable() = any(Callable encl | mayBenefitFromCallContext0(_, callNode, encl, _, _, _))
789+
)
790+
}
739791

740792
/**
741793
* Gets a viable dispatch target of `call` in the context `ctx`. This is
742794
* restricted to those `call`s for which a context might make a difference.
743795
*/
744-
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
796+
pragma[nomagic]
797+
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
798+
exists(Module tp, Module m, boolean exact, string name |
799+
result.asCallable() = lookupMethod(tp, name) and
800+
mayBenefitFromCallContext0(ctx.asCall(), call.asCall(), _, m, exact, name)
801+
|
802+
tp = m
803+
or
804+
exact = false and
805+
tp.getSuperClass+() = m
806+
)
807+
}
745808

746809
predicate exprNodeReturnedFrom = exprNodeReturnedFromCached/2;
747810

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
failures
2-
| call_sensitivity.rb:51:10:51:10 | x | Unexpected result: hasValueFlow=12 |
3-
| call_sensitivity.rb:51:10:51:10 | x | Unexpected result: hasValueFlow=13 |
42
edges
53
| call_sensitivity.rb:9:7:9:13 | call to taint : | call_sensitivity.rb:9:6:9:14 | ( ... ) |
64
| call_sensitivity.rb:9:7:9:13 | call to taint : | call_sensitivity.rb:9:6:9:14 | ( ... ) |
@@ -40,6 +38,8 @@ edges
4038
| call_sensitivity.rb:44:26:44:33 | call to taint : | call_sensitivity.rb:21:27:21:27 | x : |
4139
| call_sensitivity.rb:50:15:50:15 | x : | call_sensitivity.rb:51:10:51:10 | x |
4240
| call_sensitivity.rb:50:15:50:15 | x : | call_sensitivity.rb:51:10:51:10 | x |
41+
| call_sensitivity.rb:50:15:50:15 | x : | call_sensitivity.rb:51:10:51:10 | x |
42+
| call_sensitivity.rb:50:15:50:15 | x : | call_sensitivity.rb:51:10:51:10 | x |
4343
| call_sensitivity.rb:54:15:54:15 | x : | call_sensitivity.rb:55:13:55:13 | x : |
4444
| call_sensitivity.rb:54:15:54:15 | x : | call_sensitivity.rb:55:13:55:13 | x : |
4545
| call_sensitivity.rb:55:13:55:13 | x : | call_sensitivity.rb:50:15:50:15 | x : |
@@ -52,10 +52,6 @@ edges
5252
| call_sensitivity.rb:64:11:64:18 | call to taint : | call_sensitivity.rb:54:15:54:15 | x : |
5353
| call_sensitivity.rb:65:14:65:22 | call to taint : | call_sensitivity.rb:58:18:58:18 | y : |
5454
| call_sensitivity.rb:65:14:65:22 | call to taint : | call_sensitivity.rb:58:18:58:18 | y : |
55-
| call_sensitivity.rb:74:11:74:18 | call to taint : | call_sensitivity.rb:54:15:54:15 | x : |
56-
| call_sensitivity.rb:74:11:74:18 | call to taint : | call_sensitivity.rb:54:15:54:15 | x : |
57-
| call_sensitivity.rb:75:14:75:22 | call to taint : | call_sensitivity.rb:58:18:58:18 | y : |
58-
| call_sensitivity.rb:75:14:75:22 | call to taint : | call_sensitivity.rb:58:18:58:18 | y : |
5955
nodes
6056
| call_sensitivity.rb:9:6:9:14 | ( ... ) | semmle.label | ( ... ) |
6157
| call_sensitivity.rb:9:6:9:14 | ( ... ) | semmle.label | ( ... ) |
@@ -105,6 +101,8 @@ nodes
105101
| call_sensitivity.rb:44:26:44:33 | call to taint : | semmle.label | call to taint : |
106102
| call_sensitivity.rb:50:15:50:15 | x : | semmle.label | x : |
107103
| call_sensitivity.rb:50:15:50:15 | x : | semmle.label | x : |
104+
| call_sensitivity.rb:50:15:50:15 | x : | semmle.label | x : |
105+
| call_sensitivity.rb:50:15:50:15 | x : | semmle.label | x : |
108106
| call_sensitivity.rb:51:10:51:10 | x | semmle.label | x |
109107
| call_sensitivity.rb:51:10:51:10 | x | semmle.label | x |
110108
| call_sensitivity.rb:54:15:54:15 | x : | semmle.label | x : |
@@ -119,10 +117,6 @@ nodes
119117
| call_sensitivity.rb:64:11:64:18 | call to taint : | semmle.label | call to taint : |
120118
| call_sensitivity.rb:65:14:65:22 | call to taint : | semmle.label | call to taint : |
121119
| call_sensitivity.rb:65:14:65:22 | call to taint : | semmle.label | call to taint : |
122-
| call_sensitivity.rb:74:11:74:18 | call to taint : | semmle.label | call to taint : |
123-
| call_sensitivity.rb:74:11:74:18 | call to taint : | semmle.label | call to taint : |
124-
| call_sensitivity.rb:75:14:75:22 | call to taint : | semmle.label | call to taint : |
125-
| call_sensitivity.rb:75:14:75:22 | call to taint : | semmle.label | call to taint : |
126120
subpaths
127121
#select
128122
| 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 : |
@@ -132,5 +126,13 @@ subpaths
132126
| call_sensitivity.rb:43:32:43:32 | x | call_sensitivity.rb:44:26:44:33 | call to taint : | call_sensitivity.rb:43:32:43:32 | x | $@ | call_sensitivity.rb:44:26:44:33 | call to taint : | call to taint : |
133127
| call_sensitivity.rb:51:10:51:10 | x | call_sensitivity.rb:64:11:64:18 | call to taint : | call_sensitivity.rb:51:10:51:10 | x | $@ | call_sensitivity.rb:64:11:64:18 | call to taint : | call to taint : |
134128
| call_sensitivity.rb:51:10:51:10 | x | call_sensitivity.rb:65:14:65:22 | call to taint : | call_sensitivity.rb:51:10:51:10 | x | $@ | call_sensitivity.rb:65:14:65:22 | call to taint : | call to taint : |
135-
| call_sensitivity.rb:51:10:51:10 | x | call_sensitivity.rb:74:11:74:18 | call to taint : | call_sensitivity.rb:51:10:51:10 | x | $@ | call_sensitivity.rb:74:11:74:18 | call to taint : | call to taint : |
136-
| call_sensitivity.rb:51:10:51:10 | x | call_sensitivity.rb:75:14:75:22 | call to taint : | call_sensitivity.rb:51:10:51:10 | x | $@ | call_sensitivity.rb:75:14:75:22 | call to taint : | call to taint : |
129+
mayBenefitFromCallContext
130+
| call_sensitivity.rb:51:5:51:10 | call to sink | call_sensitivity.rb:50:3:52:5 | method1 |
131+
| call_sensitivity.rb:55:5:55:13 | call to method1 | call_sensitivity.rb:54:3:56:5 | method2 |
132+
| call_sensitivity.rb:59:5:59:16 | call to method1 | call_sensitivity.rb:58:3:60:5 | method3 |
133+
viableImplInCallContext
134+
| call_sensitivity.rb:51:5:51:10 | call to sink | call_sensitivity.rb:55:5:55:13 | call to method1 | call_sensitivity.rb:5:1:7:3 | sink |
135+
| call_sensitivity.rb:55:5:55:13 | call to method1 | call_sensitivity.rb:64:1:64:19 | call to method2 | call_sensitivity.rb:50:3:52:5 | method1 |
136+
| call_sensitivity.rb:55:5:55:13 | call to method1 | call_sensitivity.rb:74:1:74:19 | call to method2 | call_sensitivity.rb:68:3:70:5 | method1 |
137+
| call_sensitivity.rb:59:5:59:16 | call to method1 | call_sensitivity.rb:65:1:65:23 | call to method3 | call_sensitivity.rb:50:3:52:5 | method1 |
138+
| call_sensitivity.rb:59:5:59:16 | call to method1 | call_sensitivity.rb:75:1:75:23 | call to method3 | call_sensitivity.rb:68:3:70:5 | method1 |

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ import ruby
66
import codeql.ruby.DataFlow
77
import TestUtilities.InlineFlowTest
88
import DataFlow::PathGraph
9+
import codeql.ruby.dataflow.internal.DataFlowDispatch as DataFlowDispatch
10+
11+
query predicate mayBenefitFromCallContext = DataFlowDispatch::mayBenefitFromCallContext/2;
12+
13+
query predicate viableImplInCallContext = DataFlowDispatch::viableImplInCallContext/2;
914

1015
from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultTaintFlowConf conf
1116
where conf.hasFlowPath(source, sink)

0 commit comments

Comments
 (0)