diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSources.qll b/java/ql/src/semmle/code/java/dataflow/FlowSources.qll
index 3d3cac9ca4b6..fc4a951f4dc7 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSources.qll
+++ b/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)
+ )
+ }
+}
diff --git a/java/ql/test/query-tests/security/CWE-749/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-749/AndroidManifest.xml
index b215e4d34669..91413466f4dd 100755
--- a/java/ql/test/query-tests/security/CWE-749/AndroidManifest.xml
+++ b/java/ql/test/query-tests/security/CWE-749/AndroidManifest.xml
@@ -42,9 +42,14 @@
+
+
+
+
+
diff --git a/java/ql/test/query-tests/security/CWE-749/BaseActivity.java b/java/ql/test/query-tests/security/CWE-749/BaseActivity.java
new file mode 100644
index 000000000000..81536581a0a3
--- /dev/null
+++ b/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");
+ }
+}
diff --git a/java/ql/test/query-tests/security/CWE-749/SafeActivity4.java b/java/ql/test/query-tests/security/CWE-749/SafeActivity4.java
new file mode 100644
index 000000000000..54f52f26dfc6
--- /dev/null
+++ b/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
+ }
+}
diff --git a/java/ql/test/query-tests/security/CWE-749/UnsafeActivity4.java b/java/ql/test/query-tests/security/CWE-749/UnsafeActivity4.java
index 992e753f38d0..24a5a69639eb 100644
--- a/java/ql/test/query-tests/security/CWE-749/UnsafeActivity4.java
+++ b/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
+
}
}
diff --git a/java/ql/test/query-tests/security/CWE-749/UnsafeActivity5.java b/java/ql/test/query-tests/security/CWE-749/UnsafeActivity5.java
new file mode 100644
index 000000000000..16c521c40494
--- /dev/null
+++ b/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
+ }
+}