Skip to content

Commit 7bae643

Browse files
hvitvedhvitved
committed
Ruby: Call sensitive instance method resolution
1 parent c3ae159 commit 7bae643

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
@@ -444,87 +444,100 @@ private DataFlow::LocalSourceNode trackModuleAccess(Module m) {
444444
result = trackModuleAccess(m, TypeTracker::end())
445445
}
446446

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

794+
/** Same as `isInstance`, but includes local must-flow through SSA definitions. */
795+
private predicate isInstanceLocalFlow(DataFlow::Node n, Module tp, boolean exact) {
796+
isInstance(n, tp, exact)
797+
or
798+
exists(DataFlow::Node mid | isInstanceLocalFlow(mid, tp, exact) |
799+
n.asExpr() = mid.(SsaDefinitionNode).getDefinition().getARead()
800+
or
801+
n.(SsaDefinitionNode).getDefinition().(Ssa::WriteDefinition).assigns(mid.asExpr())
802+
)
803+
}
804+
805+
/**
806+
* Holds if `ctx` targets `c`, which is the enclosing callable of `call`, and
807+
* the receiver of `call` is a parameter access, where the corresponding argument
808+
* of `ctx` has type `tp`.
809+
*
810+
* `name` is the name of the method being called by `call`, and `exact` is pertaining
811+
* to the type of the argument.
812+
*/
813+
pragma[nomagic]
814+
private predicate mayBenefitFromCallContext0(
815+
CfgNodes::ExprNodes::CallCfgNode ctx, CfgNodes::ExprNodes::CallCfgNode call, Callable c,
816+
Module tp, boolean exact, string name
817+
) {
818+
exists(
819+
ParameterNodeImpl p, DataFlow::Node ssaNode, ParameterPosition ppos, ArgumentPosition apos,
820+
ArgumentNode arg
821+
|
822+
ssaNode = trackInstance(_, _) and
823+
LocalFlow::localFlowSsaParamInput(p, ssaNode) and
824+
flowsToMethodCall(pragma[only_bind_into](call), pragma[only_bind_into](ssaNode),
825+
pragma[only_bind_into](name)) and
826+
c = call.getScope() and
827+
p.isParameterOf(TCfgScope(c), ppos) and
828+
getTarget(ctx) = c and
829+
arg.sourceArgumentOf(ctx, apos) and
830+
parameterMatch(ppos, apos) and
831+
isInstanceLocalFlow(arg, pragma[only_bind_out](tp), exact) and
832+
exists(lookupMethod(tp, pragma[only_bind_into](name)))
833+
)
834+
}
835+
789836
/**
790837
* Holds if the set of viable implementations that can be called by `call`
791838
* might be improved by knowing the call context. This is the case if the
792-
* qualifier accesses a parameter of the enclosing callable `c` (including
839+
* receiver accesses a parameter of the enclosing callable `c` (including
793840
* the implicit `self` parameter).
794841
*/
795-
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) { none() }
842+
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
843+
call.asCall() =
844+
any(CfgNodes::ExprNodes::CallCfgNode callNode |
845+
c.asCallable() = any(Callable encl | mayBenefitFromCallContext0(_, callNode, encl, _, _, _))
846+
)
847+
}
796848

797849
/**
798850
* Gets a viable dispatch target of `call` in the context `ctx`. This is
799851
* restricted to those `call`s for which a context might make a difference.
800852
*/
801-
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
853+
pragma[nomagic]
854+
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
855+
exists(Module tp, Module m, boolean exact, string name |
856+
result.asCallable() = lookupMethod(tp, name) and
857+
mayBenefitFromCallContext0(ctx.asCall(), call.asCall(), _, m, exact, name)
858+
|
859+
tp = m
860+
or
861+
exact = false and
862+
tp.getSuperClass+() = m
863+
)
864+
}
802865

803866
predicate exprNodeReturnedFrom = exprNodeReturnedFromCached/2;
804867

‎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)