Skip to content

Commit d9c2b78

Browse files
aibaarsaibaars
committed
Ruby: flow through instance variables
1 parent c518150 commit d9c2b78

File tree

9 files changed

+200
-2
lines changed

9 files changed

+200
-2
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,17 @@ class GlobalVariableReadAccess extends GlobalVariableAccess, VariableReadAccess
181181
/** An access to an instance variable. */
182182
class InstanceVariableAccess extends VariableAccess instanceof InstanceVariableAccessImpl {
183183
final override string getAPrimaryQlClass() { result = "InstanceVariableAccess" }
184+
185+
/**
186+
* Gets a synthetic `self` variable access.
187+
*/
188+
final SelfVariableAccess getSelfVariableAccess() { synthChild(this, 0, result) }
189+
190+
final override AstNode getAChild(string pred) {
191+
result = VariableAccess.super.getAChild(pred)
192+
or
193+
pred = "getSelfVariableAccess" and result = this.getSelfVariableAccess()
194+
}
184195
}
185196

186197
/** An access to an instance variable where the value is updated. */

‎ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,30 @@ private module ImplicitSelfSynthesis {
210210
regularMethodCallSelfSynthesis(parent, i, child)
211211
}
212212
}
213+
214+
/**
215+
* Gets the "owner" of a node. For real nodes this is the node itself, for synthetic nodes
216+
* this is the closest parent that is a real node.
217+
*/
218+
private TAstNodeReal owner(AstNode node) { result = synthParent*(node) }
219+
220+
/**
221+
* Gets the parent of a synthetic node
222+
*/
223+
private AstNode synthParent(AstNode node) { node = getSynthChild(result, _) }
224+
225+
pragma[nomagic]
226+
private predicate instanceVariableSelfSynthesis(InstanceVariableAccess var, int i, Child child) {
227+
child =
228+
SynthChild(SelfKind(TSelfVariable(scopeOf(toGenerated(owner(var))).getEnclosingSelfScope()))) and
229+
i = 0
230+
}
231+
232+
private class InstanceVariableSelfSynthesis extends Synthesis {
233+
final override predicate child(AstNode parent, int i, Child child) {
234+
instanceVariableSelfSynthesis(parent, i, child)
235+
}
236+
}
213237
}
214238

215239
private module SetterDesugar {

‎ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,22 @@ module ExprNodes {
615615
final override VariableReadAccess getExpr() { result = ExprCfgNode.super.getExpr() }
616616
}
617617

618+
private class InstanceVariableAccessMapping extends ExprChildMapping, InstanceVariableAccess {
619+
override predicate relevantChild(AstNode n) { n = this.getSelfVariableAccess() }
620+
}
621+
622+
/** A control-flow node that wraps a `InstanceVariableAccess` AST expression. */
623+
class InstanceVariableAccessCfgNode extends ExprCfgNode {
624+
override InstanceVariableAccessMapping e;
625+
626+
final override InstanceVariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
627+
628+
/**
629+
* Gets a synthetic `self` variable access.
630+
*/
631+
final CfgNode getSelfVariableAccess() { e.hasCfgChild(e.getSelfVariableAccess(), this, result) }
632+
}
633+
618634
/** A control-flow node that wraps a `VariableWriteAccess` AST expression. */
619635
class VariableWriteAccessCfgNode extends ExprCfgNode {
620636
override VariableWriteAccess e;

‎ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,11 @@ module Trees {
10061006
final override ControlFlowTree getChildElement(int i) { result = this.getComponent(i) }
10071007
}
10081008

1009-
private class InstanceVariableTree extends LeafTree, InstanceVariableAccess { }
1009+
private class InstanceVariableTree extends StandardPostOrderTree, InstanceVariableAccess {
1010+
final override ControlFlowTree getChildElement(int i) {
1011+
result = this.getSelfVariableAccess() and i = 0
1012+
}
1013+
}
10101014

10111015
private class KeywordParameterTree extends DefaultValueParameterTree, KeywordParameter {
10121016
final override Expr getDefaultValueExpr() { result = this.getDefaultValue() }

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,10 @@ private module Cached {
217217
} or
218218
TSelfParameterNode(MethodBase m) or
219219
TBlockParameterNode(MethodBase m) or
220-
TExprPostUpdateNode(CfgNodes::ExprCfgNode n) { n instanceof Argument } or
220+
TExprPostUpdateNode(CfgNodes::ExprCfgNode n) {
221+
n instanceof Argument or
222+
n = any(CfgNodes::ExprNodes::InstanceVariableAccessCfgNode v).getSelfVariableAccess()
223+
} or
221224
TSummaryNode(
222225
FlowSummaryImpl::Public::SummarizedCallable c,
223226
FlowSummaryImpl::Private::SummaryNodeState state
@@ -327,6 +330,7 @@ private module Cached {
327330
not cv.isInt(_) or
328331
cv.getInt() in [0 .. 10]
329332
} or
333+
TFieldContent(string name) { name = any(InstanceVariable v).getName() } or
330334
TUnknownElementContent()
331335

332336
/**
@@ -783,11 +787,42 @@ predicate jumpStep(Node pred, Node succ) {
783787
succ.asExpr().getExpr().(ConstantReadAccess).getValue() = pred.asExpr().getExpr()
784788
}
785789

790+
/**
791+
* Holds if data can flow from `node1` to `node2` via an assignment to
792+
* content `c`.
793+
*/
786794
predicate storeStep(Node node1, ContentSet c, Node node2) {
795+
// Instance variable assignment, `@var = src`
796+
node2.(PostUpdateNode).getPreUpdateNode().asExpr() =
797+
any(CfgNodes::ExprNodes::InstanceVariableAccessCfgNode var |
798+
var.getExpr() instanceof InstanceVariableWriteAccess and
799+
exists(CfgNodes::ExprNodes::AssignExprCfgNode assign |
800+
var = assign.getLhs() and
801+
node1.asExpr() = assign.getRhs()
802+
|
803+
c.isSingleton(any(Content::FieldContent ct |
804+
ct.getName() = var.getExpr().getVariable().getName()
805+
))
806+
)
807+
).getSelfVariableAccess()
808+
or
787809
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
788810
}
789811

812+
/**
813+
* Holds if there is a read step of content `c` from `node1` to `node2`.
814+
*/
790815
predicate readStep(Node node1, ContentSet c, Node node2) {
816+
// Instance variable read access, `@var`
817+
node2.asExpr() =
818+
any(CfgNodes::ExprNodes::InstanceVariableAccessCfgNode var |
819+
var.getExpr() instanceof InstanceVariableReadAccess and
820+
node1.asExpr() = var.getSelfVariableAccess() and
821+
c.isSingleton(any(Content::FieldContent ct |
822+
ct.getName() = var.getExpr().getVariable().getName()
823+
))
824+
)
825+
or
791826
FlowSummaryImpl::Private::Steps::summaryReadStep(node1, c, node2)
792827
}
793828

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,18 @@ module Content {
223223
override string toString() { result = "element" }
224224
}
225225

226+
/** A field of an object, for example an instance variable. */
227+
class FieldContent extends Content, TFieldContent {
228+
private string name;
229+
230+
FieldContent() { this = TFieldContent(name) }
231+
232+
/** Gets the name of the field. */
233+
string getName() { result = name }
234+
235+
override string toString() { result = name }
236+
}
237+
226238
/** Gets the element content corresponding to constant value `cv`. */
227239
ElementContent getElementContent(ConstantValue cv) {
228240
result = TKnownElementContent(cv)

‎ruby/ql/test/library-tests/dataflow/global/Flow.expected

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
failures
2+
edges
3+
| instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:18:3:18 | x : |
4+
| instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:18:3:18 | x : |
5+
| instance_variables.rb:3:18:3:18 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : |
6+
| instance_variables.rb:3:18:3:18 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : |
7+
| instance_variables.rb:5:5:7:7 | self in get_field [@field] : | instance_variables.rb:6:16:6:21 | self [@field] : |
8+
| instance_variables.rb:5:5:7:7 | self in get_field [@field] : | instance_variables.rb:6:16:6:21 | self [@field] : |
9+
| instance_variables.rb:6:16:6:21 | @field : | instance_variables.rb:6:9:6:21 | return : |
10+
| instance_variables.rb:6:16:6:21 | @field : | instance_variables.rb:6:9:6:21 | return : |
11+
| instance_variables.rb:6:16:6:21 | self [@field] : | instance_variables.rb:6:16:6:21 | @field : |
12+
| instance_variables.rb:6:16:6:21 | self [@field] : | instance_variables.rb:6:16:6:21 | @field : |
13+
| instance_variables.rb:11:5:11:8 | [post] self [@foo] : | instance_variables.rb:12:10:12:13 | self [@foo] : |
14+
| instance_variables.rb:11:5:11:8 | [post] self [@foo] : | instance_variables.rb:12:10:12:13 | self [@foo] : |
15+
| instance_variables.rb:11:12:11:22 | call to source : | instance_variables.rb:11:5:11:8 | [post] self [@foo] : |
16+
| instance_variables.rb:11:12:11:22 | call to source : | instance_variables.rb:11:5:11:8 | [post] self [@foo] : |
17+
| instance_variables.rb:12:10:12:13 | self [@foo] : | instance_variables.rb:12:10:12:13 | @foo |
18+
| instance_variables.rb:12:10:12:13 | self [@foo] : | instance_variables.rb:12:10:12:13 | @foo |
19+
| instance_variables.rb:16:1:16:3 | [post] foo [@field] : | instance_variables.rb:17:6:17:8 | foo [@field] : |
20+
| instance_variables.rb:16:1:16:3 | [post] foo [@field] : | instance_variables.rb:17:6:17:8 | foo [@field] : |
21+
| instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:2:19:2:19 | x : |
22+
| instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:2:19:2:19 | x : |
23+
| instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:16:1:16:3 | [post] foo [@field] : |
24+
| instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:16:1:16:3 | [post] foo [@field] : |
25+
| instance_variables.rb:17:6:17:8 | foo [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : |
26+
| instance_variables.rb:17:6:17:8 | foo [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : |
27+
| instance_variables.rb:17:6:17:8 | foo [@field] : | instance_variables.rb:17:6:17:18 | call to get_field |
28+
| instance_variables.rb:17:6:17:8 | foo [@field] : | instance_variables.rb:17:6:17:18 | call to get_field |
29+
nodes
30+
| instance_variables.rb:2:19:2:19 | x : | semmle.label | x : |
31+
| instance_variables.rb:2:19:2:19 | x : | semmle.label | x : |
32+
| instance_variables.rb:3:9:3:14 | [post] self [@field] : | semmle.label | [post] self [@field] : |
33+
| instance_variables.rb:3:9:3:14 | [post] self [@field] : | semmle.label | [post] self [@field] : |
34+
| instance_variables.rb:3:18:3:18 | x : | semmle.label | x : |
35+
| instance_variables.rb:3:18:3:18 | x : | semmle.label | x : |
36+
| instance_variables.rb:5:5:7:7 | self in get_field [@field] : | semmle.label | self in get_field [@field] : |
37+
| instance_variables.rb:5:5:7:7 | self in get_field [@field] : | semmle.label | self in get_field [@field] : |
38+
| instance_variables.rb:6:9:6:21 | return : | semmle.label | return : |
39+
| instance_variables.rb:6:9:6:21 | return : | semmle.label | return : |
40+
| instance_variables.rb:6:16:6:21 | @field : | semmle.label | @field : |
41+
| instance_variables.rb:6:16:6:21 | @field : | semmle.label | @field : |
42+
| instance_variables.rb:6:16:6:21 | self [@field] : | semmle.label | self [@field] : |
43+
| instance_variables.rb:6:16:6:21 | self [@field] : | semmle.label | self [@field] : |
44+
| instance_variables.rb:11:5:11:8 | [post] self [@foo] : | semmle.label | [post] self [@foo] : |
45+
| instance_variables.rb:11:5:11:8 | [post] self [@foo] : | semmle.label | [post] self [@foo] : |
46+
| instance_variables.rb:11:12:11:22 | call to source : | semmle.label | call to source : |
47+
| instance_variables.rb:11:12:11:22 | call to source : | semmle.label | call to source : |
48+
| instance_variables.rb:12:10:12:13 | @foo | semmle.label | @foo |
49+
| instance_variables.rb:12:10:12:13 | @foo | semmle.label | @foo |
50+
| instance_variables.rb:12:10:12:13 | self [@foo] : | semmle.label | self [@foo] : |
51+
| instance_variables.rb:12:10:12:13 | self [@foo] : | semmle.label | self [@foo] : |
52+
| instance_variables.rb:16:1:16:3 | [post] foo [@field] : | semmle.label | [post] foo [@field] : |
53+
| instance_variables.rb:16:1:16:3 | [post] foo [@field] : | semmle.label | [post] foo [@field] : |
54+
| instance_variables.rb:16:15:16:24 | call to source : | semmle.label | call to source : |
55+
| instance_variables.rb:16:15:16:24 | call to source : | semmle.label | call to source : |
56+
| instance_variables.rb:17:6:17:8 | foo [@field] : | semmle.label | foo [@field] : |
57+
| instance_variables.rb:17:6:17:8 | foo [@field] : | semmle.label | foo [@field] : |
58+
| instance_variables.rb:17:6:17:18 | call to get_field | semmle.label | call to get_field |
59+
| instance_variables.rb:17:6:17:18 | call to get_field | semmle.label | call to get_field |
60+
subpaths
61+
| instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : | instance_variables.rb:16:1:16:3 | [post] foo [@field] : |
62+
| instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : | instance_variables.rb:16:1:16:3 | [post] foo [@field] : |
63+
| instance_variables.rb:17:6:17:8 | foo [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : | instance_variables.rb:6:9:6:21 | return : | instance_variables.rb:17:6:17:18 | call to get_field |
64+
| instance_variables.rb:17:6:17:8 | foo [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : | instance_variables.rb:6:9:6:21 | return : | instance_variables.rb:17:6:17:18 | call to get_field |
65+
#select
66+
| instance_variables.rb:12:10:12:13 | @foo | instance_variables.rb:11:12:11:22 | call to source : | instance_variables.rb:12:10:12:13 | @foo | $@ | instance_variables.rb:11:12:11:22 | call to source : | call to source : |
67+
| instance_variables.rb:17:6:17:18 | call to get_field | instance_variables.rb:16:15:16:24 | call to source : | instance_variables.rb:17:6:17:18 | call to get_field | $@ | instance_variables.rb:16:15:16:24 | call to source : | call to source : |

‎ruby/ql/test/library-tests/dataflow/global/Flow.ql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import ruby
6+
import codeql.ruby.DataFlow
7+
private import TestUtilities.InlineFlowTest
8+
import DataFlow::PathGraph
9+
10+
from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
11+
where conf.hasFlowPath(source, sink)
12+
select sink, source, sink, "$@", source, source.toString()

‎ruby/ql/test/library-tests/dataflow/global/instance_variables.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
class Foo
2+
def set_field x
3+
@field = x
4+
end
5+
def get_field
6+
return @field
7+
end
8+
def inc_field
9+
@field += 1
10+
end
11+
@foo = source("7")
12+
sink(@foo) # $ hasValueFlow=7
13+
14+
end
15+
foo = Foo.new
16+
foo.set_field(source(42))
17+
sink(foo.get_field) # $ hasValueFlow=42

0 commit comments

Comments
 (0)