[C++] Adding marking field access as tainted when qualifier is tainted in additionalTaintStep results in duplicate paths

[gsingh93]

I want to add an additionalTaintStep to a TaintTracking::Configuration which will treat any accesses of fields or methods of a tainted object as tainted. For example, if Foo f is tainted, then f.bar would be tainted and f.bar.func() would also be tainted. I've implemented this in the following query:

/**
 * @kind path-problem
 */

import cpp
import semmle.code.cpp.dataflow.TaintTracking
import DataFlow::PathGraph

class Config extends TaintTracking::Configuration {
  Config() { this = "Config" }

  override predicate isSource(DataFlow::Node source) {
    // Any parameter of the function `test` is tainted
    source.asParameter().getFunction().hasName("test")
  }

  override predicate isSink(DataFlow::Node sink) {
    exists(FunctionCall c | c.getTarget().hasName("sink") | c.getAnArgument() = sink.asExpr())
  }

  override predicate isAdditionalTaintStep(DataFlow::Node a, DataFlow::Node b) {
    a.asExpr() = b.asExpr().(VariableAccess).getQualifier() or
    a.asExpr() = b.asExpr().(Call).getQualifier()
  }
}

from DataFlow::PathNode source, DataFlow::PathNode sink, Config config
where config.hasFlowPath(source, sink)
select sink, source, sink, "Found flow"

I tested that query on this C++ file:

void sink(int);

class Bar {
 public:
    int x;
    int func();
};

class Foo {
 public:
    int x;
    Bar bar;
    int func();
};

void test(Foo tainted) {
  // Should alert
  sink(tainted.x);

  // Should alert
  sink(tainted.func());

  // Should alert
  sink(tainted.bar.x);

  // Should alert
  sink(tainted.bar.func());
}

While I get four alerts in the results as expected, some of the alerts have multiple paths:

Also note that in the above image, the fourth flow should go tainted -> bar -> call to func(), but instead it goes tainted -> call to func().

Checking the results of the edges query shows this:

You can see that there are two edges from tainted to x, when there should only be one. There is also no edge from bar to call to func().

While this isn't a huge deal in this small example, it's making it difficult to triage results in a real codebase. I'm also guessing there might be some performance hit by considering all these extra paths. What's going on here and how can I fix it?

[MathiasVP]

Which CodeQL version are you using? On the most-recent version I'm only getting one path per sink:

While this isn't a huge deal in this small example, it's making it difficult to triage results in a real codebase. I'm also guessing there might be some performance hit by considering all these extra paths. What's going on here and how can I fix it?

I can totally sympathize with this. What you can do is reduce the number of paths CodeQL reports. In VSCode you can set the "Max Paths" configuration to 1 (I believe the default is 4):

Also note that in the above image, the fourth flow should go tainted -> bar -> call to func(), but instead it goes tainted -> call to func().

The paths are compressed before being shown (for performance reasons, and to avoid showing excessively long paths). If you want to see the full path, you can hack your way to it: Navigate to https://github.com/github/codeql/blob/main/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll#L226 and replace none() with any(). This will make the query render the full path, but it will be more costly to compute.

Note that, even if you're not seeing the full path, all the nodes are still available when you're defining your barriers. A good trick when defining a barrier is to replace none() with any() in https://github.com/github/codeql/blob/main/cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll#L226 and check that the node you're targeting in a barrier is actually part of the dataflow graph.