Issue metadata
Sign in to add a comment
|
Security: Chrome OS drive and downloads exposed to arbitrary Android apps |
||||||||||||||||||||
Issue description
org.chromium.arc.file_system.ChromeContentProvider has following definition:
<provider
android:name=".ChromeContentProvider"
android:authorities="org.chromium.arc.chromecontentprovider"
android:grantUriPermissions="true"
android:exported="true"
/>
This definition lacks any permission requirement (android:grantUriPermissions is not one), so any app can access this content provider even if it does not have storage permissions.
For example, an app can read
content://org.chromium.arc.chromecontentprovider/externalfile:drive-0123456789abcdef/root/secret.pdf
to read "secret.pdf" on Google Drive.
Note that although URL contains user hash (0123456789abcdef), it can be scraped from /proc/self/mountinfo. I have a PoC code that does this.
The fix would be to set android:exported="false" or android:permission.
,
Mar 7 2018
,
Mar 8 2018
I thought this won't result in allowing all apps to access because Android's documentation says "If a provider's application doesn't specify any permissions, then other applications have no access to the provider's data." https://developer.android.com/guide/topics/providers/content-provider-basics.html#Permissions Also, doesn't exported="false" result in prohibiting all access (incl. the intended use case)? Could you share the repro code?
,
Mar 8 2018
hashimoto: That document does not match with my local observation... Result of reading ChromeContentProvider URL changes by alternating android:exported. I've verified setting exported=false does not break ChromeContentProvider functionality. It looks grantUriPermission supersedes exported. I'm attaching the repro code.
,
Mar 8 2018
and here is an apk. it attempts to read a file named hypercat.jpg on Drive and Downloads.
,
Mar 8 2018
Thanks for the explanation. Android's documentation about application manifests says nothing about android:grantUriPermission superseding android:exported, but if it's the current behavior, it looks like the simplest solution. https://developer.android.com/guide/topics/manifest/provider-element.html
,
Mar 8 2018
The fix has landed. Security team, could you please guide us how to proceed with cherry-picks?
,
Mar 8 2018
We should consider merging to 66, which is still in dev. The process is basically: -Add Merge-Request flag. -CC the TPM for the release where you want to merge (info at https://chromiumdash.appspot.com/schedule) 66 TPM is josafat@.
,
Mar 8 2018
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 9 2018
Okay, so security bugs also follow the standard procedure of cherry-picks. Thanks Jorge!
,
Mar 9 2018
,
Mar 9 2018
,
Mar 9 2018
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 12 2018
Fix merged to nyc-mr1-arc-m66 branch: https://googleplex-android-review.git.corp.google.com/c/platform/vendor/google_arc/+/3718393
,
Mar 13 2018
Thanks everyone for your help. Do I need to do anything else?
,
Mar 13 2018
I think this is complete being marked as Fixed and merged.
,
Mar 13 2018
Got it, thanks!
,
Jun 15 2018
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by sheriffbot@chromium.org
, Mar 7 2018