Break dataflow on EnhancedForStmt of Java

[LFYSec]

Hey, I want to ask, when I use TaintTracking, why there is no data flow from the roleIds parameter to testaaa(roleId)? for (Long roleId : roleIds) seems to cause the data flow to break

public int test(Long[] roleIds) {
    for (Long roleId : roleIds) {
        if (testaa(roleId) > 0) {
            return;
        }
    }
}

[MathiasVP]

Hi @LFYSec,

Thank you for raising this issue! Can you show me the TaintTracking::Configuration (or even better: paste the entire query) you're using?

[smowton]

@LFYSec I looked into this today, and found that this isn't down to EnhancedForStmt specifically, but rather boxed primitive types by default not using taint tracking for array reads.

For most types, if T[] t is tainted, then t[e] will inherit that taint. However, if T is a primitive, boxed primitive or java.lang.Number then this behaviour is disabled by default, likely because excessive false positives or excessive cost of analysis was observed in the past.

The implementation of this behaviour can be seen at https://github.com/github/codeql/blob/main/java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll#L127

If you're writing a custom query, you can opt back into array taint propagation for all types. For example, with your example code slightly adapted, this works for me:

import java

import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.dataflow.internal.ContainerFlow

class Cfg extends TaintTracking::Configuration {

  Cfg() { this = "testConfig" }

  override predicate isSource(DataFlow::Node n) {
    n.asParameter().getName() = "roleIds"
  }

  override predicate isSink(DataFlow::Node n) {
    n.asExpr().(Argument).getCall().getCallee().getName() = "testaa"
  }

  override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
    arrayReadStep(fromNode, toNode, _)
  }

}

from Cfg c, DataFlow::PathNode src, DataFlow::PathNode snk
where c.hasFlowPath(src, snk)
select src, snk

[bitbounty85]

great