Skip to content

Commit bd11946

Browse files
committed
Ruby: support WithoutContent steps in restricted cases
fixup ContentFilter fixup basicWith(out)contentstep
1 parent 323abf4 commit bd11946

File tree

2 files changed

+162
-0
lines changed

2 files changed

+162
-0
lines changed

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ private module Cached {
1717
LoadStoreStep(TypeTrackerContent load, TypeTrackerContent store) {
1818
basicLoadStoreStep(_, _, load, store)
1919
} or
20+
WithContent(ContentFilter filter) { basicWithContentStep(_, _, filter) } or
21+
WithoutContent(ContentFilter filter) { basicWithoutContentStep(_, _, filter) } or
2022
JumpStep()
2123

2224
cached
@@ -64,6 +66,14 @@ private module Cached {
6466
or
6567
step = JumpStep() and
6668
result = MkTypeTracker(false, currentContents)
69+
or
70+
exists(ContentFilter filter | result = tt |
71+
step = WithContent(filter) and
72+
currentContents = filter.getAMatchingContent()
73+
or
74+
step = WithoutContent(filter) and
75+
not currentContents = filter.getAMatchingContent()
76+
)
6777
)
6878
or
6979
exists(TypeTrackerContent storeContents, boolean hasCall |
@@ -109,6 +119,14 @@ private module Cached {
109119
or
110120
step = JumpStep() and
111121
result = MkTypeBackTracker(false, content)
122+
or
123+
exists(ContentFilter filter | result = tbt |
124+
step = WithContent(filter) and
125+
content = filter.getAMatchingContent()
126+
or
127+
step = WithoutContent(filter) and
128+
not content = filter.getAMatchingContent()
129+
)
112130
)
113131
or
114132
exists(TypeTrackerContent loadContents, boolean hasReturn |
@@ -174,6 +192,14 @@ private module Cached {
174192
flowsToLoadStoreStep(nodeFrom, nodeTo, loadContent, storeContent) and
175193
summary = LoadStoreStep(loadContent, storeContent)
176194
)
195+
or
196+
exists(ContentFilter filter |
197+
basicWithContentStep(nodeFrom, nodeTo, filter) and
198+
summary = WithContent(filter)
199+
or
200+
basicWithoutContentStep(nodeFrom, nodeTo, filter) and
201+
summary = WithoutContent(filter)
202+
)
177203
}
178204

179205
cached

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

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,32 @@ class OptionalTypeTrackerContent extends DataFlowPrivate::TOptionalContentSet {
3434
}
3535
}
3636

37+
private newtype TContentFilter =
38+
MkElementFilter() or
39+
MkPairValueFilter()
40+
41+
/**
42+
* A label to use for `WithContent` and `WithoutContent` steps, restricting
43+
* which `ContentSet` may pass through.
44+
*/
45+
class ContentFilter extends TContentFilter {
46+
/** Gets a string representation of this content filter. */
47+
string toString() {
48+
this = MkElementFilter() and result = "elements"
49+
or
50+
this = MkPairValueFilter() and result = "pair value"
51+
}
52+
53+
/** Gets the content of a type-tracker that matches this filter. */
54+
TypeTrackerContent getAMatchingContent() {
55+
this = MkElementFilter() and
56+
result.getAReadContent() instanceof DataFlow::Content::ElementContent
57+
or
58+
this = MkPairValueFilter() and
59+
result.getAReadContent() instanceof DataFlow::Content::PairValueContent
60+
}
61+
}
62+
3763
/**
3864
* Holds if a value stored with `storeContents` can be read back with `loadContents`.
3965
*/
@@ -254,6 +280,38 @@ predicate basicLoadStoreStep(
254280
)
255281
}
256282

283+
/**
284+
* Holds if type-tracking should step from `nodeFrom` to `nodeTo` but block flow of contents matched by `filter` through here.
285+
*/
286+
predicate basicWithoutContentStep(Node nodeFrom, Node nodeTo, ContentFilter filter) {
287+
exists(
288+
SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
289+
SummaryComponentStack output
290+
|
291+
hasWithoutContentSummary(callable, filter, pragma[only_bind_into](input),
292+
pragma[only_bind_into](output)) and
293+
call.asExpr().getExpr() = callable.getACallSimple() and
294+
nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
295+
nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
296+
)
297+
}
298+
299+
/**
300+
* Holds if type-tracking should step from `nodeFrom` to `nodeTo` if inside a content matched by `filter`.
301+
*/
302+
predicate basicWithContentStep(Node nodeFrom, Node nodeTo, ContentFilter filter) {
303+
exists(
304+
SummarizedCallable callable, DataFlowPublic::CallNode call, SummaryComponentStack input,
305+
SummaryComponentStack output
306+
|
307+
hasWithContentSummary(callable, filter, pragma[only_bind_into](input),
308+
pragma[only_bind_into](output)) and
309+
call.asExpr().getExpr() = callable.getACallSimple() and
310+
nodeFrom = evaluateSummaryComponentStackLocal(callable, call, input) and
311+
nodeTo = evaluateSummaryComponentStackLocal(callable, call, output)
312+
)
313+
}
314+
257315
/**
258316
* A utility class that is equivalent to `boolean` but does not require type joining.
259317
*/
@@ -289,6 +347,78 @@ private predicate hasLoadStoreSummary(
289347
push(SummaryComponent::content(storeContents), output), true)
290348
}
291349

350+
/**
351+
* Gets a content filter to use for a `WithoutContent[content]` step, or has no result if
352+
* the step should be treated as ordinary flow.
353+
*
354+
* `WithoutContent` is often used to perform strong updates on individual collection elements, but for
355+
* type-tracking this is rarely beneficial and quite expensive. However, `WithoutContent` can be quite useful
356+
* for restricting the type of an object, and in these cases we translate it to a filter.
357+
*/
358+
private ContentFilter getFilterFromWithoutContentStep(DataFlow::ContentSet content) {
359+
(
360+
content.isAnyElement()
361+
or
362+
content.isElementLowerBound(_)
363+
or
364+
content.isElementLowerBoundOrUnknown(_)
365+
or
366+
content.isSingleton(any(DataFlow::Content::UnknownElementContent c))
367+
) and
368+
result = MkElementFilter()
369+
or
370+
content.isSingleton(any(DataFlow::Content::UnknownPairValueContent c)) and
371+
result = MkPairValueFilter()
372+
}
373+
374+
pragma[nomagic]
375+
private predicate hasWithoutContentSummary(
376+
SummarizedCallable callable, ContentFilter filter, SummaryComponentStack input,
377+
SummaryComponentStack output
378+
) {
379+
exists(DataFlow::ContentSet content |
380+
callable.propagatesFlow(push(SummaryComponent::withoutContent(content), input), output, true) and
381+
filter = getFilterFromWithoutContentStep(content) and
382+
input != output
383+
)
384+
}
385+
386+
/**
387+
* Gets a content filter to use for a `WithoutContent[content]` step, or has no result if
388+
* the step should be treated as ordinary flow.
389+
*
390+
* `WithoutContent` is often used to perform strong updates on individual collection elements, but for
391+
* type-tracking this is rarely beneficial and quite expensive. However, `WithoutContent` can be quite useful
392+
* for restricting the type of an object, and in these cases we translate it to a filter.
393+
*/
394+
private ContentFilter getFilterFromWithContentStep(DataFlow::ContentSet content) {
395+
(
396+
content.isAnyElement()
397+
or
398+
content.isElementLowerBound(_)
399+
or
400+
content.isElementLowerBoundOrUnknown(_)
401+
or
402+
content.isSingleton(any(DataFlow::Content::ElementContent c))
403+
) and
404+
result = MkElementFilter()
405+
or
406+
content.isSingleton(any(DataFlow::Content::PairValueContent c)) and
407+
result = MkPairValueFilter()
408+
}
409+
410+
pragma[nomagic]
411+
private predicate hasWithContentSummary(
412+
SummarizedCallable callable, ContentFilter filter, SummaryComponentStack input,
413+
SummaryComponentStack output
414+
) {
415+
exists(DataFlow::ContentSet content |
416+
callable.propagatesFlow(push(SummaryComponent::withContent(content), input), output, true) and
417+
filter = getFilterFromWithContentStep(content) and
418+
input != output
419+
)
420+
}
421+
292422
/**
293423
* Gets a data flow node corresponding an argument or return value of `call`,
294424
* as specified by `component`.
@@ -367,5 +497,11 @@ private DataFlow::Node evaluateSummaryComponentStackLocal(
367497
or
368498
head = SummaryComponent::return() and
369499
result.(DataFlowPrivate::SynthReturnNode).getCfgScope() = prev.asExpr().getExpr()
500+
or
501+
exists(DataFlow::ContentSet content |
502+
head = SummaryComponent::withoutContent(content) and
503+
not exists(getFilterFromWithoutContentStep(content)) and
504+
result = prev
505+
)
370506
)
371507
}

0 commit comments

Comments
 (0)