[RESOLVED] How do I match the contructor?

[CaledoniaProject]

I'm still learning codeql by creating basic queries

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

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

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

  override predicate isSource(DataFlow::Node source) {
    exists(MethodAccess call, Method method |
        method.hasName("getParameter")
        and method.getDeclaringType().getAnAncestor().hasQualifiedName("javax.servlet", "ServletRequest")
        and call.getMethod() = method
        // and source.asParameter() = method.getParameter(0)
    )
  }

  override predicate isSink(DataFlow::Node sink) {
    exists(MethodAccess call, Method method |
        sink.asExpr() = call.getAnArgument() and
        call.getMethod() = method and
        (
            (
                method.hasName("exec") and
                method.getDeclaringType().hasQualifiedName("java.lang", "Runtime")
            )
            or
            (
                method.hasName("ProcessBuilder") and
                method.getDeclaringType().hasQualifiedName("java.lang", "ProcessBuilder")
            )
        )
    )
  }
}

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"

The query above works for this one

protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        try {
            String cmd = req.getParameter("cmd");
            if (cmd != null) {
                Runtime.getRuntime().exec(cmd);
            }
        } catch (IOException e) {
            resp.getWriter().println(e);
        }
    }

But not ProcessBuilder constructor:

protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
        String cmd = req.getParameter("cmd");
        ProcessBuilder builder = new ProcessBuilder("cmd", "/c", cmd);
}

What was wrong?

[hvitved]

I think this is because new ProcessBuilder("cmd", "/c", cmd) is not a MethodAccess, but instead a ConstructorCall. What if you generalize isSink to

  override predicate isSink(DataFlow::Node sink) {
    exists(Call call, Callable callable |
        sink.asExpr() = call.getAnArgument() and
        call.getCallee() = callable and
        (
            (
                callable.hasName("exec") and
                callable.getDeclaringType().hasQualifiedName("java.lang", "Runtime")
            )
            or
            (
                callable.hasName("ProcessBuilder") and
                callable.getDeclaringType().hasQualifiedName("java.lang", "ProcessBuilder")
            )
        )
    )
  }

?

[CaledoniaProject]

It worked.

1. Is there any situation that I should use MethodAccess instead of Callable? Looks like Callable can completely replace MethodAccess.
2. The query above results in duplicated results, what was missing?

[hvitved]

1. If you only want to consider method calls/methods, then use MethodAccess/Method instead of Call/Callable.
2. This can happen if there are multiple paths from a given source to a given sink.

[CaledoniaProject]

I was wrong, there are no duplicated results. I have two calls of ProcessBuilder in the source code.

[CaledoniaProject]

I'm using

override predicate isSource(DataFlow::Node source) {
    exists(MethodAccess call, Method method |
        method.hasName("getParameter")
        and method.getDeclaringType().getAnAncestor().hasQualifiedName("javax.servlet", "ServletRequest")
        and call.getMethod() = method
    )
}

To match

String cmd = req.getParameter("cmd");

Because source parameter is unused, the query looks wrong to me. What do you suggest?

[hvitved]

source.asExp() = call?

[CaledoniaProject]

This works and stripped the isolated result.