Skip to content

Commit e3a0449

Browse files
asgerfasgerf
committed
Ruby: minor overhaul of ActiveResource model
1 parent 8bc4193 commit e3a0449

3 files changed

Lines changed: 114 additions & 111 deletions

File tree

‎ruby/ql/lib/codeql/ruby/frameworks/ActiveResource.qll‎

Lines changed: 92 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@ module ActiveResource {
1818
* An ActiveResource model class. This is any (transitive) subclass of ActiveResource.
1919
*/
2020
pragma[nomagic]
21-
private API::Node modelApiNode() {
22-
result = API::getTopLevelMember("ActiveResource").getMember("Base").getASubclass()
21+
private API::Node activeResourceBaseClass() {
22+
result = API::getTopLevelMember("ActiveResource").getMember("Base")
23+
}
24+
25+
private DataFlow::ClassNode activeResourceClass() {
26+
result = activeResourceBaseClass().getADescendentModule()
2327
}
2428

2529
/**
@@ -30,16 +34,8 @@ module ActiveResource {
3034
* end
3135
* ```
3236
*/
33-
class ModelClass extends ClassDeclaration {
34-
API::Node model;
35-
36-
ModelClass() {
37-
model = modelApiNode() and
38-
this.getSuperclassExpr() = model.getAValueReachableFromSource().asExpr().getExpr()
39-
}
40-
41-
/** Gets the API node for this model */
42-
API::Node getModelApiNode() { result = model }
37+
class ModelClassNode extends DataFlow::ClassNode {
38+
ModelClassNode() { this = activeResourceClass() }
4339

4440
/** Gets a call to `site=`, which sets the base URL for this model. */
4541
SiteAssignCall getASiteAssignment() { result.getModelClass() = this }
@@ -49,6 +45,46 @@ module ActiveResource {
4945
c = this.getASiteAssignment() and
5046
c.disablesCertificateValidation()
5147
}
48+
49+
/** Gets a method call on this class that returns an instance of the class. */
50+
private DataFlow::CallNode getAChainedCall() {
51+
result.(FindCall).getModelClass() = this
52+
or
53+
result.(CreateCall).getModelClass() = this
54+
or
55+
result.(CustomHttpCall).getModelClass() = this
56+
or
57+
result.(CollectionCall).getCollection().getModelClass() = this and
58+
result.getMethodName() = ["first", "last"]
59+
}
60+
61+
/** Gets an API node referring to an instance of this class. */
62+
API::Node getAnInstanceReference() {
63+
result = this.trackInstance()
64+
or
65+
result = this.getAChainedCall().track()
66+
}
67+
}
68+
69+
/** DEPRECATED. Use `ModelClassNode` instead. */
70+
deprecated class ModelClass extends ClassDeclaration {
71+
private ModelClassNode cls;
72+
73+
ModelClass() { this = cls.getADeclaration() }
74+
75+
/** Gets the class for which this is a declaration. */
76+
ModelClassNode getClassNode() { result = cls }
77+
78+
/** Gets the API node for this class object. */
79+
deprecated API::Node getModelApiNode() { result = cls.trackModule() }
80+
81+
/** Gets a call to `site=`, which sets the base URL for this model. */
82+
SiteAssignCall getASiteAssignment() { result = cls.getASiteAssignment() }
83+
84+
/** Holds if `c` sets a base URL which does not use HTTPS. */
85+
predicate disablesCertificateValidation(SiteAssignCall c) {
86+
cls.disablesCertificateValidation(c)
87+
}
5288
}
5389

5490
/**
@@ -62,38 +98,31 @@ module ActiveResource {
6298
* ```
6399
*/
64100
class ModelClassMethodCall extends DataFlow::CallNode {
65-
API::Node model;
101+
private ModelClassNode cls;
66102

67-
ModelClassMethodCall() {
68-
model = modelApiNode() and
69-
this = classMethodCall(model, _)
70-
}
103+
ModelClassMethodCall() { this = cls.trackModule().getAMethodCall(_) }
71104

72105
/** Gets the model class for this call. */
73-
ModelClass getModelClass() { result.getModelApiNode() = model }
106+
ModelClassNode getModelClass() { result = cls }
74107
}
75108

76109
/**
77110
* A call to `site=` on an ActiveResource model class.
78111
* This sets the base URL for all HTTP requests made by this class.
79112
*/
80-
private class SiteAssignCall extends DataFlow::CallNode {
81-
API::Node model;
82-
83-
SiteAssignCall() { model = modelApiNode() and this = classMethodCall(model, "site=") }
113+
private class SiteAssignCall extends ModelClassMethodCall {
114+
SiteAssignCall() { this.getMethodName() = "site=" }
84115

85116
/**
86117
* Gets a node that contributes to the URLs used for HTTP requests by the parent
87118
* class.
88119
*/
89120
DataFlow::Node getAUrlPart() { result = this.getArgument(0) }
90121

91-
/** Gets the model class for this call. */
92-
ModelClass getModelClass() { result.getModelApiNode() = model }
93-
94122
/** Holds if this site value specifies HTTP rather than HTTPS. */
95123
predicate disablesCertificateValidation() {
96124
this.getAUrlPart()
125+
// TODO: We should not need all this just to get the string value
97126
.asExpr()
98127
.(ExprNodes::AssignExprCfgNode)
99128
.getRhs()
@@ -141,87 +170,70 @@ module ActiveResource {
141170
}
142171

143172
/**
173+
* DEPRECATED. Use `ModelClassNode.getAnInstanceReference()` instead.
174+
*
144175
* An ActiveResource model object.
145176
*/
146-
class ModelInstance extends DataFlow::Node {
147-
ModelClass cls;
148-
149-
ModelInstance() {
150-
exists(API::Node model | model = modelApiNode() |
151-
this = model.getInstance().getAValueReachableFromSource() and
152-
cls.getModelApiNode() = model
153-
)
154-
or
155-
exists(FindCall call | call.flowsTo(this) | cls = call.getModelClass())
156-
or
157-
exists(CreateCall call | call.flowsTo(this) | cls = call.getModelClass())
158-
or
159-
exists(CustomHttpCall call | call.flowsTo(this) | cls = call.getModelClass())
160-
or
161-
exists(CollectionCall call |
162-
call.getMethodName() = ["first", "last"] and
163-
call.flowsTo(this)
164-
|
165-
cls = call.getCollection().getModelClass()
166-
)
167-
}
177+
deprecated class ModelInstance extends DataFlow::Node {
178+
private ModelClassNode cls;
179+
180+
ModelInstance() { this = cls.getAnInstanceReference().getAValueReachableFromSource() }
168181

169182
/** Gets the model class for this instance. */
170-
ModelClass getModelClass() { result = cls }
183+
ModelClassNode getModelClass() { result = cls }
171184
}
172185

173186
/**
174187
* A call to a method on an ActiveResource model object.
175188
*/
176189
class ModelInstanceMethodCall extends DataFlow::CallNode {
177-
ModelInstance i;
190+
private ModelClassNode cls;
178191

179-
ModelInstanceMethodCall() { this.getReceiver() = i }
192+
ModelInstanceMethodCall() { this = cls.getAnInstanceReference().getAMethodCall(_) }
180193

181194
/** Gets the model instance for this call. */
182-
ModelInstance getInstance() { result = i }
195+
deprecated ModelInstance getInstance() { result = this.getReceiver() }
183196

184197
/** Gets the model class for this call. */
185-
ModelClass getModelClass() { result = i.getModelClass() }
198+
ModelClassNode getModelClass() { result = cls }
186199
}
187200

188201
/**
189-
* A collection of ActiveResource model objects.
202+
* DEPRECATED. Use `CollectionSource` instead.
203+
*
204+
* A data flow node that may refer to a collection of ActiveResource model objects.
190205
*/
191-
class Collection extends DataFlow::Node {
192-
ModelClassMethodCall classMethodCall;
193-
194-
Collection() {
195-
classMethodCall.flowsTo(this) and
196-
(
197-
classMethodCall.getMethodName() = "all"
198-
or
199-
classMethodCall.getMethodName() = "find" and
200-
classMethodCall.getArgument(0).asExpr().getConstantValue().isStringlikeValue("all")
201-
)
202-
}
206+
deprecated class Collection extends DataFlow::Node {
207+
Collection() { this = any(CollectionSource src).track().getAValueReachableFromSource() }
208+
}
203209

204-
/** Gets the model class for this collection. */
205-
ModelClass getModelClass() { result = classMethodCall.getModelClass() }
210+
/**
211+
* A call that returns a collection of ActiveResource model objects.
212+
*/
213+
class CollectionSource extends ModelClassMethodCall {
214+
CollectionSource() {
215+
this.getMethodName() = "all"
216+
or
217+
this.getArgument(0).asExpr().getConstantValue().isStringlikeValue("all")
218+
}
206219
}
207220

208221
/**
209222
* A method call on a collection.
210223
*/
211224
class CollectionCall extends DataFlow::CallNode {
212-
CollectionCall() { this.getReceiver() instanceof Collection }
225+
private CollectionSource collection;
226+
227+
CollectionCall() { this = collection.track().getAMethodCall(_) }
213228

214229
/** Gets the collection for this call. */
215-
Collection getCollection() { result = this.getReceiver() }
230+
CollectionSource getCollection() { result = collection }
216231
}
217232

218233
private class ModelClassMethodCallAsHttpRequest extends Http::Client::Request::Range,
219234
ModelClassMethodCall
220235
{
221-
ModelClass cls;
222-
223236
ModelClassMethodCallAsHttpRequest() {
224-
this.getModelClass() = cls and
225237
this.getMethodName() = ["all", "build", "create", "create!", "find", "first", "last"]
226238
}
227239

@@ -230,23 +242,22 @@ module ActiveResource {
230242
override predicate disablesCertificateValidation(
231243
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
232244
) {
233-
cls.disablesCertificateValidation(disablingNode) and
245+
this.getModelClass().disablesCertificateValidation(disablingNode) and
234246
// TODO: highlight real argument origin
235247
argumentOrigin = disablingNode
236248
}
237249

238-
override DataFlow::Node getAUrlPart() { result = cls.getASiteAssignment().getAUrlPart() }
250+
override DataFlow::Node getAUrlPart() {
251+
result = this.getModelClass().getASiteAssignment().getAUrlPart()
252+
}
239253

240254
override DataFlow::Node getResponseBody() { result = this }
241255
}
242256

243257
private class ModelInstanceMethodCallAsHttpRequest extends Http::Client::Request::Range,
244258
ModelInstanceMethodCall
245259
{
246-
ModelClass cls;
247-
248260
ModelInstanceMethodCallAsHttpRequest() {
249-
this.getModelClass() = cls and
250261
this.getMethodName() =
251262
[
252263
"exists?", "reload", "save", "save!", "destroy", "delete", "get", "patch", "post", "put",
@@ -259,42 +270,15 @@ module ActiveResource {
259270
override predicate disablesCertificateValidation(
260271
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
261272
) {
262-
cls.disablesCertificateValidation(disablingNode) and
273+
this.getModelClass().disablesCertificateValidation(disablingNode) and
263274
// TODO: highlight real argument origin
264275
argumentOrigin = disablingNode
265276
}
266277

267-
override DataFlow::Node getAUrlPart() { result = cls.getASiteAssignment().getAUrlPart() }
278+
override DataFlow::Node getAUrlPart() {
279+
result = this.getModelClass().getASiteAssignment().getAUrlPart()
280+
}
268281

269282
override DataFlow::Node getResponseBody() { result = this }
270283
}
271-
272-
/**
273-
* A call to a class method.
274-
*
275-
* TODO: is this general enough to be useful elsewhere?
276-
*
277-
* Examples:
278-
* ```rb
279-
* class A
280-
* def self.m; end
281-
*
282-
* m # call
283-
* end
284-
*
285-
* A.m # call
286-
* ```
287-
*/
288-
private DataFlow::CallNode classMethodCall(API::Node classNode, string methodName) {
289-
// A.m
290-
result = classNode.getAMethodCall(methodName)
291-
or
292-
// class A
293-
// A.m
294-
// end
295-
result.getReceiver().asExpr() instanceof ExprNodes::SelfVariableAccessCfgNode and
296-
result.asExpr().getExpr().getEnclosingModule().(ClassDeclaration).getSuperclassExpr() =
297-
classNode.getAValueReachableFromSource().asExpr().getExpr() and
298-
result.getMethodName() = methodName
299-
}
300284
}

‎ruby/ql/test/library-tests/frameworks/active_resource/ActiveResource.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ modelInstances
3333
| active_resource.rb:26:9:26:14 | people |
3434
| active_resource.rb:26:9:26:20 | call to first |
3535
| active_resource.rb:27:1:27:5 | alice |
36+
modelInstancesAsSource
37+
| active_resource.rb:1:1:3:3 | Person | active_resource.rb:5:9:5:33 | call to new |
38+
| active_resource.rb:1:1:3:3 | Person | active_resource.rb:8:9:8:22 | call to find |
39+
| active_resource.rb:1:1:3:3 | Person | active_resource.rb:16:1:16:23 | call to new |
40+
| active_resource.rb:1:1:3:3 | Person | active_resource.rb:18:1:18:22 | call to get |
41+
| active_resource.rb:1:1:3:3 | Person | active_resource.rb:24:10:24:26 | call to find |
42+
| active_resource.rb:1:1:3:3 | Person | active_resource.rb:26:9:26:20 | call to first |
3643
modelInstanceMethodCalls
3744
| active_resource.rb:6:1:6:10 | call to save |
3845
| active_resource.rb:9:1:9:13 | call to address= |
@@ -50,3 +57,6 @@ collections
5057
| active_resource.rb:24:1:24:26 | ... = ... |
5158
| active_resource.rb:24:10:24:26 | call to find |
5259
| active_resource.rb:26:9:26:14 | people |
60+
collectionSources
61+
| active_resource.rb:23:10:23:19 | call to all |
62+
| active_resource.rb:24:10:24:26 | call to find |

‎ruby/ql/test/library-tests/frameworks/active_resource/ActiveResource.ql‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import codeql.ruby.DataFlow
33
import codeql.ruby.frameworks.ActiveResource
44

55
query predicate modelClasses(
6-
ActiveResource::ModelClass c, DataFlow::Node siteAssignCall, boolean disablesCertificateValidation
6+
ActiveResource::ModelClassNode c, DataFlow::Node siteAssignCall,
7+
boolean disablesCertificateValidation
78
) {
89
c.getASiteAssignment() = siteAssignCall and
910
if c.disablesCertificateValidation(siteAssignCall)
@@ -13,8 +14,16 @@ query predicate modelClasses(
1314

1415
query predicate modelClassMethodCalls(ActiveResource::ModelClassMethodCall c) { any() }
1516

16-
query predicate modelInstances(ActiveResource::ModelInstance c) { any() }
17+
deprecated query predicate modelInstances(ActiveResource::ModelInstance c) { any() }
18+
19+
query predicate modelInstancesAsSource(
20+
ActiveResource::ModelClassNode cls, DataFlow::LocalSourceNode node
21+
) {
22+
node = cls.getAnInstanceReference().asSource()
23+
}
1724

1825
query predicate modelInstanceMethodCalls(ActiveResource::ModelInstanceMethodCall c) { any() }
1926

20-
query predicate collections(ActiveResource::Collection c) { any() }
27+
deprecated query predicate collections(ActiveResource::Collection c) { any() }
28+
29+
query predicate collectionSources(ActiveResource::CollectionSource c) { any() }

0 commit comments

Comments
 (0)