Skip to content

Commit dc0720b

Browse files
hvitvedhvitved
committed
Ruby: Call sensitive instance method resolution
1 parent 64978b0 commit dc0720b

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
@@ -14,6 +14,9 @@ class Module extends TModule {
1414
/** Gets the super class of this module, if any. */
1515
Module getSuperClass() { result = getSuperClass(this) }
1616

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

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

Lines changed: 162 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -438,90 +438,103 @@ private DataFlow::LocalSourceNode trackModuleAccess(Module m) {
438438
result = trackModuleAccess(m, TypeTracker::end())
439439
}
440440

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

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

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

795866
predicate exprNodeReturnedFrom = exprNodeReturnedFromCached/2;
796867

‎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 codeql.ruby.AST
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)