search

airbnb / Showkase

🔦 Showkase is an annotation-processor based Android library that helps you organize, discover, search and visualize Jetpack Compose UI elements
https://medium.com/airbnb-engineering/introducing-showkase-a-library-to-organize-discover-and-visualize-your-jetpack-compose-elements-d5c34ef01095
Apache License 2.0
2.08k stars 106 forks source link

exported ShowkaseBrowserActivity #347

Closed Skeletonxf closed 10 months ago

Skeletonxf commented 10 months ago

Hi, as part of an automated security scan on an app I'm working on Showkase was flagged due to the Showkase browser activity being exported by default.

Issue Severity Description
Activity (com.airbnb.android.showkase.ui.ShowkaseBrowserActivity) is not Protected. [android:exported=true] high An Activity is found to be shared with other apps on the device therefore leaving it accessible to any other application on the device.

This is easily overridden app side via

<activity
    android:name="com.airbnb.android.showkase.ui.ShowkaseBrowserActivity"
    android:exported="false"
    tools:replace="android:exported" >
</activity>

however the default being set to true could be surprising for some users of this library? I had initially thought that if I disabled the in app means of launching the browser then it wouldn't be accessible in a production build of the app, but given it is exported by default another malicious app could theoretically open this, and perhaps gain access to data in the screenshots that they shouldn't if anything sensitive is there.

rovkinmax commented 10 months ago

I bealeve that ShowkaseBrowserActivity should not be included in a release build

vinaygaba commented 10 months ago

@Skeletonxf Thanks for flagging. I see that this line was added in this PR (https://github.com/airbnb/Showkase/commit/469e14de06227d32974d2b3fa67646ff53013e17#diff-fdcbc654693654c9611addef6b4e7ef93e1ae16aa06522b1c02d9786a3d5ecb5R10) and I didn't catch it during the review. I've reached out to @oas004 to figure out why this was needed, otherwise I'm inclined to remove this line of code from the library.

vinaygaba commented 10 months ago

Opened PR that fixes this - https://github.com/airbnb/Showkase/pull/355