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

FlowDroid fails to propagate taint when involving type casting #470

Closed
jimmy66688 opened this issue May 17, 2022 · 11 comments
Closed

FlowDroid fails to propagate taint when involving type casting #470

jimmy66688 opened this issue May 17, 2022 · 11 comments

Comments

@jimmy66688
Copy link

jimmy66688 commented May 17, 2022

considering this running example:

public class Case21CastingAfterMapGet {

    public static void main(String[] args) {
        Map<String, Object> conf = new HashMap<String, Object>();
        conf.put("mode", 'c');
        conf.put("jobs", 1);
        char mode = (char) (conf.get("mode"));

        if (mode == 'c') {
            System.out.println("mode is c");
        }

        int jobs = (int) (conf.get("jobs"));
        if (jobs == 1){
            System.out.println("jobs is 1");
        }
    }
}

the source method signature is "<java.util.Map: java.lang.Object get(java.lang.Object)>"
the sink we foucus on is every if statement, so I instrument a dummy sink void sink(Object o) before every if statment, so that i can track if
the log shows that [main] WARN No results found.
so, the question i wanna know is What causes this?, i found a paper Analyzing Android Taint Analysis Tools: FlowDroid, Amandroid, and DroidSafe described this problem, but without any solution.

@jimmy66688
Copy link
Author

InfoflowConfiguration is:

    protected static InfoflowConfiguration globalConf;
    public static void initEnv() {
        // reset soot env
        soot.G.reset();
        System.gc();
        // init configuration

        // config information flow
        globalConf = new InfoflowConfiguration();
        //more precise call graph algorithm
        globalConf.setCallgraphAlgorithm(InfoflowConfiguration.CallgraphAlgorithm.SPARK);
        globalConf.setImplicitFlowMode(InfoflowConfiguration.ImplicitFlowMode.AllImplicitFlows);

        // globalConf.setCodeEliminationMode(InfoflowConfiguration.CodeEliminationMode.PropagateConstants);
       globalConf.setCodeEliminationMode(InfoflowConfiguration.CodeEliminationMode.NoCodeElimination);
        globalConf.getAccessPathConfiguration().setAccessPathLength(20);
        globalConf.getSolverConfiguration().setDataFlowSolver(InfoflowConfiguration.DataFlowSolver.ContextFlowSensitive);
        globalConf.setMaxThreadNum(-1);

        globalConf.setInspectSources(false);
        globalConf.setInspectSinks(false);

        globalConf.setAliasingAlgorithm(InfoflowConfiguration.AliasingAlgorithm.FlowSensitive);

        globalConf.setFlowSensitiveAliasing(true);
        globalConf.setStopAfterFirstFlow(false);
        globalConf.setStaticFieldTrackingMode(InfoflowConfiguration.StaticFieldTrackingMode.ContextFlowSensitive);
        // do not track exception
        globalConf.setEnableExceptionTracking(false);
        globalConf.setOneSourceAtATime(true);
        globalConf.getPathConfiguration().setPathReconstructionMode(InfoflowConfiguration.PathReconstructionMode.Precise);

        globalConf.getSolverConfiguration().setMaxJoinPointAbstractions(-1);
        LOGGER.info("information flow config done.");
    }

@StevenArzt
Copy link
Member

How did you specify your sources and sinks? Did you just place the map-get into SourcesAndSinks.txt? That won't work. This simple definition format assumes that the tainted value is the return value of the method for which you provide the signature. In your case, you want to taint the base object. You can use the much more expressive XML format for sources and sinks. Have a look at the files in soot-infoflow-android/testXmlParser for an example. In complete.xml, there is a taint on the base object that you can study.

The paper would have benefited from asking here...

Concerning your configuration: Why do you enable "one source at a time"? There are only very few use cases for this option. Access Path length 20 is quite a lot and is very unlikely to scale on real-world applications.

@jimmy66688
Copy link
Author

jimmy66688 commented May 17, 2022

Thank for your concerning, I am a freshman. I use a class MapGetSourceSinkManager implements ISourceSinkManager to specify my sources and sinks.
Most of the code copy from DefaultSourceSinkManager, only diff in method boolean isSourceMethod(InfoflowManager manager, Stmt sCallSite).
I modified the globalConf.setCallgraphAlgorithm(InfoflowConfiguration.CallgraphAlgorithm.SPARK); to globalConf.setCallgraphAlgorithm(InfoflowConfiguration.CallgraphAlgorithm.RTA);, the results successfully found.

  - The sink staticinvoke <analysis.option.MySink: void sink(java.lang.Object)>(mode) on line 14 in method <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])> was called with values from the following sources:
  - - $stack0#15 = interfaceinvoke conf.<java.util.Map: java.lang.Object get(java.lang.Object)>("mode") on line 12 in method <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 	on Path: 
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> $stack0#15 = interfaceinvoke conf.<java.util.Map: java.lang.Object get(java.lang.Object)>("mode")
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> $stack0#16 = (java.lang.Character) $stack0#15
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> mode = virtualinvoke $stack0#16.<java.lang.Character: char charValue()>()
  - 	 -> <java.lang.Character: char charValue()>
  - 		 -> $stack0#2 = l0.<java.lang.Character: char value>
  - 	 -> <java.lang.Character: char charValue()>
  - 		 -> return $stack0#2
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> staticinvoke <analysis.option.MySink: void sink(java.lang.Object)>(mode)
  - The sink staticinvoke <analysis.option.MySink: void sink(java.lang.Object)>(jobs) on line 19 in method <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])> was called with values from the following sources:
  - - $stack0#24 = interfaceinvoke conf.<java.util.Map: java.lang.Object get(java.lang.Object)>("jobs") on line 18 in method <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 	on Path: 
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> $stack0#24 = interfaceinvoke conf.<java.util.Map: java.lang.Object get(java.lang.Object)>("jobs")
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> $stack0#25 = (java.lang.Integer) $stack0#24
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> jobs = virtualinvoke $stack0#25.<java.lang.Integer: int intValue()>()
  - 	 -> <java.lang.Integer: int intValue()>
  - 		 -> $stack0#2 = l0.<java.lang.Integer: int value>
  - 	 -> <java.lang.Integer: int intValue()>
  - 		 -> return $stack0#2
  - 	 -> <objectiveProgram.Case21CastingAfterMapGet: void main(java.lang.String[])>
  - 		 -> staticinvoke <analysis.option.MySink: void sink(java.lang.Object)>(jobs)

So, the question is: Why SPARK can not propagate taint in this case

@StevenArzt
Copy link
Member

RTA performs an over-approximation. This may lead to additional leaks. However, it is not a proper solution in most cases.

Your second post seems to relate to another test application. There is no call to sink() in the original code you posted. Please stick to a consistent example to avoid confusion.

For proper handling of library classes such as HashMap, you need to enable a taint wrapper. I'd recommend the SummaryTaintWrapper from StubDroid.

Your approach of duplicating the source/sink manager is technically possible, but unnecessarily complex. You can use FlowDroid's built-in mechanisms such as the XML-based specification language.

@jimmy66688
Copy link
Author

jimmy66688 commented May 17, 2022

Sorry, I describe my case not clearly. I instrument a sink void sink(Object o) before if statement so that i can track all if statement.
As for TaintWrapper and XML sources/sinks manager, I would give it a try.Until now, I only use soot-infoflow without soot-infoflow-android because I analyze the traditional Java program, not Android program.
Thanks for your reply.

@zhouyuhao1018
Copy link

zhouyuhao1018 commented May 17, 2022

How did you specify your sources and sinks? Did you just place the map-get into SourcesAndSinks.txt? That won't work. This simple definition format assumes that the tainted value is the return value of the method for which you provide the signature. In your case, you want to taint the base object. You can use the much more expressive XML format for sources and sinks. Have a look at the files in soot-infoflow-android/testXmlParser for an example. In complete.xml, there is a taint on the base object that you can study.

The paper would have benefited from asking here...

Concerning your configuration: Why do you enable "one source at a time"? There are only very few use cases for this option. Access Path length 20 is quite a lot and is very unlikely to scale on real-world applications.

Sorry, I am a freshman who is learning FlowDroid, and I wanna know why you say " In your case, you want to taint the base object "?
If he places the map-get method signature <java.util.Map: java.lang.Object get(java.lang.Object)> into SourcesAndSinks.txt, Is this method's return value a taint that will taint the leftOp ? Why does he need to taint the base object ?

@StevenArzt
Copy link
Member

get is a sink, that's not a problem The leak is detected if at least one argument passed to the sink method is tainted. The problem lies with the source:

conf.put("mode", 'c');

You need to taint the base object conf. In the simple SourcesAndSinks.txt file, FlowDroid expects that the return value is tainted, e.g.,

String secret = source();

You have a special case, and the more expressive XML format for sources and sinks can be used to model such cases.

@jimmy66688
Copy link
Author

jimmy66688 commented May 18, 2022

RTA performs an over-approximation. This may lead to additional leaks. However, it is not a proper solution in most cases.

Your second post seems to relate to another test application. There is no call to sink() in the original code you posted. Please stick to a consistent example to avoid confusion.

For proper handling of library classes such as HashMap, you need to enable a taint wrapper. I'd recommend the SummaryTaintWrapper from StubDroid.

Your approach of duplicating the source/sink manager is technically possible, but unnecessarily complex. You can use FlowDroid's built-in mechanisms such as the XML-based specification language.

I am considering: Is the real problem in handling of library class?
conf.get("mode") is source.
if (mode == 'c') is sink.
My analysis successfully identify source and sink.
Using CallgraphAlgorithm.SPARK can not build infoflow path, but CallgraphAlgorithm.RTA can! Is there some problem in point-to analysis? I use FlowDroid version 2.8.
Type casting between source and sink. Type casting involved <java.lang.Character: char charValue()>(), that means I need a taint wrapper to handling the <java.lang.Character: char charValue()>() method?

@zhouyuhao1018
Copy link

RTA performs an over-approximation. This may lead to additional leaks. However, it is not a proper solution in most cases.
Your second post seems to relate to another test application. There is no call to sink() in the original code you posted. Please stick to a consistent example to avoid confusion.
For proper handling of library classes such as HashMap, you need to enable a taint wrapper. I'd recommend the SummaryTaintWrapper from StubDroid.
Your approach of duplicating the source/sink manager is technically possible, but unnecessarily complex. You can use FlowDroid's built-in mechanisms such as the XML-based specification language.

I considering: Is the real problem in handling of library class? conf.get("mode") is source. if (mode == 'c') is sink. My analysis is successfully identify source and sink. Using CallgraphAlgorithm.SPARK can not build infoflow path, but CallgraphAlgorithm.RTA can! Is there some problem in point-to analysis? I use FlowDroid version 2.8. Type casting between source and sink. Type casting involved <java.lang.Character: char charValue()>(), that means I need a taint wrapper to handling the <java.lang.Character: char charValue()>() method?

May I ask how do you define the stmt if (mode == 'c') as a sink? do you use the StatementSourceSinkDefinition with its constructor and pass the if stmt and local mode to it?

@jimmy66688
Copy link
Author

RTA performs an over-approximation. This may lead to additional leaks. However, it is not a proper solution in most cases.
Your second post seems to relate to another test application. There is no call to sink() in the original code you posted. Please stick to a consistent example to avoid confusion.
For proper handling of library classes such as HashMap, you need to enable a taint wrapper. I'd recommend the SummaryTaintWrapper from StubDroid.
Your approach of duplicating the source/sink manager is technically possible, but unnecessarily complex. You can use FlowDroid's built-in mechanisms such as the XML-based specification language.

I considering: Is the real problem in handling of library class? conf.get("mode") is source. if (mode == 'c') is sink. My analysis is successfully identify source and sink. Using CallgraphAlgorithm.SPARK can not build infoflow path, but CallgraphAlgorithm.RTA can! Is there some problem in point-to analysis? I use FlowDroid version 2.8. Type casting between source and sink. Type casting involved <java.lang.Character: char charValue()>(), that means I need a taint wrapper to handling the <java.lang.Character: char charValue()>() method?

May I ask how do you define the stmt if (mode == 'c') as a sink? do you use the StatementSourceSinkDefinition with its constructor and pass the if stmt and local mode to it?

I instrument a customize sink method before if statement.You can think about if (mode == 'c') to sink(mode); if (mode == 'c') in jimple IR.

@DeepakUniAdel
Copy link

DeepakUniAdel commented Aug 2, 2022

RTA performs an over-approximation. This may lead to additional leaks. However, it is not a proper solution in most cases.
Your second post seems to relate to another test application. There is no call to sink() in the original code you posted. Please stick to a consistent example to avoid confusion.
For proper handling of library classes such as HashMap, you need to enable a taint wrapper. I'd recommend the SummaryTaintWrapper from StubDroid.
Your approach of duplicating the source/sink manager is technically possible, but unnecessarily complex. You can use FlowDroid's built-in mechanisms such as the XML-based specification language.

I considering: Is the real problem in handling of library class? conf.get("mode") is source. if (mode == 'c') is sink. My analysis is successfully identify source and sink. Using CallgraphAlgorithm.SPARK can not build infoflow path, but CallgraphAlgorithm.RTA can! Is there some problem in point-to analysis? I use FlowDroid version 2.8. Type casting between source and sink. Type casting involved <java.lang.Character: char charValue()>(), that means I need a taint wrapper to handling the <java.lang.Character: char charValue()>() method?

May I ask how do you define the stmt if (mode == 'c') as a sink? do you use the StatementSourceSinkDefinition with its constructor and pass the if stmt and local mode to it?

I instrument a customize sink method before if statement.You can think about if (mode == 'c') to sink(mode); if (mode == 'c') in jimple IR.

@jimmy66688 @zhouyuhao1018 how do u instrument a customised sink before if statements can you please explain a bit..thanks

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

4 participants