Merge pull request #225 from MaxNad/issue_224

Issue #224 - Added an exception for the LocalBroadcastManager in the detector.
find-sec-bugs · Oct 3, 2016 · f115d16 · f115d16
2 parents 7abc1e0 + 7061d38
commit f115d16
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 8 deletions.
diff --git a/plugin-deps/src/main/java/android/support/v4/content/LocalBroadcastManager.java b/plugin-deps/src/main/java/android/support/v4/content/LocalBroadcastManager.java
@@ -0,0 +1,14 @@
+package android.support.v4.content;
+
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.Intent;
+
+public class LocalBroadcastManager {
+    public static LocalBroadcastManager getInstance(Context context) { return new LocalBroadcastManager(); }
+
+    //public void registerReceiver(BroadcastReceiver receiver, IntentFilter filter) {};
+    public boolean sendBroadcast(Intent intent) { return true; };
+    public void sendBroadcastSync(Intent intent) {};
+    public void unregisterReceiver(BroadcastReceiver receiver) {};
+}
diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/android/BroadcastDetector.java b/plugin/src/main/java/com/h3xstream/findsecbugs/android/BroadcastDetector.java
@@ -17,6 +17,7 @@
  */
 package com.h3xstream.findsecbugs.android;
 
+import com.h3xstream.findsecbugs.common.InterfaceUtils;
 import edu.umd.cs.findbugs.BugInstance;
 import edu.umd.cs.findbugs.BugReporter;
 import edu.umd.cs.findbugs.Priorities;
@@ -37,13 +38,21 @@ public void sawOpcode(int seen) {
         //printOpCode(seen);
 
         if (seen == Constants.INVOKEVIRTUAL &&
-                (getNameConstantOperand().equals("sendBroadcast") ||
-                getNameConstantOperand().equals("sendBroadcastAsUser") ||
-                getNameConstantOperand().equals("sendOrderedBroadcast") ||
-                getNameConstantOperand().equals("sendOrderedBroadcastAsUser")
+                (
+                    getNameConstantOperand().equals("sendBroadcast") ||
+                    getNameConstantOperand().equals("sendBroadcastAsUser") ||
+                    getNameConstantOperand().equals("sendOrderedBroadcast") ||
+                    getNameConstantOperand().equals("sendOrderedBroadcastAsUser")
                 )) {
-            bugReporter.reportBug(new BugInstance(this, ANDROID_BROADCAST_TYPE, Priorities.NORMAL_PRIORITY) //
-                    .addClass(this).addMethod(this).addSourceLine(this));
+
+            // The LocalBroadcastManager object is safe. The broadcast doesn't leave the application scope.
+            // We check if the class extends android.support.v4.content.LocalBroadcastManager
+            // We will also check if the class is named "LocalBroadcastManager" in case the version in the namespace changes.
+            if (!InterfaceUtils.isSubtype(getClassConstantOperand(), "android.support.v4.content.LocalBroadcastManager")
+                    && !getClassConstantOperand().endsWith("LocalBroadcastManager")) {
+                bugReporter.reportBug(new BugInstance(this, ANDROID_BROADCAST_TYPE, Priorities.NORMAL_PRIORITY) //
+                        .addClass(this).addMethod(this).addSourceLine(this));
+            }
         }
     }
 }
diff --git a/plugin/src/test/java/com/h3xstream/findsecbugs/android/BroadcastDetectorTest.java b/plugin/src/test/java/com/h3xstream/findsecbugs/android/BroadcastDetectorTest.java
@@ -44,11 +44,11 @@ public void detectSendBroadcast() throws Exception {
                         .bugType("ANDROID_BROADCAST") //
                         .inClass("BroadcastIntentActivity") //
                         .inMethod("onCreate") //
-                        .atLine(23) //
+                        .atLine(24) //
                         .build()
         );
 
-        int line = 25; //First line
+        int line = 26; //First line
         while(line++ < 30) {
             verify(reporter).doReportBug(
                     bugDefinition() //

diff --git a/plugin/src/test/java/testcode/android/BroadcastIntentActivity.java b/plugin/src/test/java/testcode/android/BroadcastIntentActivity.java
@@ -2,6 +2,7 @@
 
 import android.app.Activity;
 import android.content.Intent;
+import android.support.v4.content.LocalBroadcastManager;
 import android.os.Bundle;
 
 public class BroadcastIntentActivity extends Activity {
@@ -28,5 +29,15 @@ protected void onCreate(Bundle b) {
         sendOrderedBroadcast(i,null);
         sendOrderedBroadcast(i,null,null,null,0,null,null);
         sendOrderedBroadcastAsUser(i,null,null,null,null,0,null,null);
+
+        /* These calls are safe.
+         *
+         * https://developer.android.com/reference/android/support/v4/content/LocalBroadcastManager.html
+         *      > "You know that the data you are broadcasting won't leave your app, so don't need to worry about leaking private data."
+         */
+        LocalBroadcastManager.getInstance(this).sendBroadcast(i);
+
+        // This class extends the Android LocalBroadcastManager and is used to test the InterfaceUtils.isSubtype condition.
+        CustomLocalBroadcastManager.getInstance(this).sendBroadcast(i);
     }
 }
diff --git a/plugin/src/test/java/testcode/android/CustomLocalBroadcastManager.java b/plugin/src/test/java/testcode/android/CustomLocalBroadcastManager.java
@@ -0,0 +1,8 @@
+package testcode.android;
+
+import android.support.v4.content.LocalBroadcastManager;
+
+/**
+ * This class is used in the BroadcastDetector to test the Interface.isSubtype([...],  "LocalBroadcastManager") condition.
+ */
+public class CustomLocalBroadcastManager extends LocalBroadcastManager { }