Skip to content

Commit ce740b4

Browse files
authored
Merge pull request github#10637 from egregius313/egregius313/android-misconfigured-contentprovider
Android ContentProvider Incomplete Permissions
2 parents 338ce83 + 80cc3fc commit ce740b4

13 files changed

+261
-0
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* Added a new predicate, `hasIncompletePermissions`, in the `AndroidProviderXmlElement` class. This predicate detects if a provider element does not provide both read and write permissions.

java/ql/lib/semmle/code/xml/AndroidManifest.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,17 @@ class AndroidProviderXmlElement extends AndroidComponentXmlElement {
180180
attr.getValue() = "true"
181181
)
182182
}
183+
184+
/**
185+
* Holds if the provider element is only protected by either `android:readPermission` or `android:writePermission`.
186+
*/
187+
predicate hasIncompletePermissions() {
188+
(
189+
this.getAnAttribute().(AndroidPermissionXmlAttribute).isWrite() or
190+
this.getAnAttribute().(AndroidPermissionXmlAttribute).isRead()
191+
) and
192+
not this.requiresPermissions()
193+
}
183194
}
184195

185196
/**
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The Android manifest file specifies the content providers for the application
7+
using <code>provider</code> elements. The <code>provider</code> element
8+
specifies the explicit permissions an application requires in order to access a
9+
resource using that provider.
10+
11+
You specify the permissions using
12+
the <code>android:readPermission</code>, <code>android:writePermission</code>,
13+
or <code>android:permission</code> attributes.
14+
If you do not specify the permission required to perform an operation, the application will implicitly have access to perform that operation.
15+
For example, if you specify only <code>android:readPermission</code>, the application must have explicit permission to read data, but requires no permission to write data.
16+
</p>
17+
18+
</overview>
19+
20+
<recommendation>
21+
<p>To prevent permission bypass, you should create <code>provider</code> elements that either
22+
specify both the <code>android:readPermission</code>
23+
and <code>android:writePermission</code> attributes, or specify
24+
the <code>android:permission</code> attribute.
25+
</p>
26+
</recommendation>
27+
28+
<example>
29+
30+
<p>In the following two (bad) examples, the provider is configured with only
31+
read or write permissions. This allows a malicious application to bypass the permission check by requesting access to the unrestricted operation.</p>
32+
33+
<sample src="ContentProviderIncompletePermissionsReadOnly.xml"/>
34+
35+
<sample src="ContentProviderIncompletePermissionsWriteOnly.xml"/>
36+
37+
<p>In the following (good) examples, the provider is configured with full permissions, protecting it from a permissions bypass.</p>
38+
39+
<sample src="ContentProviderIncompletePermissionsReadWrite.xml"/>
40+
41+
<sample src="ContentProviderIncompletePermissionsFull.xml"/>
42+
</example>
43+
44+
<references>
45+
<li>
46+
Android Documentation:
47+
<a href="https://developer.android.com/guide/topics/manifest/provider-element">Provider element</a>
48+
</li>
49+
<li>
50+
CVE-2021-41166: <a href="https://nvd.nist.gov/vuln/detail/CVE-2021-41166">Insufficient
51+
permission control in Nextcloud Android app</a>
52+
</li>
53+
<li>
54+
GitHub Security Lab Research:
55+
<a href="https://securitylab.github.com/advisories/GHSL-2021-1007-Nextcloud_Android_app/#issue-2-permission-bypass-in-disklruimagecachefileprovider-ghsl-2021-1008">Insufficient permission control in Nextcloud Android app</a>
56+
</li>
57+
</references>
58+
</qhelp>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Missing read or write permission in a content provider
3+
* @description Android content providers which do not configure both read and write permissions can allow permission bypass.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 8.2
7+
* @id java/android/incomplete-provider-permissions
8+
* @tags security
9+
* external/cwe/cwe-926
10+
* @precision medium
11+
*/
12+
13+
import java
14+
import semmle.code.xml.AndroidManifest
15+
16+
from AndroidProviderXmlElement provider
17+
where
18+
not provider.getFile().(AndroidManifestXmlFile).isInBuildDirectory() and
19+
provider.isExported() and
20+
provider.hasIncompletePermissions()
21+
select provider, "Exported provider has incomplete permissions."
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<manifest ... >
2+
<application ...>
3+
<!-- Good: 'android:permission' is set -->
4+
<provider
5+
android:name=".MyContentProvider"
6+
android:authorities="table"
7+
android:enabled="true"
8+
android:exported="true"
9+
android:permission="android.permission.MANAGE_DOCUMENTS">
10+
</provider>
11+
</application>
12+
</manifest>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<manifest ... >
2+
<application ...>
3+
<!-- BAD: only 'android:readPermission' is set -->
4+
<provider
5+
android:name=".MyContentProvider"
6+
android:authorities="table"
7+
android:enabled="true"
8+
android:exported="true"
9+
android:readPermission="android.permission.MANAGE_DOCUMENTS">
10+
</provider>
11+
</application>
12+
</manifest>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<manifest ... >
2+
<application ...>
3+
<!-- Good: both 'android:readPermission' and 'android:writePermission' are set -->
4+
<provider
5+
android:name=".MyContentProvider"
6+
android:authorities="table"
7+
android:enabled="true"
8+
android:exported="true"
9+
android:writePermission="android.permission.MANAGE_DOCUMENTS"
10+
android:readPermission="android.permission.MANAGE_DOCUMENTS">
11+
</provider>
12+
</application>
13+
</manifest>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<manifest ... >
2+
<application ...>
3+
<!-- BAD: only 'android:writePermission' is set -->
4+
<provider
5+
android:name=".MyContentProvider"
6+
android:authorities="table"
7+
android:enabled="true"
8+
android:exported="true"
9+
android:writePermission="android.permission.MANAGE_DOCUMENTS">
10+
</provider>
11+
</application>
12+
</manifest>
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `java/android/incomplete-provider-permissions`, to detect if an Android ContentProvider is not protected with a correct set of permissions.
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
3+
xmlns:tools="http://schemas.android.com/tools"
4+
package="com.example.myapplication">
5+
6+
<application
7+
android:allowBackup="false"
8+
android:dataExtractionRules="@xml/data_extraction_rules"
9+
android:fullBackupContent="@xml/backup_rules"
10+
android:icon="@mipmap/ic_launcher"
11+
android:label="@string/app_name"
12+
android:roundIcon="@mipmap/ic_launcher_round"
13+
android:supportsRtl="true"
14+
15+
android:theme="@style/Theme.MyApplication"
16+
tools:targetApi="31">
17+
18+
<!-- Read Only -->
19+
20+
<!-- $ hasIncompletePermissions --><provider
21+
android:name=".MyContentProviderRO"
22+
android:authorities="table"
23+
android:enabled="true"
24+
android:exported="true"
25+
android:readPermission="android.permission.MANAGE_DOCUMENTS"></provider>
26+
27+
28+
<!-- Write Only -->
29+
30+
<!-- $ hasIncompletePermissions --> <provider
31+
android:name=".MyContentProviderWO"
32+
android:authorities="table"
33+
android:enabled="true"
34+
android:exported="true"
35+
android:writePermission="android.permission.MANAGE_DOCUMENTS"></provider>
36+
37+
<!-- Full -->
38+
39+
<!-- Safe: provider has full permissions set --> <provider
40+
android:name=".MyContentProviderFull"
41+
android:authorities="morestuff"
42+
android:enabled="true"
43+
android:exported="true"
44+
android:permission="android.permission.MANAGE_DOCUMENTS"></provider>
45+
46+
<!-- Read-Write -->
47+
48+
<!-- Safe: has both read and write permission --><provider
49+
android:name=".MyContentProviderRW"
50+
android:authorities="table"
51+
android:enabled="true"
52+
android:exported="true"
53+
android:readPermission="android.permission.MANAGE_DOCUMENTS"
54+
android:writePermission="android.permission.MANAGE_DOCUMENTS"></provider>
55+
56+
57+
<!-- Not exported -->
58+
59+
<!-- Safe: provider not exported --> <provider
60+
android:name=".MyContentProviderNE"
61+
android:authorities="table"
62+
android:enabled="true"
63+
android:writePermission="android.permission.MANAGE_DOCUMENTS"></provider>
64+
65+
</application>
66+
67+
</manifest>

0 commit comments

Comments
 (0)