FlowDroid icon indicating copy to clipboard operation
FlowDroid copied to clipboard

Missed sink when using --pathreconstructionmode PRECISE

Open draftyfrog opened this issue 1 year ago • 1 comments

Please consider the following code:

public void onCreate(Bundle savedInstanceState){
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main);

    String taint_1 = function1(source());
    String taint_2 = function1(taint_1);
    sink(taint_2);
}
public String source(){ // Defined as source 
    return "Secret";
}

public void sink(String param){ // Defined as sink
}

public String function1(String arg1){
    arg1 = function2(arg1);
    return arg1;
}
public String function2(String arg1){
    return arg1;
}

There is a taint path from the source()-call in onCreate to the sink, traversing function1 and function2 two times. If I run FlowDroid with the following command

java -jar ./soot-infoflow-cmd-2.13.0-jar-with-dependencies.jar \
 -a {path-to-apk} \
 -s ./SourcesAndSinks.xml \
 -o ./out.xml \
 -p {path-to-android-platforms-folder} \
 --mergedexfiles \
 --pathreconstructionmode PRECISE

it won't report this leak. If I change the --pathreconstructionmode from PRECISE to FAST or NONE (or just remove the whole argument), FlowDroid reports this leak.

If relevant, my SourcesAndSinks.xml looks like this

<sinkSources>
    <category id="NO_CATEGORY">
        <method signature="{package-name}.MainActivity: java.lang.String source()&gt;">
            <return type="java.lang.String">
                <accessPath isSource="true" isSink="false">
                </accessPath>
            </return>
        </method>
        <method signature="{package-name}.MainActivity: void sink(java.lang.String)&gt;">
            <param index="0" type="java.lang.String">
                <accessPath isSource="false" isSink="true"/>
            </param>
        </method>
    </category>
</sinkSources>

draftyfrog avatar Sep 14 '24 14:09 draftyfrog

That's sort of a known limitation.

The result of the IFDS-based analysis is a taint graph where the nodes are the domain elements. Transforming this graph into a path through the program is basically equal to unrolling the DFS path. Because taint graphs can be cyclic (recursion) and we need to ensure termination, there is the arbitrarily-chosen limit of unrolling each taint at most once.

However, a limit of one also prevents building the path through repeatedly called functions.

https://github.com/secure-software-engineering/FlowDroid/blob/3a45a5d22656f20f4bb8602f13f7752252d0b536/soot-infoflow/src/soot/jimple/infoflow/data/SourceContextAndPath.java#L187-L195

Further, setting the path mode also affects an optimization in the IFDS solver itself.

  • If the user does not want any paths, there's no need to keep the taint graph of callees at all. So in your example, at the end, we just have a graph source() -> taint_1 -> taint_2.
  • If the user just cares about things that contribute to the taint propagation, we can cut out all callees that returned the calling context. Basically, this removes the arg -> param -> arg edges out of the taint graph.
  • PRECISE equals to just print every statement associated with a taint in the graph, so there is no cutting at return sites at all. Therefore, the cutting might remove the cycle before the graph is being passed into the path builder.

https://github.com/secure-software-engineering/FlowDroid/blob/3a45a5d22656f20f4bb8602f13f7752252d0b536/soot-infoflow/src/soot/jimple/infoflow/solver/AbstractIFDSSolver.java#L26-L54

If I change the --pathreconstructionmode from PRECISE to FAST or NONE (or just remove the whole argument), FlowDroid reports this leak.

If I run your example, only NONE will report the leak, as expected by the explanation above.

t1mlange avatar May 09 '25 15:05 t1mlange