LGTM.com - Java - Lost type information leads to incomplete path and false positive

[JLLeitschuh]

Description of the false-positive

It looks like the query language is connecting sources and sinks through a type checking system that is interpreting methods like Object::toString when called on Object when at runtime that type will be String to be the same as MyCustomType::toString.

URL to the alert on the project page on LGTM.com
https://lgtm.com/projects/g/spring-projects/spring-framework/snapshot/a21fcec7555409e8859aba85fb2fe95d53760f80/files/spring-web/src/main/java/org/springframework/web/context/support/HttpRequestHandlerServlet.java?sort=name&dir=ASC&mode=heatmap#xe877feaabe7d1240:1

Analysis

QL Correctly identifies a source of user-supplied data and correctly tracks it to a place where it is used in the toString method.

Now we have some arbitrary location in the codebase where toString is called on Object.

Now we see where those code paths allow this toString value to be returned up the stack.

Now we get to the sink location. However, we see that StringUtils.arrayToDelimitedString is being passed a String[] not a ServletWebRequest that would mean this was indeed a vulnerability.

TL;DR: Looks like a type confusion issue? Or perhaps there's no logic for detecting the true type that toString will be called on.

[aschackmull]

Thank you very much for this very detailed report. The short answer is that we indeed currently are missing the type of logic needed to rule out this FP.
The long answer is that virtual dispatch is a hard problem and that we do a lot of type flow analysis to improve it, and since Object.toString() in particular has a known tendency to induce FPs like this, we even have a special purpose analysis to improve the virtual dispatch of those calls. On top of this we use a one-level call context to improve dispatch further in the context of data flow, but this is of course insufficient here. One aspect that makes pruning FPs like this hard, is that we cannot perform the check on the path as we see it, since we're just shown a few representative paths among possibly exponentially many possible paths in the data-flow graph. All this said, this particular case has given me some ideas on general pruning improvements that we could potentially make in the data-flow library - this is something that I'll likely look further into in the hopefully not too distant future.