Skip to content

Commit 3e97fc9

Browse files
Improve localTaint predicate performance in STLContainer.
Previous implementation was using nomagic to prevent bad joins from being introduced in localTaint. However, this forces the evaluator to construct the entire graph of local taint paths, which is about 25 million tuples on pandas. This blocks updating to new dataflow where the performance issues get worse. The evaluator creates `additionalTaintStep+`, a relation with 6.7 billion rows. This is done via a higher-order-predicate and is surprisingly fast (8sec). However, the next step takes the `+`-style transitive closure and tries to make the `*` version, by unioning the `+` version with a scan of all dataflow nodes n into tuples (n, n)`. I don't know why this union OOMs, to be honest, but the crash stack trace includes an "unsorted relation writer," which maybe indicates that the OOM comes from trying to sort the 6.7 billion values. In any case, I'm satisfied that we shouldn't be constructing a 6.7 billion relation here. Fixed by identifying the root candidate set of expressions that come from container function calls that we're interested in, which is 799 nodes on pandas, and then the transitive relation `containerTaint` is just 1765 rows on old dataflow. On new dataflow this is 701 sources and 5779 nodes in the transitive closure.
1 parent aaac22d commit 3e97fc9

1 file changed

Lines changed: 26 additions & 16 deletions

File tree

cpp/common/src/codingstandards/cpp/standardlibrary/STLContainers.qll

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,30 @@ abstract class ContainerAccess extends VariableAccess {
312312
abstract Variable getOwningContainer();
313313
}
314314

315-
pragma[noinline, nomagic]
316-
private predicate localTaint(DataFlow::Node n1, DataFlow::Node n2) {
317-
TaintTracking::localTaint(n1, n2)
315+
pragma[nomagic]
316+
private predicate containerTaintSource(FunctionCall fc, DataFlow::Node n1) {
317+
n1 = DataFlow::exprNode(fc) and
318+
exists(STLContainer c |
319+
fc = c.getACallToAFunction() and
320+
// There are a few cases where the value is tainted
321+
// but no actual link to the underlying container is established.
322+
// For example, calling Vector<int>.size() returns an int but the
323+
// resulting variable doesn't depend on the underlying container
324+
// anymore.
325+
(
326+
fc.getTarget().getType() instanceof ReferenceType or
327+
fc.getTarget().getType() instanceof PointerType or
328+
fc.getTarget().getType() instanceof IteratorType
329+
)
330+
)
331+
}
332+
333+
pragma[nomagic]
334+
private predicate containerTaint(FunctionCall fc, DataFlow::Node n2) {
335+
exists(DataFlow::Node n1 |
336+
containerTaintSource(fc, n1) and
337+
TaintTracking::localTaint(n1, n2)
338+
)
318339
}
319340

320341
/**
@@ -326,19 +347,8 @@ class ContainerPointerOrReferenceAccess extends ContainerAccess {
326347
Variable owningContainer;
327348

328349
ContainerPointerOrReferenceAccess() {
329-
exists(STLContainer c, FunctionCall fc |
330-
fc = c.getACallToAFunction() and
331-
// There are a few cases where the value is tainted
332-
// but no actual link to the underlying container is established.
333-
// For example, calling Vector<int>.size() returns an int but the
334-
// resulting variable doesn't depend on the underlying container
335-
// anymore.
336-
(
337-
fc.getTarget().getType() instanceof ReferenceType or
338-
fc.getTarget().getType() instanceof PointerType or
339-
fc.getTarget().getType() instanceof IteratorType
340-
) and
341-
localTaint(DataFlow::exprNode(fc), DataFlow::exprNode(this)) and
350+
exists(FunctionCall fc |
351+
containerTaint(fc, DataFlow::exprNode(this)) and
342352
(getUnderlyingType() instanceof ReferenceType or getUnderlyingType() instanceof PointerType) and
343353
fc.getQualifier().(VariableAccess).getTarget() = owningContainer and
344354
// Exclude cases where we see taint into the owning container

0 commit comments

Comments
 (0)