Merge pull request #5846 from atorralba/atorralba/promote-unsafe-android-webview-fetch

Java: Promote Unsafe resource loading in Android WebView from experimental

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

@@ -0,0 +1,51 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.example.app"
+    android:installLocation="auto"
+    android:versionCode="1"
+    android:versionName="0.1" >
+
+    <uses-permission android:name="android.permission.INTERNET" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name"
+        android:theme="@style/AppTheme" >
+        <activity
+            android:name=".UnsafeAndroidAccess"
+            android:icon="@drawable/ic_launcher"
+			android:label="@string/app_name">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+
+        <activity android:name=".UnsafeActivity1" android:exported="true">
+            <intent-filter>
+                <action android:name="android.intent.action.VIEW"/>
+            </intent-filter>
+        </activity>
+        
+        <activity android:name=".UnsafeActivity2">
+            <intent-filter>
+            <action android:name="android.intent.action.VIEW"/>
+            </intent-filter>
+        </activity>
+        
+        <activity android:name=".SafeActivity1" android:exported="false">
+            <intent-filter>
+                <action android:name="android.intent.action.VIEW"/>
+            </intent-filter>
+        </activity>
+
+        <activity android:name=".SafeActivity2" android:exported="false" />
+
+        <activity android:name=".SafeActivity3" />
+
+        <activity android:name=".UnsafeActivity3" android:exported="true" />
+        <activity android:name=".UnsafeActivity4" android:exported="true" />
+
+        <receiver android:name=".UnsafeAndroidBroadcastReceiver" android:exported="true" />        
+    </application>
+
+</manifest>

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

@@ -0,0 +1,24 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+/** A utility program for getting intent extra information from Android activity */
+public class IntentUtils {
+	/** Get intent extra */
+	public static String getIntentUrl(Activity a) {
+		String thisUrl = a.getIntent().getStringExtra("url");
+		return thisUrl;
+	}
+
+	/** Get bundle extra */
+	public static String getBundleUrl(Activity a) {
+		String thisUrl = a.getIntent().getExtras().getString("url");
+		return thisUrl;
+	}
+}

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

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class SafeActivity1 extends Activity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras.
+	// The Activity is explicitly not exported, even though it has an intent-filter.
+	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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // Safe
+	}
+}

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

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class SafeActivity2 extends Activity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras.
+	// The Activity is explicitly 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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // Safe
+	}
+}

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

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class SafeActivity3 extends Activity {
+	// 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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // Safe
+	}
+}

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

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class UnsafeActivity1 extends Activity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras.
+	// The Activity is exported and has an intent-filter.
+	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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+}

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

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class UnsafeActivity2 extends Activity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras.
+	// The Activity is implicitly exported because it has an intent-filter.
+	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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+}

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

@@ -0,0 +1,36 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class UnsafeActivity3 extends Activity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras.
+	// The Activity is explicitly 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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+}

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

@@ -0,0 +1,44 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class UnsafeActivity4 extends Activity {
+	/*
+	 * 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 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);
+		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 = IntentUtils.getIntentUrl(this);
+		thisUrl = IntentUtils.getBundleUrl(this);
+		wv.loadUrl(thisUrl); // $ MISSING: hasUnsafeAndroidAccess
+	}
+}

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

@@ -0,0 +1,151 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.os.Bundle;
+
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+// The Activity is implicitly exported because it has an intent-filter.
+public class UnsafeAndroidAccess extends Activity {
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from bundle extras
+	public void testOnCreate1(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 = getIntent().getExtras().getString("url");
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs from string extra
+	public void testOnCreate2(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 = getIntent().getStringExtra("url");
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+
+	// Test onCreate with both JavaScript and cross-origin resource access disabled by default while
+	// taking remote user inputs
+	public void testOnCreate3(Bundle savedInstanceState) {
+		super.onCreate(savedInstanceState);
+		setContentView(-1);
+
+		WebView wv = (WebView) findViewById(-1);
+		WebSettings webSettings = wv.getSettings();
+
+		wv.setWebViewClient(new WebViewClient() {
+			@Override
+			public boolean shouldOverrideUrlLoading(WebView view, String url) {
+				view.loadUrl(url);
+				return true;
+			}
+		});
+
+		String thisUrl = getIntent().getStringExtra("url");
+		wv.loadUrl(thisUrl); // Safe
+	}
+
+	// Test onCreate with JavaScript enabled but cross-origin resource access disabled while taking
+	// remote user inputs
+	public void testOnCreate4(Bundle savedInstanceState) {
+		super.onCreate(savedInstanceState);
+		setContentView(-1);
+
+		WebView wv = (WebView) findViewById(-1);
+		WebSettings webSettings = wv.getSettings();
+
+		webSettings.setJavaScriptEnabled(true);
+
+		wv.setWebViewClient(new WebViewClient() {
+			@Override
+			public boolean shouldOverrideUrlLoading(WebView view, String url) {
+				view.loadUrl(url);
+				return true;
+			}
+		});
+
+		String thisUrl = getIntent().getStringExtra("url");
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while not taking
+	// remote user inputs
+	public void testOnCreate5(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;
+			}
+		});
+
+		wv.loadUrl("https://www.mycorp.com"); // Safe
+	}
+
+	// Test onCreate with both JavaScript and cross-origin resource access enabled while taking
+	// remote user inputs and concatenating them to a safe URL.
+	public void testOnCreate6(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 = getIntent().getStringExtra("url");
+		wv.loadUrl("https://www.mycorp.com/" + thisUrl); // Safe
+	}
+}

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

@@ -0,0 +1,20 @@
+import java
+import semmle.code.java.security.UnsafeAndroidAccessQuery
+import TestUtilities.InlineExpectationsTest
+
+class UnsafeAndroidAccessTest extends InlineExpectationsTest {
+  UnsafeAndroidAccessTest() { this = "HasUnsafeAndroidAccess" }
+
+  override string getARelevantTag() { result = "hasUnsafeAndroidAccess" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasUnsafeAndroidAccess" and
+    exists(DataFlow::Node src, DataFlow::Node sink, FetchUntrustedResourceConfiguration conf |
+      conf.hasFlow(src, sink)
+    |
+      sink.getLocation() = location and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}

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

@@ -0,0 +1,35 @@
+package com.example.app;
+
+import android.app.Activity;
+
+import android.content.Context;
+import android.content.Intent;
+import android.content.BroadcastReceiver;
+import android.os.Bundle;
+import android.webkit.WebSettings;
+import android.webkit.WebView;
+import android.webkit.WebViewClient;
+
+public class UnsafeAndroidBroadcastReceiver extends BroadcastReceiver {
+	// Test onCreate with JavaScript enabled but cross-origin resource access disabled while taking
+	// remote user inputs
+	@Override
+	public void onReceive(Context context, Intent intent) {
+		String thisUrl = intent.getStringExtra("url");
+		WebView wv = null;
+		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;
+			}
+		});
+
+		wv.loadUrl(thisUrl); // $hasUnsafeAndroidAccess
+	}
+}

java/ql/test/query-tests/security/CWE-749/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/android