[Soot-list] FlowDroid produces unreasonable false positive
flanker017
flankerhqd017 at gmail.com
Thu Oct 30 03:38:11 EDT 2014
Thanks Steven for your reply, I understand this tradeoff although in many
cases it will lead to taint explosion and the results are misleading. I'm
trying to work on a better approach, especially for instance fields.
2014-10-29 19:52 GMT+08:00 Steven Arzt <Steven.Arzt at cased.de>:
> Hi,
>
>
>
> You found the correct piece of code. There is even a comment that
> documents precisely the behavior you describe:
>
>
>
> // if the base object which executes the method is tainted the sink is
> reached, too.
>
>
>
> The reasoning behind this rule is as follows: If I write a tainted value
> into an object and a method on the class of that object is defined as a
> sink, it could theoretically read out the tainted value and leak it. Take
> the following example:
>
>
>
> Container c = new Container();
>
> c.data = source();
>
> c.leakMyData();
>
>
>
> Since the classes containing sinks are assumed to be library classes, you
> cannot look into them in the usual case. You thus cannot know whether
> “leakMyData()” actually leaks the contents of field “data” or not. We thus
> conservatively assume it does.
>
>
>
> The only way around this problem would be to extend the SourceSinkManager
> to have distinct notations for sinks which only leak data if one of their
> parameters is tainted and ones which read out fields. More precisely, a
> sink description would need to speak about concrete fields and concrete
> parameters instead of just saying “if anything that flows into this method
> is tainted, consider it as a leak”. While this would be helpful in some
> cases, it also requires some implementation effort. If you volunteer to
> extend FlowDroid in that direction, I would be happy to merge the results
> back into the open-source project and also provide some assistance.
>
>
>
> Best regards,
>
> Steven
>
>
>
>
>
> M.Sc. M.Sc. Steven Arzt
>
> Secure Software Engineering Group (SSE)
>
> European Center for Security and Privacy by Design (EC SPRIDE)
>
> Mornewegstraße 32
>
> D-64293 Darmstadt
>
> Phone: +49 61 51 16-75426
>
> Fax: +49 61 51 16-72118
>
> eMail: steven.arzt at ec-spride.de
>
> Web: http://sse.ec-spride.de
>
>
>
>
>
>
>
> *Von:* soot-list-bounces at CS.McGill.CA [mailto:
> soot-list-bounces at CS.McGill.CA] *Im Auftrag von *flanker017
> *Gesendet:* Mittwoch, 29. Oktober 2014 12:00
> *An:* soot-list at CS.McGill.CA
> *Betreff:* [Soot-list] FlowDroid produces unreasonable false positive
>
>
>
> Hi, I found FlowDroid produces unreasonable false positive on fields.
>
>
>
> if f.a is tainted, FlowDroid will mark f as tainted, thus introducing huge
> amount of false positives.
>
>
>
> Detail can be found at
> https://github.com/secure-software-engineering/soot-infoflow-android/issues/38
> .
>
>
>
> FlowDroid will deduce that there is a flow from intent.getStringExtra('r')
> to this.getString(2131230774) for the following source, which is no doubt a
> false positive.
>
>
>
>
>
> package com.example.testwebview;
>
>
>
> import android.app.Activity;
>
> import android.content.Intent;
>
> import android.os.Bundle;
>
> import android.widget.Button;
>
>
>
> public class MainActivity extends Activity {
>
> protected String n;
>
> protected Button T;
>
>
>
> @Override
>
> protected void onCreate(Bundle savedInstanceState) {
>
> super.onCreate(savedInstanceState);
>
>
>
> Intent intent = this.getIntent();
>
> this.n = intent.getStringExtra("r");
>
> this.T = new Button(this);
>
> this.T.setText(this.getString(2131230774));
>
>
>
> }
>
> }
>
>
>
> Produced flow:
>
>
>
> [main] INFO soot.jimple.infoflow.data.pathBuilders.ContextInsensitivePathBuilder - Obtainted 1 connections between sources and sinks
>
> [main] INFO soot.jimple.infoflow.data.pathBuilders.ContextInsensitivePathBuilder - Building path 1
>
> [main] INFO soot.jimple.infoflow.data.pathBuilders.ContextInsensitivePathBuilder - Path processing took 0.002538356 seconds in total for 3 edges
>
> [main] INFO soot.jimple.infoflow.Infoflow - The sink $r3 = virtualinvoke $r0.<com.example.testwebview.MainActivity: java.lang.String getString(int)>(2131230774) in method <com.example.testwebview.MainActivity: void onCreate(android.os.Bundle)> was called with values from the following sources:
>
> [main] INFO soot.jimple.infoflow.Infoflow - - $r3 = virtualinvoke $r2.<android.content.Intent: java.lang.String getStringExtra(java.lang.String)>("r") in method <com.example.testwebview.MainActivity: void onCreate(android.os.Bundle)>
>
> [main] INFO soot.jimple.infoflow.Infoflow - on Path:
>
> [main] INFO soot.jimple.infoflow.Infoflow - -> <com.example.testwebview.MainActivity: void onCreate(android.os.Bundle)>
>
> [main] INFO soot.jimple.infoflow.Infoflow - -> $r3 = virtualinvoke $r2.<android.content.Intent: java.lang.String getStringExtra(java.lang.String)>("r")
>
> [main] INFO soot.jimple.infoflow.Infoflow - -> <com.example.testwebview.MainActivity: void onCreate(android.os.Bundle)>
>
> [main] INFO soot.jimple.infoflow.Infoflow - -> $r0.<com.example.testwebview.MainActivity: java.lang.String n> = $r3
>
> [main] INFO soot.jimple.infoflow.Infoflow - -> <com.example.testwebview.MainActivity: void onCreate(android.os.Bundle)>
>
> [main] INFO soot.jimple.infoflow.Infoflow - -> $r3 = virtualinvoke $r0.<com.example.testwebview.MainActivity: java.lang.String getString(int)>(2131230774)
>
> Running arguments: app-debug.apk /home/xxx/adt-bundle/sdk/platforms/
> --aplength 1 --nocallbacks --layoutmode none --pathalgo CONTEXTINSENSITIVE
>
>
>
> Test apk:
>
> https://www.dropbox.com/s/wnirus0as7p0p2n/sample.apk?dl=0
>
> I trace down the source code, and think the issue maybe at
>
> public FlowFunction<Abstraction> getCallToReturnFlowFunction,
>
>
>
> In field references, newSource.getAccessPath.getPlainValue() is "field
> base value", not actually tainted value. That is , if f.a is tainted, this
> method will return f, not f.a, which is not reasonable.
>
>
>
> if (isSink) {
>
> // If we are inside a
> conditional branch, we consider every sink call a leak
>
> boolean conditionalCall =
> enableImplicitFlows
>
> &&
> !interproceduralCFG().getMethodOf(call).isStatic()
>
> &&
> aliasing.mayAlias(interproceduralCFG().getMethodOf(call).getActiveBody().getThisLocal(),
>
>
> newSource.getAccessPath().getPlainValue())
>
> &&
> newSource.getAccessPath().getFirstField() == null;
>
> boolean taintedParam =
> (conditionalCall
>
> ||
> newSource.getTopPostdominator() != null
>
> ||
> newSource.getAccessPath().isEmpty())
>
> &&
> newSource.isAbstractionActive();
>
> // If the base object is
> tainted, we also consider the "code" associated
>
> // with the object's
> class as tainted.
>
> if (!taintedParam) {
>
> for (int i = 0; i
> < callArgs.length; i++) {
>
> if
> (aliasing.mayAlias(callArgs[i], newSource.getAccessPath().getPlainValue()))
> {
>
>
> taintedParam = true;
>
> break;
>
> }
>
> }
>
> }
>
>
>
> if (taintedParam &&
> newSource.isAbstractionActive())
>
> addResult(new
> AbstractionAtSink(newSource, invExpr, iStmt));
>
> // if the base object
> which executes the method is tainted the sink is reached, too.
>
> if (invExpr instanceof
> InstanceInvokeExpr) {
>
> InstanceInvokeExpr
> vie = (InstanceInvokeExpr) iStmt.getInvokeExpr();
>
> if
> (newSource.isAbstractionActive()
>
> &&
> aliasing.mayAlias(vie.getBase(), newSource.getAccessPath().getPlainValue()))
>
> {
>
>
> addResult(new AbstractionAtSink(newSource, invExpr, iStmt));
>
> }
>
> }
>
> }
>
>
>
> Will someone kindly look into this issue? Thanks.
>
--
Sincerely,
Flanker He (a.k.a. Qidan He)
Website: http://flanker017.me
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.CS.McGill.CA/pipermail/soot-list/attachments/20141030/e3f06537/attachment-0001.html
More information about the Soot-list
mailing list