secure-software-engineering / FlowDroid

FlowDroid Static Data Flow Tracker
GNU Lesser General Public License v2.1
1.06k stars 299 forks source link

False positive with sanitizing function in a loop #763

Open draftyfrog opened 2 months ago

draftyfrog commented 2 months ago

I've found a bug in the FlowDroid command line tool.

Consider the following code:

public class MainActivity extends AppCompatActivity {
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        String s = "";
        for (int i = 0; i < 10; i++) { // If removed no false positive is reported
            s = sanitize(source());
        }
        sink(s); // FlowDroid finds this leak
    }

    public String sanitize(String arg1) { // NOT defined as source or sink
        if (Math.random() == 123) { // If removed no false positive is reported
        }
        return "NoSecret";
    }

    public String source() { // Defined as source 
        return "Secret";
    }

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

FlowDroid reports one leak for the sink in onCreate. This false positive only happens in the combination "calling the sanitize function within a loop and having the if-statement inside the function". If the if is removed or the call to sanitize is moved out of the loop, FlowDroid reports no leaks. Even if we add another call s = sanitize(source()); between the loop and the sink-call, no false positive is reported.

StevenArzt commented 2 months ago

Please provide your precise FlowDroid configuration. The taint analysis should normally not look at if statements at all, unless you enable implicit flows (which I would advise against, unless you really need it).

draftyfrog commented 2 months ago

Implicit flows are not enabled, I call FlowDroid via

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

and 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>

After tinkering around a little I found that the -d argument for merging all the dex files seems to be the culprit - if a remove -d from my call, FlowDroid doesn't find the leak anymore.

Update: After tinkering around a little more I figured that my current setup doesn't find any leaks at all withouth the --mergedexfiles (-d) option. So maybe thats not the culprit for this false positive as it just turns off all leaks.