Java: unreasonable flow

[flowingair]

PostUpdateNode or ThisAcess may flow to the wrong ClassInstanceExpr of the class.

Which may lead to wrong result and slow down the analyze.

How can i fix it?

for example this.data will flow to new RealSource(data);

package WrongPath;

public class RealSource {
    private String data;

    public RealSource(String data) {
        this.data = data;
    }

    public String getData() {
        return data;
    }
}

package WrongPath;

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;

public class FakeSource {
    public FakeSource() {
        String data = "";
        RealSource realSource = new RealSource(data);
        try {
            URL url = new URL(realSource.getData());
            URLConnection urlConnection = url.openConnection();
            urlConnection.connect();
        } catch (MalformedURLException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

[jketema]

Hi @flowingair. Thanks for your question. If this shows up in a query you're writing, would you be willing to share the query? This would make it easier for us to give concrete advise. If this shows up in an existing query, could you tell us which query?

[flowingair]

Hi @flowingair. Thanks for your question. If this shows up in a query you're writing, would you be willing to share the query? This would make it easier for us to give concrete advise. If this shows up in an existing query, could you tell us which query?

Here your are.
v

import java
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
import semmle.code.java.dataflow.ExternalFlow

private class AdditionSinkModels extends SinkModelCsv {
  override predicate row(string row) {
    row =
      [
        "java.net;URL;true;URL;;;Argument;more;manual",
        "java.net;URL;true;openConnection;;;Argument[-1];more;manual",
      ]
  }
}

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

  override predicate isSource(DataFlow::Node source) {
    exists(Constructor con |
      con.getNumberOfParameters() = 1 and
      "Ljava/lang/String;" = con.getParameterType(0).getTypeDescriptor() and
      con.getAParameter() = source.asParameter()
    )
  }

  override predicate isSink(DataFlow::Node sink) { sinkNode(sink, "more") }
}

from Template template, DataFlow::PathNode source, DataFlow::PathNode sink
where template.hasFlowPath(source, sink)
select source.getNode(), source, sink, "$@", sink.getNode().getLocation(), "location"

[aschackmull]

The flow path you're getting looks correct to me. Note that the source you have defined is inside the RealSource constructor - that means that it'll reach all calls to that constructor. If you had defined your source as something outside the constructor such that flow would depend on a path into and out of the constructor, then you would get the call-site matching that I presume you're asking for.
The data flow is call-sensitive, but if you're turning every constructor into a source then that's what you get, so you may wish to tweak your source definition.

[flowingair]

I can understand that it will be useful to trace the parmeter of constructor.
however in some situaction,for example, ql debug,i have to defined a source inside a flow.
is there any chance to block flows to construtor?maybe a new selfdefined source?

[jketema]

is there any chance to block flows to construtor?

In general, flow can be blocked by overriding isBarrier is your configuration.

[flowingair]

i had tried,but it wont work.can i have an example?please.

[jketema]

Can you show us what you tried?

[flowingair]

Can you show us what you tried?

deleted.
can you show me a example on how to block the flow(in this situation).

[aschackmull]

can you show me a example on how to block the flow(in this situation).

Add something like the following to your configuration (isBarrier is called isSanitizer on TaintTracking configurations):

override predicate isSanitizer(DataFlow::Node n) {
  n.asExpr().(ClassInstanceExpr).getConstructedType().hasQualifiedName("WrongPath", "RealSource")
}

[flowingair]

can you show me a example on how to block the flow(in this situation).

Add something like the following to your configuration (isBarrier is called isSanitizer on TaintTracking configurations):

override predicate isSanitizer(DataFlow::Node n) {
  n.asExpr().(ClassInstanceExpr).getConstructedType().hasQualifiedName("WrongPath", "RealSource")
}

but i dont have a list of constructors.some constructors may call the constructor of other class.
a isSanitizer may not be suitable?

[jketema]

a isSanitizer may not be suitable?

The options are isSource and isSanitizer. It's better to restrict isSource and not define it to be all constructors, but that's not a change you were willing to make.

[aschackmull]

but i dont have a list of constructors.some constructors may call the constructor of other class

This was just an example. You of course don't have to list constructor types explicitly, you have all of CodeQL at your disposal to describe the patterns that you want in however way you want to.

[flowingair]

ql for CVE-2022-44262(ff4j/ff4j#624)
i had wrote this for finding more useable constructors.
according to what we discuss above,its no perfect and cannot be perfect.
Sanitizer or Barrier wont help either.
which dirve me crazy.

import java
import semmle.code.java.dataflow.FlowSources
import DataFlow::PathGraph
import semmle.code.java.dataflow.ExternalFlow

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

  override predicate isSource(DataFlow::Node source) {
    exists(Constructor con |
      con.getNumberOfParameters() = 2 and
      "Ljava/lang/String;" = con.getParameterType(0).getTypeDescriptor() and
      "Ljava/lang/String;" = con.getParameterType(1).getTypeDescriptor() and
      con.getAParameter() = source.asParameter() and
      (
        source.getType() instanceof PrimitiveType or
        source.getType() instanceof TypeString
      )
    )
  }

  override predicate isSink(DataFlow::Node sink) { sinkNode(sink, _) }
}

from Template template, DataFlow::PathNode source, DataFlow::PathNode sink, Constructor con, Class c
where
  template.hasFlowPath(source, sink) and
  con.getAParameter() = source.getNode().asParameter() and
  c.getAConstructor() = con
select source.getNode(), source, sink, "$@", con, "con"