Skip to content

Commit 497258e

Browse files
committed
Ruby: reuse Content type
1 parent ac1b7eb commit 497258e

File tree

4 files changed

+55
-34
lines changed

4 files changed

+55
-34
lines changed

ruby/ql/lib/codeql/ruby/ApiGraphs.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ module API {
525525
or
526526
exists(TypeTrackerSpecific::TypeTrackerContent c |
527527
TypeTrackerSpecific::basicLoadStep(node, ref, c) and
528-
lbl = Label::content(c.asContent())
528+
lbl = Label::content(c)
529529
)
530530
// note: method calls are not handled here as there is no DataFlow::Node for the intermediate MkMethodAccessNode API node
531531
}
@@ -537,7 +537,7 @@ module API {
537537
private predicate defStep(Label::ApiLabel lbl, DataFlow::Node node, DataFlow::Node rhs) {
538538
exists(TypeTrackerSpecific::TypeTrackerContent c |
539539
TypeTrackerSpecific::basicStoreStep(rhs, node, c) and
540-
lbl = Label::content(c.asContent())
540+
lbl = Label::content(c)
541541
)
542542
}
543543

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import codeql.ruby.AST
22
private import codeql.ruby.ast.internal.Synthesis
33
private import codeql.ruby.CFG
4+
private import codeql.ruby.AST as AST
45
private import codeql.ruby.dataflow.SSA
56
private import DataFlowPublic
67
private import DataFlowDispatch
@@ -385,7 +386,8 @@ private module Cached {
385386
}
386387

387388
cached
388-
newtype TContent =
389+
newtype TOptionalContent =
390+
TNoContent() or
389391
TKnownElementContent(ConstantValue cv) {
390392
not cv.isInt(_) or
391393
cv.getInt() in [0 .. 10]
@@ -408,7 +410,14 @@ private module Cached {
408410
|
409411
name = [input, output].regexpFind("(?<=(^|\\.)Field\\[)[^\\]]+(?=\\])", _, _).trim()
410412
)
411-
}
413+
} or
414+
// Only used by type-tracking
415+
TAttributeName(string name) { name = any(AST::SetterMethodCall c).getTargetName() }
416+
417+
cached
418+
class TContent =
419+
TKnownElementContent or TUnknownElementContent or TKnownPairValueContent or
420+
TUnknownPairValueContent or TFieldContent or TAttributeName;
412421

413422
/**
414423
* Holds if `e` is an `ExprNode` that may be returned by a call to `c`.

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,15 +235,18 @@ predicate localExprFlow(CfgNodes::ExprCfgNode e1, CfgNodes::ExprCfgNode e2) {
235235
localFlow(exprNode(e1), exprNode(e2))
236236
}
237237

238-
/** A reference contained in an object. */
239-
class Content extends TContent {
238+
/** A reference contained in an object, or the `noContent()` value. */
239+
class OptionalContent extends TOptionalContent {
240240
/** Gets a textual representation of this content. */
241241
string toString() { none() }
242242

243243
/** Gets the location of this content. */
244244
Location getLocation() { none() }
245245
}
246246

247+
/** A reference contained in an object. */
248+
class Content extends OptionalContent, TContent { }
249+
247250
/** Provides different sub classes of `Content`. */
248251
module Content {
249252
/** An element in a collection, for example an element in an array or in a hash. */
@@ -314,6 +317,34 @@ module Content {
314317
class UnknownPairValueContent extends PairValueContent, TUnknownPairValueContent {
315318
override string toString() { result = "pair" }
316319
}
320+
321+
/**
322+
* A value stored behind a getter/setter pair.
323+
*
324+
* This is used (only) by type-tracking, as a heuristic since getter/setter pairs tend to operate
325+
* on similar types of objects (i.e. the type flowing into a setter will likely flow out of the getter).
326+
*/
327+
class AttributeNameContent extends Content, TAttributeName {
328+
private string name;
329+
330+
AttributeNameContent() { this = TAttributeName(name) }
331+
332+
override string toString() { result = "attribute " + name }
333+
334+
/** Gets the attribute name. */
335+
string getName() { result = name }
336+
}
337+
338+
/** Gets `AttributeNameContent` of the given name. */
339+
AttributeNameContent getAttributeName(string name) { result.getName() = name }
340+
341+
/** A value representing no content. */
342+
class NoContent extends OptionalContent, TNoContent {
343+
override string toString() { result = "noContent()" }
344+
}
345+
346+
/** Gets the `noContent()` value. */
347+
NoContent noContent() { any() }
317348
}
318349

319350
/**

ruby/ql/lib/codeql/ruby/typetracking/TypeTrackerSpecific.qll

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,31 +20,9 @@ private newtype TOptionalTypeTrackerContent =
2020
MkContent(DataFlowPublic::Content content) or
2121
MkNoContent()
2222

23-
/** A content for use by type trackers, or the empty content `noContent()` */
24-
class OptionalTypeTrackerContent extends TOptionalTypeTrackerContent {
25-
/** Gets a textual representation of this content. */
26-
string toString() {
27-
result = "attribute " + this.asAttributeName()
28-
or
29-
result = this.asContent().toString()
30-
or
31-
this instanceof MkNoContent and result = "no content"
32-
}
33-
34-
/** Gets the attribute name represented by this content, if any. */
35-
string asAttributeName() { this = MkAttribute(result) }
36-
37-
/** Gets the data flow content by this type-tracker content, if any. */
38-
DataFlowPublic::Content asContent() { this = MkContent(result) }
39-
}
40-
41-
/** Gets the value representing no content, that is, the empty access path. */
42-
OptionalTypeTrackerContent noContent() { result = MkNoContent() }
43-
44-
private class TTypeTrackerContent = MkAttribute or MkContent;
23+
class TypeTrackerContent = DataFlowPublic::Content;
4524

46-
/** A content for use by type trackers. */
47-
class TypeTrackerContent extends OptionalTypeTrackerContent, TTypeTrackerContent { }
25+
predicate noContent = DataFlowPublic::Content::noContent/0;
4826

4927
/** Holds if there is a simple local flow step from `nodeFrom` to `nodeTo` */
5028
predicate simpleLocalFlowStep = DataFlowPrivate::localFlowStepTypeTracker/2;
@@ -185,7 +163,7 @@ predicate basicStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content)
185163
SummaryComponentStack::singleton(output))) and
186164
nodeFrom = evaluateSummaryComponentLocal(call, input) and
187165
nodeTo = evaluateSummaryComponentLocal(call, output) and
188-
content.asContent() = contents.getAStoreContent()
166+
content = contents.getAStoreContent()
189167
)
190168
}
191169

@@ -196,7 +174,10 @@ predicate basicStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content)
196174
predicate postUpdateStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content) {
197175
// TODO: support SetterMethodCall inside TuplePattern
198176
exists(ExprNodes::MethodCallCfgNode call |
199-
content = MkAttribute(call.getExpr().(Ast::SetterMethodCall).getTargetName()) and
177+
content =
178+
DataFlowPublic::Content::getAttributeName(call.getExpr()
179+
.(Ast::SetterMethodCall)
180+
.getTargetName()) and
200181
nodeTo.(DataFlowPublic::PostUpdateNode).getPreUpdateNode().asExpr() = call.getReceiver() and
201182
call.getExpr() instanceof Ast::SetterMethodCall and
202183
call.getArgument(call.getNumberOfArguments() - 1) =
@@ -210,7 +191,7 @@ predicate postUpdateStoreStep(Node nodeFrom, Node nodeTo, TypeTrackerContent con
210191
predicate basicLoadStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content) {
211192
exists(ExprNodes::MethodCallCfgNode call |
212193
call.getExpr().getNumberOfArguments() = 0 and
213-
content = MkAttribute(call.getExpr().getMethodName()) and
194+
content = DataFlowPublic::Content::getAttributeName(call.getExpr().getMethodName()) and
214195
nodeFrom.asExpr() = call.getReceiver() and
215196
nodeTo.asExpr() = call
216197
)
@@ -225,7 +206,7 @@ predicate basicLoadStep(Node nodeFrom, Node nodeTo, TypeTrackerContent content)
225206
SummaryComponentStack::singleton(output)) and
226207
nodeFrom = evaluateSummaryComponentLocal(call, input) and
227208
nodeTo = evaluateSummaryComponentLocal(call, output) and
228-
content.asContent() = contents.getAReadContent()
209+
content = contents.getAReadContent()
229210
)
230211
}
231212

0 commit comments

Comments
 (0)