Add test cases; fix handling of recievers declared through xml

Author: joefarebrother

java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll

@@ -72,6 +72,11 @@ class SystemActionName extends Top {
 
   /** Gets the name of the system intent that this expression or attriute represents. */
   string getName() { result = name }
+
+  override string toString() {
+    result =
+      [this.(StringLiteral).toString(), this.(FieldRead).toString(), this.(XMLAttribute).toString()]
+  }
 }
 
 /** A call to `Context.registerReceiver` */
@@ -140,10 +145,10 @@ predicate xmlUnverifiedSystemReceiver(
     filter.hasName("intent-filter") and
     action.hasName("action") and
     filter = rec.getAChild() and
-    action = rec.getAChild() and
+    action = filter.getAChild() and
     ormty = orm.getDeclaringType() and
-    rec.getAttribute("android:name").getValue() = ["." + ormty.getName(), ormty.getQualifiedName()] and
-    action.getAttribute("android:name") = sa
+    rec.getAttribute("name").getValue() = ["." + ormty.getName(), ormty.getQualifiedName()] and
+    action.getAttribute("name") = sa
   )
 }
 

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

@@ -0,0 +1,9 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android" package="test">
+    <application>
+        <receiver android:name=".BootReceiverXml">
+            <intent-filter>
+                <action android:name="android.intent.action.BOOT_COMPLETED" />
+            </intent-filter>
+        </receiver>
+    </application>
+</manifest>

java/ql/test/query-tests/security/CWE-925/BootReceiverXml.java

@@ -0,0 +1,13 @@
+package test;
+import android.content.Intent;
+import android.content.Context;
+import android.content.BroadcastReceiver;
+
+class BootReceiverXml extends BroadcastReceiver {
+    void doStuff(Intent intent) {}
+
+    @Override 
+    public void onReceive(Context ctx, Intent intent) { // $hasResult
+        doStuff(intent);
+    }
+}

java/ql/test/query-tests/security/CWE-925/ImproperIntentVerification.expected

@@ -0,0 +1,18 @@
+import java
+import semmle.code.java.security.ImproperIntentVerificationQuery
+import TestUtilities.InlineExpectationsTest
+
+class HasFlowTest extends InlineExpectationsTest {
+  HasFlowTest() { this = "HasFlowTest" }
+
+  override string getARelevantTag() { result = "hasResult" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasResult" and
+    exists(Method orm | unverifiedSystemReceiver(_, orm, _) |
+      orm.getLocation() = location and
+      element = orm.toString() and
+      value = ""
+    )
+  }
+}

java/ql/test/query-tests/security/CWE-925/ImproperIntentVerification.ql

@@ -0,0 +1,31 @@
+package test;
+import android.content.Intent;
+import android.content.IntentFilter;
+import android.content.Context;
+import android.content.BroadcastReceiver;
+
+class ImproperIntentVerificationTest {
+    static void doStuff(Intent intent) {}
+
+    class ShutdownBroadcastReceiver extends BroadcastReceiver {
+        @Override
+        public void onReceive(Context ctx, Intent intent) { // $hasResult
+            doStuff(intent);
+        }
+    }
+
+    class ShutdownBroadcastReceiverSafe extends BroadcastReceiver {
+        @Override
+        public void onReceive(Context ctx, Intent intent) {
+            if (!intent.getAction().equals(Intent.ACTION_SHUTDOWN)) {
+                return;
+            }
+            doStuff(intent);
+        }
+    }
+
+    void test(Context c) {
+        c.registerReceiver(new ShutdownBroadcastReceiver(), new IntentFilter(Intent.ACTION_SHUTDOWN));
+        c.registerReceiver(new ShutdownBroadcastReceiverSafe(), new IntentFilter(Intent.ACTION_SHUTDOWN));
+    }
+}

java/ql/test/query-tests/security/CWE-925/ImproperIntentVerificationTest.java

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