[RESOLVED] Can't find command injection with this query

[CaledoniaProject]

I'm trying to match detect command injection in this code: bug.cpp.txt

I wrote the following query and quick evaluation in isSource and isSink works fine, but it yields no results:

/**
 * @name test
 * @kind path-problem
 */

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

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

  override predicate isSource(DataFlow::Node source) {
    exists(Parameter p, Function f |
        source.asParameter() = p
        and p.hasName("argv")
        and f.hasName("main")
    )
  }

  override predicate isSink(DataFlow::Node sink) {
    exists(FunctionCall fc |
      sink.asExpr() = fc and
      fc.getTarget().hasQualifiedName("system")
    )
  }
}

from DataFlow::PathNode source, DataFlow::PathNode sink, MyTaintConfig conf
where conf.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "cmdi from $@.", source.getNode(), "this user input"

What was wrong?

[MathiasVP]

Hi @CaledoniaProject,

Your isSource and isSink aren't totally right. Let's start with your isSource:

override predicate isSource(DataFlow::Node source) {
  exists(Parameter p, Function f |
      source.asParameter() = p
      and p.hasName("argv")
      and f.hasName("main")
  )
}

Here you're saying that:

1. Your source is a parameter called argv, and
2. That there exists a function called main.

Note that, in particular, you're not specifying any relationship between the function and the parameter. I'm guessing you want to say that p is a parameter of the function f. This can be done by changing your isSource to:

override predicate isSource(DataFlow::Node source) {
  exists(Parameter p, Function f |
    source.asParameter() = p and
    p.hasName("argv") and
    f.hasName("main") and
    p.getFunction() = f // <-- I added this line.
  )
}

Next, let's discuss your isSink:

override predicate isSink(DataFlow::Node sink) {
  exists(FunctionCall fc |
    sink.asExpr() = fc and
    fc.getTarget().hasQualifiedName("system")
  )
}

Two things to point out here:

1. This says that the return value of system is your sink. That's probably not what you want. Instead, you probably want to mark the argument to system as your sink.
2. You're using a deprecated single-argument version of hasQualifiedName. As the QLDoc mentioned, you should use hasGlobalName, hasGlobalOrStdName, or the 2- or 3-argument version of hasQualifiedName. For this case I recommend using hasGlobalOrStdName (so that you also automatically catch calls to std::system).

With those changes your isSink becomes:

override predicate isSink(DataFlow::Node sink) {
  exists(FunctionCall fc |
    sink.asExpr() = fc.getAnArgument() and
    fc.getTarget().hasGlobalOrStdName("system")
  )
}

and with those definitions of isSource and isSink you should be able to find the command injection.

I hope this helps! :)

[CaledoniaProject]

Thanks for pointing out, this worked for me.