Skip to content

Commit 0e0923e

Browse files
hvitvedhvitved
committed
Ruby: Call sensitive instance method resolution
1 parent 0f60d85 commit 0e0923e

File tree

4 files changed

+184
-103
lines changed

4 files changed

+184
-103
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: 162 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -442,90 +442,103 @@ private DataFlow::LocalSourceNode trackModuleAccess(Module m) {
442442
result = trackModuleAccess(m, TypeTracker::end())
443443
}
444444

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

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

793847
/**
794848
* Gets a viable dispatch target of `call` in the context `ctx`. This is
795849
* restricted to those `call`s for which a context might make a difference.
796850
*/
797-
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
851+
pragma[nomagic]
852+
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
853+
exists(CfgNodes::ExprNodes::CallCfgNode call0, Callable res |
854+
call0 = call.asCall() and
855+
res = result.asCallable() and
856+
res = getTarget(call0) and // make sure to not include e.g. private methods
857+
exists(Module tp, Module m, boolean exact, string name |
858+
res = lookupMethod(tp, name) and
859+
mayBenefitFromCallContext0(ctx.asCall(), pragma[only_bind_into](call0), _,
860+
pragma[only_bind_into](m), exact, pragma[only_bind_into](name))
861+
|
862+
tp = m
863+
or
864+
exact = false and
865+
tp.getSuperClass+() = m
866+
)
867+
)
868+
}
798869

799870
predicate exprNodeReturnedFrom = exprNodeReturnedFromCached/2;
800871

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