Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive with sanitizing function in a loop #763

Open
draftyfrog opened this issue Sep 9, 2024 · 2 comments
Open

False positive with sanitizing function in a loop #763

draftyfrog opened this issue Sep 9, 2024 · 2 comments

Comments

@draftyfrog
Copy link

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
Copy link
Member

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
Copy link
Author

draftyfrog commented Sep 9, 2024

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.

@draftyfrog draftyfrog changed the title False positive with sanitizing function in a loop False positive with sanitizing function in a loop using --mergedexfiles Sep 10, 2024
@draftyfrog draftyfrog changed the title False positive with sanitizing function in a loop using --mergedexfiles False positive with sanitizing function in a loop Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants