Skip to content

Commit 60527df

Browse files
committed
Python: Fix py/meta/alerts/remote-flow-sources-reach
1 parent d7be27a commit 60527df

File tree

1 file changed

+8
-21
lines changed

1 file changed

+8
-21
lines changed

python/ql/src/meta/alerts/RemoteFlowSourcesReach.ql

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,28 +25,15 @@ class RemoteFlowSourceReach extends TaintTracking::Configuration {
2525
}
2626

2727
override predicate isSink(DataFlow::Node node) {
28-
not node.getLocation().getFile() instanceof IgnoredFile and
29-
(
30-
node instanceof RemoteFlowSource
31-
or
32-
this.isAdditionalFlowStep(_, node)
33-
) and
34-
// In september 2021 we changed how we do taint-propagation for method calls (mostly
35-
// relating to modeled frameworks/libraries). We used to do `obj -> obj.meth` and
36-
// `obj.meth -> obj.meth()` in two separate steps, and now do them in one
37-
// `obj -> obj.meth()`. To be able to compare the overall reach between these two
38-
// version, we don't want this query to alert us to the fact that we no longer taint
39-
// the node in the middle (since that is just noise).
40-
// see https://github.com/github/codeql/pull/6349
28+
not node.getLocation().getFile() instanceof IgnoredFile
29+
// We could try to reduce the number of sinks in this configuration, by only
30+
// allowing something that is on one end of a localFlowStep, readStep or storeStep,
31+
// however, it's a brittle solution that requires us to remember to update this file
32+
// if/when adding something new to the data-flow library.
4133
//
42-
// We should be able to remove the following few lines of code once we don't care to
43-
// compare with the old (before September 2021) way of doing taint-propagation for
44-
// method calls.
45-
not exists(DataFlow::MethodCallNode c |
46-
node = c.getFunction() and
47-
this.isAdditionalFlowStep(c.getObject(), node) and
48-
this.isAdditionalFlowStep(node, c)
49-
)
34+
// From testing on a few projects, trying to reduce the number of nodes, we only
35+
// gain a reduction in the range of 40%, and while that's nice, it doesn't seem
36+
// worth it to me for a meta query.
5037
}
5138
}
5239

0 commit comments

Comments
 (0)