False positive rule xml:S5594: "Implement permissions on this exported component" for the main Android activity

Product: SonarCloud

Latest version of SonarCloud requires to set permission on any exported component, even if it’s a main activity with intent-filter action android.intent.action.MAIN and category android.intent.category.LAUNCHER, and that looks odd. For example:

https://sonarcloud.io/project/issues?id=vkgeo_vkgeo-android&open=AXe_HSzOHw0diHznDSpf

I think that’s a false positive, because such activity is intended to be launched by 3rd parties by design.

2 Likes

Yeah, thats definitely a false positiv!
All Activities with android.intent.action.VIEW in their intent-filter are intended to be launched from other applications without any special permissions. For example to allow app-links.

The Compliant Solution states that you have to define a (custom) permissions. But from the official Google documentation:

Creating a new permission is relatively uncommon for most applications, because the system-defined permissions cover many situations.

@oleg-derevenetz it does seem that the rule could be enhanced to allow no permissions on activities with the action of android.intent.action.MAIN defined. Seems like a good idea to have this check sense that’s how MAIN is intended to be used. That being said defining the permission as empty string resolves the issues and explicitly shows your intent that there are no permissions needed to launch this activity.

<activity
    . . .
    android:exported="true"
    android:permission="" >

@G00fY2 one might argue that an application with multiple entry points is hard to secure and this check gives you pause to consider this. Of course it could very well be that your application’s use case is to have multiple entry points (more power to ya), in that case see above.

The solution above is arguably hackey, but it is explicit.

1 Like

Hello everyone and thank you for your feedback!

@Josh_Nunez , your idea is certainly interesting.
@oleg-derevenetz & @G00fY2 , is this a solution that would be acceptable for you or do you have another idea in mind?

Hi @Hendrik_Buchwald,

Yes, I think that exception for activities with the action of android.intent.action.MAIN should be enough.

Thank you!

@Hendrik_Buchwald I agree. Activities with android.intent.action.MAIN should be excluded.

For activities with android.intent.action.VIEW I personally see no usecase where you want to define permissions. But as @Josh_Nunez said, it raises your awareness and is therefore fine.

I wonder whether you will also add a rule for the latest changes Google added in Android 12:
https://developer.android.com/about/versions/12/behavior-changes-12#exported

This is a similar security risk but one that Google actually forces us to explicitly handle.

Ticket created, SONARXML-113, to exclude activities with android.intent.action.MAIN.

@G00fY2 about Android 12, indeed it could be a new rule, we’ll think about it very soon. Thank you for the idea.

Eric

2 Likes

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.