Skip to content

Commit b0a97f9

Browse files
aibaarsaibaars
committed
Ruby: flow through getters/setters
1 parent 79fb9e8 commit b0a97f9

File tree

3 files changed

+120
-2
lines changed

3 files changed

+120
-2
lines changed

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,14 @@ private module Cached {
349349
TUnknownElementContent() or
350350
TKnownPairValueContent(ConstantValue cv) or
351351
TUnknownPairValueContent() or
352-
TFieldContent(string name) { name = any(InstanceVariable v).getName() }
352+
TFieldContent(string name) {
353+
name = any(InstanceVariable v).getName()
354+
or
355+
exists(SetterMethodCall c, string methodName |
356+
methodName = c.getMethodName() and
357+
name = "@" + methodName.prefix(methodName.length() - 1)
358+
)
359+
}
353360

354361
/**
355362
* Holds if `e` is an `ExprNode` that may be returned by a call to `c`.
@@ -825,6 +832,20 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
825832
)
826833
).getReceiver()
827834
or
835+
// Attribute assignment, `receiver.property = value`
836+
node2.(PostUpdateNode).getPreUpdateNode().asExpr() =
837+
any(CfgNodes::ExprNodes::MethodCallCfgNode call |
838+
call.getExpr() instanceof SetterMethodCall and
839+
node1.asExpr() = call.getArgument(0) and
840+
call.getNumberOfArguments() = 1 and
841+
c.isSingleton(any(Content::FieldContent ct, string methodName |
842+
methodName = call.getExpr().getMethodName() and
843+
ct.getName() = "@" + methodName.prefix(methodName.length() - 1)
844+
|
845+
ct
846+
))
847+
).getReceiver()
848+
or
828849
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
829850
or
830851
// Needed for pairs passed into method calls where the key is not a symbol,
@@ -860,6 +881,19 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
860881
))
861882
)
862883
or
884+
// Attribute read, `receiver.field`. Note that we do not check whether
885+
// the `field` method is really an attribute reader. This is probably fine
886+
// because the read step has only effect if there exists a matching store step
887+
// (instance variable assignmentor setter method call).
888+
node2.asExpr() =
889+
any(CfgNodes::ExprNodes::MethodCallCfgNode call |
890+
node1.asExpr() = call.getReceiver() and
891+
call.getNumberOfArguments() = 0 and
892+
c.isSingleton(any(Content::FieldContent ct |
893+
ct.getName() = "@" + call.getExpr().getMethodName()
894+
))
895+
)
896+
or
863897
FlowSummaryImpl::Private::Steps::summaryReadStep(node1, c, node2)
864898
}
865899

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,34 @@ edges
3333
| instance_variables.rb:20:15:20:23 | call to source : | instance_variables.rb:20:1:20:3 | [post] bar [@field] : |
3434
| instance_variables.rb:21:6:21:8 | bar [@field] : | instance_variables.rb:8:5:10:7 | self in inc_field [@field] : |
3535
| instance_variables.rb:21:6:21:8 | bar [@field] : | instance_variables.rb:21:6:21:18 | call to inc_field |
36+
| instance_variables.rb:24:1:24:4 | [post] foo1 [@field] : | instance_variables.rb:25:6:25:9 | foo1 [@field] : |
37+
| instance_variables.rb:24:1:24:4 | [post] foo1 [@field] : | instance_variables.rb:25:6:25:9 | foo1 [@field] : |
38+
| instance_variables.rb:24:14:24:23 | call to source : | instance_variables.rb:24:1:24:4 | [post] foo1 [@field] : |
39+
| instance_variables.rb:24:14:24:23 | call to source : | instance_variables.rb:24:1:24:4 | [post] foo1 [@field] : |
40+
| instance_variables.rb:25:6:25:9 | foo1 [@field] : | instance_variables.rb:25:6:25:15 | call to field |
41+
| instance_variables.rb:25:6:25:9 | foo1 [@field] : | instance_variables.rb:25:6:25:15 | call to field |
42+
| instance_variables.rb:28:1:28:4 | [post] foo2 [@field] : | instance_variables.rb:29:6:29:9 | foo2 [@field] : |
43+
| instance_variables.rb:28:1:28:4 | [post] foo2 [@field] : | instance_variables.rb:29:6:29:9 | foo2 [@field] : |
44+
| instance_variables.rb:28:14:28:23 | call to source : | instance_variables.rb:28:1:28:4 | [post] foo2 [@field] : |
45+
| instance_variables.rb:28:14:28:23 | call to source : | instance_variables.rb:28:1:28:4 | [post] foo2 [@field] : |
46+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : |
47+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : |
48+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | instance_variables.rb:29:6:29:19 | call to get_field |
49+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | instance_variables.rb:29:6:29:19 | call to get_field |
50+
| instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : | instance_variables.rb:33:6:33:9 | foo3 [@field] : |
51+
| instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : | instance_variables.rb:33:6:33:9 | foo3 [@field] : |
52+
| instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:2:19:2:19 | x : |
53+
| instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:2:19:2:19 | x : |
54+
| instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : |
55+
| instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : |
56+
| instance_variables.rb:33:6:33:9 | foo3 [@field] : | instance_variables.rb:33:6:33:15 | call to field |
57+
| instance_variables.rb:33:6:33:9 | foo3 [@field] : | instance_variables.rb:33:6:33:15 | call to field |
58+
| instance_variables.rb:36:1:36:4 | [post] foo4 [@other] : | instance_variables.rb:37:6:37:9 | foo4 [@other] : |
59+
| instance_variables.rb:36:1:36:4 | [post] foo4 [@other] : | instance_variables.rb:37:6:37:9 | foo4 [@other] : |
60+
| instance_variables.rb:36:14:36:23 | call to source : | instance_variables.rb:36:1:36:4 | [post] foo4 [@other] : |
61+
| instance_variables.rb:36:14:36:23 | call to source : | instance_variables.rb:36:1:36:4 | [post] foo4 [@other] : |
62+
| instance_variables.rb:37:6:37:9 | foo4 [@other] : | instance_variables.rb:37:6:37:15 | call to other |
63+
| instance_variables.rb:37:6:37:9 | foo4 [@other] : | instance_variables.rb:37:6:37:15 | call to other |
3664
nodes
3765
| instance_variables.rb:2:19:2:19 | x : | semmle.label | x : |
3866
| instance_variables.rb:2:19:2:19 | x : | semmle.label | x : |
@@ -70,6 +98,38 @@ nodes
7098
| instance_variables.rb:20:15:20:23 | call to source : | semmle.label | call to source : |
7199
| instance_variables.rb:21:6:21:8 | bar [@field] : | semmle.label | bar [@field] : |
72100
| instance_variables.rb:21:6:21:18 | call to inc_field | semmle.label | call to inc_field |
101+
| instance_variables.rb:24:1:24:4 | [post] foo1 [@field] : | semmle.label | [post] foo1 [@field] : |
102+
| instance_variables.rb:24:1:24:4 | [post] foo1 [@field] : | semmle.label | [post] foo1 [@field] : |
103+
| instance_variables.rb:24:14:24:23 | call to source : | semmle.label | call to source : |
104+
| instance_variables.rb:24:14:24:23 | call to source : | semmle.label | call to source : |
105+
| instance_variables.rb:25:6:25:9 | foo1 [@field] : | semmle.label | foo1 [@field] : |
106+
| instance_variables.rb:25:6:25:9 | foo1 [@field] : | semmle.label | foo1 [@field] : |
107+
| instance_variables.rb:25:6:25:15 | call to field | semmle.label | call to field |
108+
| instance_variables.rb:25:6:25:15 | call to field | semmle.label | call to field |
109+
| instance_variables.rb:28:1:28:4 | [post] foo2 [@field] : | semmle.label | [post] foo2 [@field] : |
110+
| instance_variables.rb:28:1:28:4 | [post] foo2 [@field] : | semmle.label | [post] foo2 [@field] : |
111+
| instance_variables.rb:28:14:28:23 | call to source : | semmle.label | call to source : |
112+
| instance_variables.rb:28:14:28:23 | call to source : | semmle.label | call to source : |
113+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | semmle.label | foo2 [@field] : |
114+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | semmle.label | foo2 [@field] : |
115+
| instance_variables.rb:29:6:29:19 | call to get_field | semmle.label | call to get_field |
116+
| instance_variables.rb:29:6:29:19 | call to get_field | semmle.label | call to get_field |
117+
| instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : | semmle.label | [post] foo3 [@field] : |
118+
| instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : | semmle.label | [post] foo3 [@field] : |
119+
| instance_variables.rb:32:16:32:25 | call to source : | semmle.label | call to source : |
120+
| instance_variables.rb:32:16:32:25 | call to source : | semmle.label | call to source : |
121+
| instance_variables.rb:33:6:33:9 | foo3 [@field] : | semmle.label | foo3 [@field] : |
122+
| instance_variables.rb:33:6:33:9 | foo3 [@field] : | semmle.label | foo3 [@field] : |
123+
| instance_variables.rb:33:6:33:15 | call to field | semmle.label | call to field |
124+
| instance_variables.rb:33:6:33:15 | call to field | semmle.label | call to field |
125+
| instance_variables.rb:36:1:36:4 | [post] foo4 [@other] : | semmle.label | [post] foo4 [@other] : |
126+
| instance_variables.rb:36:1:36:4 | [post] foo4 [@other] : | semmle.label | [post] foo4 [@other] : |
127+
| instance_variables.rb:36:14:36:23 | call to source : | semmle.label | call to source : |
128+
| instance_variables.rb:36:14:36:23 | call to source : | semmle.label | call to source : |
129+
| instance_variables.rb:37:6:37:9 | foo4 [@other] : | semmle.label | foo4 [@other] : |
130+
| instance_variables.rb:37:6:37:9 | foo4 [@other] : | semmle.label | foo4 [@other] : |
131+
| instance_variables.rb:37:6:37:15 | call to other | semmle.label | call to other |
132+
| instance_variables.rb:37:6:37:15 | call to other | semmle.label | call to other |
73133
subpaths
74134
| 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] : |
75135
| 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] : |
@@ -78,7 +138,15 @@ subpaths
78138
| instance_variables.rb:20:15:20:23 | call to source : | instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : | instance_variables.rb:20:1:20:3 | [post] bar [@field] : |
79139
| instance_variables.rb:21:6:21:8 | bar [@field] : | instance_variables.rb:8:5:10:7 | self in inc_field [@field] : | instance_variables.rb:8:5:10:7 | self in inc_field [@field] : | instance_variables.rb:21:6:21:18 | call to inc_field |
80140
| instance_variables.rb:21:6:21:8 | bar [@field] : | instance_variables.rb:8:5:10:7 | self in inc_field [@field] : | instance_variables.rb:9:9:9:14 | [post] self [@field] : | instance_variables.rb:21:6:21:18 | call to inc_field |
141+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : | instance_variables.rb:6:9:6:21 | return : | instance_variables.rb:29:6:29:19 | call to get_field |
142+
| instance_variables.rb:29:6:29:9 | foo2 [@field] : | instance_variables.rb:5:5:7:7 | self in get_field [@field] : | instance_variables.rb:6:9:6:21 | return : | instance_variables.rb:29:6:29:19 | call to get_field |
143+
| instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : | instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : |
144+
| instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:2:19:2:19 | x : | instance_variables.rb:3:9:3:14 | [post] self [@field] : | instance_variables.rb:32:1:32:4 | [post] foo3 [@field] : |
81145
#select
82146
| 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 : |
83147
| 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 : |
84148
| instance_variables.rb:21:6:21:18 | call to inc_field | instance_variables.rb:20:15:20:23 | call to source : | instance_variables.rb:21:6:21:18 | call to inc_field | $@ | instance_variables.rb:20:15:20:23 | call to source : | call to source : |
149+
| instance_variables.rb:25:6:25:15 | call to field | instance_variables.rb:24:14:24:23 | call to source : | instance_variables.rb:25:6:25:15 | call to field | $@ | instance_variables.rb:24:14:24:23 | call to source : | call to source : |
150+
| instance_variables.rb:29:6:29:19 | call to get_field | instance_variables.rb:28:14:28:23 | call to source : | instance_variables.rb:29:6:29:19 | call to get_field | $@ | instance_variables.rb:28:14:28:23 | call to source : | call to source : |
151+
| instance_variables.rb:33:6:33:15 | call to field | instance_variables.rb:32:16:32:25 | call to source : | instance_variables.rb:33:6:33:15 | call to field | $@ | instance_variables.rb:32:16:32:25 | call to source : | call to source : |
152+
| instance_variables.rb:37:6:37:15 | call to other | instance_variables.rb:36:14:36:23 | call to source : | instance_variables.rb:37:6:37:15 | call to other | $@ | instance_variables.rb:36:14:36:23 | call to source : | call to source : |

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,20 @@ def inc_field
1818

1919
bar = Foo.new
2020
bar.set_field(source(5))
21-
sink(bar.inc_field) # $ hasTaintFlow=5
21+
sink(bar.inc_field) # $ hasTaintFlow=5
22+
23+
foo1 = Foo.new
24+
foo1.field = source(20)
25+
sink(foo1.field) # $ hasValueFlow=20
26+
27+
foo2 = Foo.new
28+
foo2.field = source(21)
29+
sink(foo2.get_field) # $ hasValueFlow=21
30+
31+
foo3 = Foo.new
32+
foo3.set_field(source(22))
33+
sink(foo3.field) # $ hasValueFlow=22
34+
35+
foo4 = "hello"
36+
foo4.other = source(23)
37+
sink(foo4.other) # $ hasValueFlow=23

0 commit comments

Comments
 (0)