Add the Intent parameter of onActivityResult as a source

Author: atorralba

java/ql/lib/semmle/code/java/dataflow/FlowSources.qll

@@ -17,6 +17,7 @@ import semmle.code.java.frameworks.android.WebView
 import semmle.code.java.frameworks.JaxWS
 import semmle.code.java.frameworks.javase.WebSocket
 import semmle.code.java.frameworks.android.Android
+import semmle.code.java.frameworks.android.OnActivityResultSource
 import semmle.code.java.frameworks.android.Intent
 import semmle.code.java.frameworks.play.Play
 import semmle.code.java.frameworks.spring.SpringWeb
@@ -264,3 +265,13 @@ class ExportedAndroidContentProviderInput extends RemoteFlowSource, AndroidConte
 
   override string getSourceType() { result = "Exported Android content provider source" }
 }
+
+/**
+ * The data Intent parameter in the `onActivityResult` method in an Activity or Fragment that
+ * calls `startActivityForResult` with an implicit Intent.
+ */
+class OnActivityResultIntentSource extends OnActivityResultIncomingIntent, RemoteFlowSource {
+  OnActivityResultIntentSource() { isRemoteSource() }
+
+  override string getSourceType() { result = "Android onActivityResult incoming Intent" }
+}

java/ql/lib/semmle/code/java/frameworks/android/Fragment.qll

@@ -1,16 +1,14 @@
-/** Provides classes and predicates to track Android fragments. */
-
 import java
 
-/** The class `android.app.Fragment` */
-class Fragment extends Class {
-  Fragment() { this.hasQualifiedName("android.app", "Fragment") }
+/** An Android Fragment. */
+class AndroidFragment extends Class {
+  AndroidFragment() { this.getASupertype*().hasQualifiedName("android.app", "Fragment") }
 }
 
 /** The method `instantiate` of the class `android.app.Fragment`. */
 class FragmentInstantiateMethod extends Method {
   FragmentInstantiateMethod() {
-    this.getDeclaringType() instanceof Fragment and
+    this.getDeclaringType() instanceof AndroidFragment and
     this.hasName("instantiate")
   }
 }

java/ql/lib/semmle/code/java/frameworks/android/OnActivityResultSource.qll

@@ -0,0 +1,70 @@
+/** Provides a remote flow source for Android's `Activity.onActivityResult` method. */
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.DataFlow5
+private import semmle.code.java.frameworks.android.Android
+private import semmle.code.java.frameworks.android.Fragment
+private import semmle.code.java.frameworks.android.Intent
+
+/**
+ * The data Intent parameter in the `onActivityResult` method.
+ */
+class OnActivityResultIncomingIntent extends DataFlow::Node {
+  OnActivityResultIncomingIntent() {
+    exists(Method onActivityResult |
+      onActivityResult.getDeclaringType() instanceof ActivityOrFragment and
+      onActivityResult.hasName("onActivityResult") and
+      this.asParameter() = onActivityResult.getParameter(2)
+    )
+  }
+
+  /**
+   * Holds if this node is a remote flow source.
+   *
+   * This is only a source when the Activity or Fragment that implements `onActivityResult` is
+   * also using an implicit Intent to start another Activity with `startActivityForResult`. This
+   * means that a malicious application can intercept it to start itself and return an arbitrary
+   * Intent to `onActivityResult`.
+   */
+  predicate isRemoteSource() {
+    exists(ImplicitStartActivityForResultConf conf, DataFlow::Node sink |
+      conf.hasFlowTo(sink) and
+      DataFlow::getInstanceArgument(sink.asExpr().(Argument).getCall()).getType() =
+        this.getEnclosingCallable().getDeclaringType()
+    )
+  }
+}
+
+/**
+ * A data flow configuration for implicit intents being used in `startActivityForResult`.
+ */
+private class ImplicitStartActivityForResultConf extends DataFlow5::Configuration {
+  ImplicitStartActivityForResultConf() { this = "ImplicitStartActivityForResultConf" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(ClassInstanceExpr cc |
+      cc.getConstructedType() instanceof TypeIntent and source.asExpr() = cc
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(ActivityOrFragment actOrFrag, MethodAccess startActivityForResult |
+      startActivityForResult.getMethod().hasName("startActivityForResult") and
+      startActivityForResult.getEnclosingCallable() = actOrFrag.getACallable() and
+      sink.asExpr() = startActivityForResult.getArgument(0)
+    )
+  }
+
+  override predicate isBarrier(DataFlow::Node barrier) {
+    barrier instanceof ExplicitIntentSanitizer
+  }
+}
+
+/** An Android Activity or Fragment. */
+private class ActivityOrFragment extends Class {
+  ActivityOrFragment() {
+    this instanceof AndroidActivity or
+    this instanceof AndroidFragment
+  }
+}

java/ql/test/library-tests/frameworks/android/sources/OnActivityResultSourceTest.expected

@@ -0,0 +1,11 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import TestUtilities.InlineFlowTest
+
+class SourceValueFlowConf extends DefaultValueFlowConf {
+  override predicate isSource(DataFlow::Node sink) { sink instanceof RemoteFlowSource }
+}
+
+class SourceInlineFlowTest extends InlineFlowTest {
+  override DataFlow::Configuration getTaintFlowConfig() { none() }
+}

java/ql/test/library-tests/frameworks/android/sources/OnActivityResultSourceTest.ql

@@ -0,0 +1,21 @@
+package com.example.app;
+
+import android.app.Activity;
+import android.content.Intent;
+import android.os.Bundle;
+
+public class Safe extends Activity {
+
+	void sink(Object o) {}
+
+	public void onCreate(Bundle saved) {
+		Intent explicitIntent = new Intent(this, Activity.class);
+		startActivityForResult(explicitIntent, 0);
+	}
+
+	protected void onActivityResult(int requestCode, int resultCode, Intent data) {
+		sink(requestCode); // safe
+		sink(resultCode); // safe
+		sink(data); // Safe
+	}
+}

java/ql/test/library-tests/frameworks/android/sources/Safe.java

@@ -0,0 +1,20 @@
+package com.example.app;
+
+import android.app.Activity;
+import android.content.Intent;
+import android.os.Bundle;
+
+public class Safe2 extends Activity {
+
+	void sink(Object o) {}
+
+	public void onCreate(Bundle saved) {
+		// activityForResult not called
+	}
+
+	protected void onActivityResult(int requestCode, int resultCode, Intent data) {
+		sink(requestCode); // safe
+		sink(resultCode); // safe
+		sink(data); // Safe
+	}
+}

java/ql/test/library-tests/frameworks/android/sources/Safe2.java

@@ -0,0 +1,21 @@
+package com.example.app;
+
+import android.app.Activity;
+import android.content.Intent;
+import android.os.Bundle;
+
+public class Test extends Activity {
+
+	void sink(Object o) {}
+
+	public void onCreate(Bundle saved) {
+		Intent implicitIntent = new Intent("SOME_ACTION");
+		startActivityForResult(implicitIntent, 0);
+	}
+
+	protected void onActivityResult(int requestCode, int resultCode, Intent data) {
+		sink(requestCode); // safe
+		sink(resultCode); // safe
+		sink(data); // $ hasValueFlow
+	}
+}

java/ql/test/library-tests/frameworks/android/sources/Test.java

@@ -0,0 +1,22 @@
+package com.example.app;
+
+import android.app.Fragment;
+import android.content.Intent;
+import android.os.Bundle;
+
+public class TestFragment extends Fragment {
+
+    void sink(Object o) {}
+
+    public void onCreate(Bundle savedInstance) {
+        Intent implicitIntent = new Intent("SOME_ACTION");
+        startActivityForResult(implicitIntent, 0);
+    }
+
+    @Override
+    public void onActivityResult(int requestCode, int resultCode, Intent data) {
+        sink(requestCode); // safe
+        sink(resultCode); // safe
+        sink(data); // $ hasValueFlow
+    }
+}

java/ql/test/library-tests/frameworks/android/sources/TestFragment.java

@@ -0,0 +1,28 @@
+package com.example.app;
+
+import android.app.Activity;
+import android.content.Context;
+import android.content.Intent;
+import android.os.Bundle;
+
+public class TestMissing extends Activity {
+
+	void sink(Object o) {}
+
+	public void onCreate(Bundle saved) {
+		Helper.startNewActivity(this);
+	}
+
+	protected void onActivityResult(int requestCode, int resultCode, Intent data) {
+		sink(requestCode); // safe
+		sink(resultCode); // safe
+		sink(data); // $ MISSING: $hasValueFlow
+	}
+
+	static class Helper {
+		public static void startNewActivity(Activity ctx) {
+			Intent implicitIntent = new Intent("SOME_ACTION");
+			ctx.startActivityForResult(implicitIntent, 0);
+		}
+	}
+}