[Java] Taint Tracking for JNI Field Access and Object Aliasing in Java

[starsalt0124]

I'm developing a custom taint tracking rule to analyze native methods in Java, but I'm encountering issues when tainted data involves field access. CodeQL seems to treat fields and their containing objects as separate entities, which breaks taint propagation in certain scenarios.

Example Case

Consider this Java code where d1.data is tainted and d2 (an alias of d1) is passed as a sink:

public class Main {
    static {
        System.loadLibrary("java2nativefieldalias");
    }

    public native void nativeTransfer(Data d1, Data d2);

    public static void main(String[] args) {
        Data d1 = new Data();
        Main m = new Main();
        // Source
        d1.data = SourceSink.source();
        Data d2 = d1;
        // aliased arguments
        m.nativeTransfer(d1, d2);  // Should detect taint flow but doesn't
        Data d3 = new Data();
        d3.data = "clean data";
        // non-aliased arguments
        m.nativeTransfer(d1, d3);
    }
}

With the corresponding native implementation:

#include <stdio.h>
#include "Main.h"

void Leak(const char* str) {
    printf("sensitive data: %s\n", str);
}

JNIEXPORT void JNICALL Java_Main_nativeTransfer
  (JNIEnv *env, jobject obj, jobject data1, jobject data2) {
    jclass dataClass = (*env)->GetObjectClass(env, data2);
    jfieldID dataField = (*env)->GetFieldID(env, dataClass, "data", "Ljava/lang/String;");
    jstring dataString = (jstring)(*env)->GetObjectField(env, data2, dataField);
    const char* data = (*env)->GetStringUTFChars(env, dataString, NULL);
    if (data == NULL) return;
    Leak(data); // leak
    (*env)->ReleaseStringUTFChars(env, dataString, data);
}

Current Query and Workaround

My current taint tracking configuration includes additional flow steps to handle field propagation and aliasing:

predicate isSource(DataFlow::Node source) {
  // Source from SourceSink.source() method
  exists(MethodAccess ma |
    ma.getMethod().getName() = "source" and
    ma.getMethod().getDeclaringType().hasName("SourceSink") and
    source.asExpr() = ma
  )
}

predicate isSink(DataFlow::Node sink) {
  // Sink: second argument to nativeTransfer method
  exists(MethodAccess ma |
    ma.getMethod().getName() = "nativeTransfer" and
    ma.getMethod().isNative() and
    sink.asExpr() = ma.getArgument(1)  // d2 parameter
  )
}

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
    // Field-to-object propagation
    exists(FieldAccess fa |
      fa.getField().getName() = "data" and
      node1.asExpr() = fa and                // Field is tainted
      node2.asExpr() = fa.getQualifier()     // Object should also be tainted
    )
    or
    // Alias propagation handling
    exists(AssignExpr assign |
      assign.getRhs() = node1.asExpr() and
      assign.getDest() = node2.asExpr()
    )
}

Questions

1. JNI Taint Analysis Best Practices: Are there recommended approaches or built-in mechanisms in CodeQL for handling taint tracking through JNI methods, particularly when dealing with field access patterns like in the example above?

2. Alias Propagation Behavior: The second predicate (alias propagation) appears redundant since CodeQL should inherently handle aliasing. However, without it, the taint flow from d1 to d2 via assignment (Data d2 = d1;) isn't detected. Is this expected behavior, or might there be an issue with my configuration?

Expected vs Actual Behavior

Expected: CodeQL should detect the taint flow from d1.data → d1 → d2 → nativeTransfer(d2) → field access in native code → Leak().

Actual: Without the custom aliasing predicate, the taint chain breaks at the object level, even though the field data itself is properly tracked.

The workaround functions but seems suboptimal. I'm seeking guidance on whether this is the correct approach or if there are better solutions available in CodeQL's taint tracking framework.

[jketema]

Hi @starsalt0124,

Thanks for your question. I've asked someone who is familiar with our Java taint tracking implementation to have a look at the Java part of your question.

Expected: CodeQL should detect the taint flow from d1.data → d1 → d2 → nativeTransfer(d2) → field access in native code → Leak().

Note that this is never going to work, as CodeQL does not support analysis across languages.

[starsalt0124]

Hi @starsalt0124,

Thanks for your question. I've asked someone who is familiar with our Java taint tracking implementation to have a look at the Java part of your question.

Expected: CodeQL should detect the taint flow from → → → → field access in native code → .d1.data``d1``d2``nativeTransfer(d2)``Leak()

Note that this is never going to work, as CodeQL does not support analysis across languages.

Thanks for pointing that out. I was actually referring to the taint flow from the source up to the native method call in Java (before crossing into native code). The issue is specifically with the Java-side tracking where the alias relationship between d1 and d2 isn't being recognized without my custom predicate.

[aschackmull]

Let's first pick apart the reasons why your workaround works (because it's likely not what you expect). Firstly, flow through assignments are fully supported out of the box, so initially it seems weird that you need to include this. But then again the step definition itself also looks weird since the target node is the left-hand side, which isn't a read, but rather a write, and such LHS expressions aren't directly part of the flow graph. So what happens is that your two steps combine to yield flow from the right-hand side of d1.data = SourceSink.source() into d1, which is a read and therefore a proper part of the flow graph. On the other hand your step does nothing in the assignment d2 = d1.

But this is all a digression. What you really need is an answer to the following: How is a sink specified such that it accepts not just a tainted object, but also an object with a tainted field? This is done by adding the following predicate to your configuration:

predicate allowImplicitRead(Node node, ContentSet c) {
  isSink(node) and c.(FieldContent).getField().hasName("data")
}

Here I've identified the field in question by name - of course you have other options, such as e.g. belonging to a specific type, etc.
With this in place you can drop your isAdditionalFlowStep and have proper flow.