Issue #224 - Added an exception for the LocalBroadcastManager in the detector. #225
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…the plugin-deps and added an exception for the LocalBroadcastManager in the detector.
h3xstream
requested changes
Oct 3, 2016
| getNameConstantOperand().equals("sendOrderedBroadcast") || | ||
| getNameConstantOperand().equals("sendOrderedBroadcastAsUser") | ||
| ) | ||
| && !getClassConstantOperand().endsWith("LocalBroadcastManager") // The LocalBroadcastManager object is safe. The broadcast doesn't leave the application scope. |
There was a problem hiding this comment.
I would use InterfaceUtils.isSubtype() has a main method to confirm that it is a LocalBroadcastManager.
The check !getClassConstantOperand().endsWith("LocalBroadcastManager") can still be present. It will be useful to have fallback when the complete class hierarchy can be loaded (aka missing classes during the scan).
| * 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); |
| @@ -0,0 +1,10 @@ | |||
| package android.content; | |||
|
|
|||
| public class LocalBroadcastManager { | |||
There was a problem hiding this comment.
I would add android.support.v4.content.LocalBroadcastManager type. It will be needed when an actual test on the class hierarchy is made. (See comment related to InterfaceUtils.isSubtype() below)
…to fit the changes requested in find-sec-bugs#225.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added a check in the BroadcastDetector to validate that the sendBroadcast call is not executed on the LocalBroadcastManager class. This should fix issue #224 .
Right now it could be fooled if someone calls its Activity LocalBroadcastManager and uses the global sendBroadcast method. I could change the check for something stricter like !"android/support/v4/content/LocalBroadcastManager".equals(getClassConstantOperand()) but this would have to be changed often since it contains the API version.