[C/C++] Is it possible to set the left-hand side of an assignment operation as the sink node in CodeQL's data flow analysis?

[Roarcannotprogramming]

Example:

struct Foo {
  int x;
  int y;
  char z[];
};

void func_bar() {
  int len;
  struct Foo* sink;

  len = source;  // just assume it is source. XD                                             [1]
  sink = some_malloc(len);  // sink is affected by the argument `len`                        [2]
  sink->y = 1;  // some field of sink is modified.                                           [3]
}

The dataflow goes from source in line [1] to len in line [2]. However, since we never use sink (or dereference of sink) as right-hand value of an assignment operation (i.e. bar = sink->x), there is no dataflow to sink->y in line [3].

Given this example, if source is fully controllable, then sink->y itself could be a potential overflow access because source could be 0.

There are many such examples in the Linux kernel. A value is assigned to a field in a function, and this field is not used again before returning to user space, resulting in no corresponding data flow record.

So I wonder if there is a way to track the dataflow to sink->y?

[MathiasVP]

Hi Roarcannotprogramming,

So you're successfully tracking flow from source into the argument of some_malloc, but then you want to track where the return value of some_malloc flows to. And you would like to track this all the way to the left-hand side of the assignment. Is that correct?

Indeed, the problem is that we don't allow flow from sink to sink->y without having previously seen a write to y. You can add such a taint-step yourself by doing something like:

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
  exists(FieldAccess fa, Field f |
    fa.getTarget() = f and
    f.hasName("y") and
    f.getDeclaringType().hasName("Foo") and
    node1.asIndirectExpr() = fa.getQualifier() and
    node2.asExpr() = fa
  )
}

Does that help?

[MathiasVP]

I should also point out that, based on this question, we've just merged #16063. Once that change has been released (or, if you're running bleeding edge) you can get the flow in your example using:

/**
 * @kind path-problem
 */

import cpp
import semmle.code.cpp.dataflow.new.TaintTracking
import semmle.code.cpp.ir.dataflow.FlowSteps
import Flow::PathGraph

/**
 * A class that specifies that taint flow from `foo` to `foo.y` when `foo` has
 * type `Foo`, or `foo` to `foo->y` when `foo` has type `Foo*`.
 */
class FooTaintInheritingContent extends TaintInheritingContent, DataFlow::FieldContent {
  FooTaintInheritingContent() {
    exists(Field f |
      this.getField() = f and
      f.hasName("y") and
      f.getDeclaringType().hasName("Foo")
    )
  }
}

module Config implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node node) { node.asExpr().(VariableAccess).getTarget().hasName("source") }

  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
    // Step from `len` to `*some_malloc(len)`.
    exists(Call call |
      call.getTarget().hasName("some_malloc") and
      node1.asExpr() = call.getAnArgument() and
      node2.asIndirectExpr() = call
    )
  }

  predicate isSink(DataFlow::Node node) { node.asIndirectExpr() = any(AssignExpr ae).getLValue() }
}

module Flow = TaintTracking::Global<Config>;

from Flow::PathNode source, Flow::PathNode sink
where Flow::flowPath(source, sink)
select sink.getNode(), source, sink, ""

Note that, I changed your example so that the source is a call to a function called source. That shouldn't make a difference on the rest of the example, though.

[Roarcannotprogramming]

I'm sorry for being tied up with other things today and unable to test. I will conduct a detailed test tomorrow.
It seems that I have always had some misunderstandings about the data flow analysis pattern of CodeQL (after all, many default propagation rules are guessed from the Path in the alert results. XD).
Thank you very much for spending time on this issue; I think this has given me a new understanding of CodeQL.