Skip to content

False positive-Spurious Java dataflow across unrelated generic instantiations #21066

@MarkLee131

Description

@MarkLee131

Description of the false positive

In a minimal Java example inspired by Retrofit and Jackson, CodeQL’s Java dataflow and taint tracking report flows from HTTP response deserialization into HTTP request construction, even though these flows pass through unrelated generic type parameters declared at different sites and occupying different semantic roles.

Concretely, values returned by:

JacksonResponseBodyConverter<T>.convert(ResponseBody)   // response -> T

are reported as flowing into request construction code based on:

ParameterHandler<T> with Converter<T, String / RequestBody>  // T -> request

Although both use a type parameter named T, these parameters are declared at different generic declaration sites and appear in different type argument positions, They represent opposite protocol directions (response-side vs request-side).

Environment

CodeQL CLI: codeql v2.23.7

Packs: codeql/[email protected]

Minimal Repro Setup

RetrofitJacksonExample.java

and use this ql file to test:

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

private module RetrofitFlowConfig implements DataFlow::ConfigSig {
  /**
   * Source: The return expression of RetrofitJacksonExample.JacksonResponseBodyConverter.convert.
   */
  predicate isSource(DataFlow::Node src) {
    exists(Method m, ReturnStmt ret, Expr e |
      // Simple class name of the nested class JacksonResponseBodyConverter
      m.getDeclaringType().getName() = "JacksonResponseBodyConverter" and
      m.getName() = "convert" and
      ret.getEnclosingCallable() = m and
      e = ret.getResult() and
      src.asExpr() = e
    )
  }

  /**
   * Sink 1: The string value of HTTP header passed into Header<T>.apply.
   */
  predicate isSink(DataFlow::Node sink) {
    exists(Method m, MethodCall ma |
      // Simple class name of the nested class RequestBuilder
      m.getDeclaringType().getName() = "RequestBuilder" and
      m.getName() = "addHeader" and
      ma.getMethod() = m and
      // The second argument is the value of the header
      sink.asExpr() = ma.getArgument(1)
    )
    or
    // Sink 2: The RequestBody passed into setBody in Body<T>.apply.
    exists(Method m2, MethodCall ma2 |
      m2.getDeclaringType().getName() = "RequestBuilder" and
      m2.getName() = "setBody" and
      ma2.getMethod() = m2 and
      sink.asExpr() = ma2.getArgument(0)
    )
  }
}

private module RetrofitFlow = TaintTracking::Global<RetrofitFlowConfig>;

from DataFlow::Node src, DataFlow::Node sink
where RetrofitFlow::flow(src, sink)
select sink,
  "Flow from JacksonResponseBodyConverter.convert(ResponseBody) to HTTP request construction."

Run the query via the following command:

codeql database create db-generics-test \
  --language=java \
  --source-root=. \
  --command='javac RetrofitJacksonExample.java'

codeql query run RetrofitJacksonExample_TaintTest.ql \
  --database=db-generics-test \
  --search-path="$HOME/.codeql/packages:."

Actual Result

The query reports flows from:

JacksonResponseBodyConverter.convert(ResponseBody)

to the following sinks:

ParameterHandler.Header<T>.apply (header construction)
ParameterHandler.Body<T>.apply (request body construction)
Image

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions