Add dataflow to track exported android components to getIntent calls

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

@@ -223,7 +223,7 @@ class ReverseDNSMethod extends Method {
 
 /** Android `Intent` that may have come from a hostile application. */
 class AndroidIntentInput extends DataFlow::Node {
-  Type receiverType;
+  RefType receiverType;
 
   AndroidIntentInput() {
     exists(MethodAccess ma, AndroidGetIntentMethod m |
@@ -242,7 +242,34 @@ class AndroidIntentInput extends DataFlow::Node {
 
 /** Exported Android `Intent` that may have come from a hostile application. */
 class ExportedAndroidIntentInput extends RemoteFlowSource, AndroidIntentInput {
-  ExportedAndroidIntentInput() { receiverType.(ExportableAndroidComponent).isExported() }
+  ExportedAndroidIntentInput() {
+    receiverType.(ExportableAndroidComponent).isExported()
+    or
+    // This generates spurious results when both an unexported and exported activity extend a base class
+    // that provides data from the intent with a helper method.
+    //
+    // exists(ExportableAndroidComponent exported | exported.isExported() |
+    //   exported.getASourceSupertype*() = receiverType
+    // )
+    // or
+    exists(DataFlow::Node sink, ExportedAndroidComponentFlowConfig cfg | cfg.hasFlowTo(sink) |
+      sink.asExpr() = this.asExpr().(MethodAccess).getQualifier()
+    )
+  }
 
   override string getSourceType() { result = "Exported Android intent source" }
 }
+
+private class ExportedAndroidComponentFlowConfig extends DataFlow2::Configuration {
+  ExportedAndroidComponentFlowConfig() { this = "FlowSources::ExportedAndroidComponentFlowConfig" }
+
+  override predicate isSource(DataFlow2::Node src) {
+    src.asExpr().getType().(ExportableAndroidComponent).isExported()
+  }
+
+  override predicate isSink(DataFlow2::Node sink) {
+    exists(AndroidGetIntentMethod m, MethodAccess ma | ma.getMethod() = m |
+      sink = DataFlow::getInstanceArgument(ma)
+    )
+  }
+}

java/ql/test/query-tests/security/CWE-749/AndroidManifest.xml

@@ -42,9 +42,14 @@
 
         <activity android:name=".SafeActivity3" />
 
+        <activity android:name=".SafeActivity4" />
+
         <activity android:name=".UnsafeActivity3" android:exported="true" />
+
         <activity android:name=".UnsafeActivity4" android:exported="true" />
 
+        <activity android:name=".UnsafeActivity5" android:exported="true" />
+
         <receiver android:name=".UnsafeAndroidBroadcastReceiver" android:exported="true" />        
     </application>
 

java/ql/test/query-tests/security/CWE-749/BaseActivity.java

@@ -0,0 +1,14 @@
+package com.example.app;
+
+import android.app.Activity;
+
+public abstract class BaseActivity extends Activity {
+
+    protected String getIntentUrl() {
+        return getIntent().getStringExtra("url");
+    }
+
+    protected String getBundleUrl() {
+        return getIntent().getExtras().getString("url");
+    }
+}

java/ql/test/query-tests/security/CWE-749/SafeActivity4.java

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class SafeActivity4 extends BaseActivity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras.
+	// The Activity is implicitly not exported.
+	public void onCreate(Bundle savedInstanceState) {
+		super.onCreate(savedInstanceState);
+		setContentView(-1);
+
+		WebView wv = (WebView) findViewById(-1);
+		WebSettings webSettings = wv.getSettings();
+
+		webSettings.setJavaScriptEnabled(true);
+		webSettings.setAllowFileAccessFromFileURLs(true);
+
+		wv.setWebViewClient(new WebViewClient() {
+			@Override
+			public boolean shouldOverrideUrlLoading(WebView view, String url) {
+				view.loadUrl(url);
+				return true;
+			}
+		});
+
+		String thisUrl = getIntentUrl();
+		wv.loadUrl(thisUrl); // Safe
+		thisUrl = getBundleUrl();
+		wv.loadUrl(thisUrl); // Safe
+	}
+}

java/ql/test/query-tests/security/CWE-749/UnsafeActivity4.java

@@ -14,10 +14,6 @@ public class UnsafeActivity4 extends Activity {
 	 * remote user inputs from bundle extras.
 	 * 
 	 * The Activity is explicitly exported.
-	 * 
-	 * Note this case of invoking a utility method that takes an Activity and then calls
-	 * `a.getIntent().getStringExtra(...)` is not yet detected thus is beyond what the query is
-	 * capable of.
 	 */
 	public void onCreate(Bundle savedInstanceState) {
 		super.onCreate(savedInstanceState);
@@ -38,7 +34,9 @@ public boolean shouldOverrideUrlLoading(WebView view, String url) {
 		});
 
 		String thisUrl = IntentUtils.getIntentUrl(this);
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
 		thisUrl = IntentUtils.getBundleUrl(this);
-		wv.loadUrl(thisUrl); // $ MISSING: hasUnsafeAndroidAccess
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+
 	}
 }

java/ql/test/query-tests/security/CWE-749/UnsafeActivity5.java

@@ -0,0 +1,43 @@
+package com.example.app;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class UnsafeActivity5 extends BaseActivity {
+	/*
+	 * Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	 * remote user inputs from bundle extras.
+	 * 
+	 * The Activity is explicitly exported.
+	 * 
+	 * Note this case of invoking a helper method from a base class that then calls to
+	 * `getIntent().getStringExtra(...)` is not yet detected thus is beyond what the query is
+	 * capable of.
+	 */
+	public void onCreate(Bundle savedInstanceState) {
+		super.onCreate(savedInstanceState);
+		setContentView(-1);
+
+		WebView wv = (WebView) findViewById(-1);
+		WebSettings webSettings = wv.getSettings();
+
+		webSettings.setJavaScriptEnabled(true);
+		webSettings.setAllowFileAccessFromFileURLs(true);
+
+		wv.setWebViewClient(new WebViewClient() {
+			@Override
+			public boolean shouldOverrideUrlLoading(WebView view, String url) {
+				view.loadUrl(url);
+				return true;
+			}
+		});
+
+		String thisUrl = getIntentUrl();
+		wv.loadUrl(thisUrl); // $ MISSING: hasUnsafeAndroidAccess
+		thisUrl = getBundleUrl();
+		wv.loadUrl(thisUrl); // $ MISSING: hasUnsafeAndroidAccess
+	}
+}