Skip to content

Commit b3e88f8

Browse files
jcogs33jcogs33
authored
Merge pull request #9983 from jcogs33/android-implicit-export
Java: query to detect implicitly exported Android components
2 parents 610c788 + 0136c75 commit b3e88f8

15 files changed

+533
-1
lines changed

‎java/ql/lib/change-notes/2022-08-09-android-manifest-new-class-and-predicates.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
category: feature
3+
---
4+
* Added a new predicate, `requiresPermissions`, in the `AndroidComponentXmlElement` and `AndroidApplicationXmlElement` classes to detect if the element has explicitly set a value for its `android:permission` attribute.
5+
* Added a new predicate, `hasAnIntentFilterElement`, in the `AndroidComponentXmlElement` class to detect if a component contains an intent filter element.
6+
* Added a new predicate, `hasExportedAttribute`, in the `AndroidComponentXmlElement` class to detect if a component has an `android:exported` attribute.
7+
* Added a new class, `AndroidCategoryXmlElement`, to represent a category element in an Android manifest file.
8+
* Added a new predicate, `getACategoryElement`, in the `AndroidIntentFilterXmlElement` class to get a category element of an intent filter.

‎java/ql/lib/semmle/code/java/security/ImplicitlyExportedAndroidComponent.qll

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/** Provides a class to identify implicitly exported Android components. */
2+
3+
private import semmle.code.xml.AndroidManifest
4+
5+
/**
6+
* An Android component without an `exported` attribute explicitly set
7+
* that also has an `intent-filter` element.
8+
*/
9+
class ImplicitlyExportedAndroidComponent extends AndroidComponentXmlElement {
10+
ImplicitlyExportedAndroidComponent() {
11+
this.hasAnIntentFilterElement() and
12+
not this.hasExportedAttribute() and
13+
// Components with category LAUNCHER or with action MAIN need to be exported since
14+
// they are entry-points to the application. As a result, we do not consider these
15+
// components to be implicitly exported since the developer intends them to be exported anyways.
16+
not this.getAnIntentFilterElement().getACategoryElement().getCategoryName() =
17+
"android.intent.category.LAUNCHER" and
18+
not this.getAnIntentFilterElement().getAnActionElement().getActionName() =
19+
"android.intent.action.MAIN" and
20+
// The `permission` attribute can be used to limit components' exposure to other applications.
21+
// As a result, we do not consider components with an explicitly set `permission` attribute to be
22+
// implicitly exported since the developer has already limited access to such components.
23+
not this.requiresPermissions() and
24+
not this.getParent().(AndroidApplicationXmlElement).requiresPermissions() and
25+
not this.getFile().(AndroidManifestXmlFile).isInBuildDirectory()
26+
}
27+
}

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ class AndroidApplicationXmlElement extends XmlElement {
6767
attr.getValue() = "true"
6868
)
6969
}
70+
71+
/**
72+
* Holds if this application element has explicitly set a value for its `android:permission` attribute.
73+
*/
74+
predicate requiresPermissions() { this.getAnAttribute().(AndroidPermissionXmlAttribute).isFull() }
7075
}
7176

7277
/**
@@ -108,7 +113,7 @@ class AndroidProviderXmlElement extends AndroidComponentXmlElement {
108113
* `android:permission` attribute or its `android:readPermission` and `android:writePermission`
109114
* attributes.
110115
*/
111-
predicate requiresPermissions() {
116+
override predicate requiresPermissions() {
112117
this.getAnAttribute().(AndroidPermissionXmlAttribute).isFull()
113118
or
114119
this.getAnAttribute().(AndroidPermissionXmlAttribute).isWrite() and
@@ -170,6 +175,11 @@ class AndroidComponentXmlElement extends XmlElement {
170175
*/
171176
AndroidIntentFilterXmlElement getAnIntentFilterElement() { result = this.getAChild() }
172177

178+
/**
179+
* Holds if this component element has an `<intent-filter>` child element.
180+
*/
181+
predicate hasAnIntentFilterElement() { exists(this.getAnIntentFilterElement()) }
182+
173183
/**
174184
* Gets the value of the `android:name` attribute of this component element.
175185
*/
@@ -220,6 +230,16 @@ class AndroidComponentXmlElement extends XmlElement {
220230
* Holds if the `android:exported` attribute of this component element is explicitly set to `false`.
221231
*/
222232
predicate isNotExported() { this.getExportedAttributeValue() = "false" }
233+
234+
/**
235+
* Holds if this component element has an `android:exported` attribute.
236+
*/
237+
predicate hasExportedAttribute() { exists(this.getExportedAttributeValue()) }
238+
239+
/**
240+
* Holds if this component element has explicitly set a value for its `android:permission` attribute.
241+
*/
242+
predicate requiresPermissions() { this.getAnAttribute().(AndroidPermissionXmlAttribute).isFull() }
223243
}
224244

225245
/**
@@ -234,6 +254,11 @@ class AndroidIntentFilterXmlElement extends XmlElement {
234254
* Gets an `<action>` child element of this `<intent-filter>` element.
235255
*/
236256
AndroidActionXmlElement getAnActionElement() { result = this.getAChild() }
257+
258+
/**
259+
* Gets a `<category>` child element of this `<intent-filter>` element.
260+
*/
261+
AndroidCategoryXmlElement getACategoryElement() { result = this.getAChild() }
237262
}
238263

239264
/**
@@ -257,3 +282,25 @@ class AndroidActionXmlElement extends XmlElement {
257282
)
258283
}
259284
}
285+
286+
/**
287+
* A `<category>` element in an Android manifest file.
288+
*/
289+
class AndroidCategoryXmlElement extends XmlElement {
290+
AndroidCategoryXmlElement() {
291+
this.getFile() instanceof AndroidManifestXmlFile and this.getName() = "category"
292+
}
293+
294+
/**
295+
* Gets the name of this category.
296+
*/
297+
string getCategoryName() {
298+
exists(XmlAttribute attr |
299+
attr = this.getAnAttribute() and
300+
attr.getNamespace().getPrefix() = "android" and
301+
attr.getName() = "name"
302+
|
303+
result = attr.getValue()
304+
)
305+
}
306+
}

‎java/ql/src/Security/CWE/CWE-926/ExampleBad.xml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<manifest ... >
2+
<application ...
3+
<!-- BAD: this component is implicitly exported -->
4+
<activity>
5+
android:name=".Activity">
6+
<intent-filter>
7+
<action android:name="android.intent.action.VIEW" />
8+
</intent-filter>
9+
</activity>
10+
</application>
11+
</manifest>

‎java/ql/src/Security/CWE/CWE-926/ExampleGood.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<manifest ... >
2+
<application ...
3+
<!-- GOOD: this component is not exported due to 'android:exported' explicitly set to 'false'-->
4+
<activity>
5+
android:name=".Activity">
6+
android:exported="false"
7+
<intent-filter>
8+
<action android:name="android.intent.action.VIEW" />
9+
</intent-filter>
10+
</activity>
11+
</application>
12+
</manifest>

‎java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.qhelp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>The Android manifest file defines configuration settings for Android applications.
8+
In this file, components can be declared with intent filters which specify what the components can do and what types
9+
of intents the components can respond to. If the <code>android:exported</code> attribute is omitted from the component
10+
when an intent filter is included, then the component will be implicitly exported.</p>
11+
12+
<p>An implicitly exported component could allow for improper access to the component and its data.</p>
13+
14+
</overview>
15+
<recommendation>
16+
17+
<p>Explicitly set the <code>android:exported</code> attribute for every component or use permissions to limit access to the component.</p>
18+
19+
</recommendation>
20+
<example>
21+
22+
<p>In the example below, the <code>android:exported</code> attribute is omitted when an intent filter is used.</p>
23+
24+
<sample src="ExampleBad.xml" />
25+
26+
<p>A corrected version sets the <code>android:exported</code> attribute to <code>false</code>.</p>
27+
28+
<sample src="ExampleGood.xml" />
29+
30+
</example>
31+
<references>
32+
33+
<li>
34+
Android Developers:
35+
<a href="https://developer.android.com/guide/topics/manifest/manifest-intro">App Manifest Overview</a>.
36+
</li>
37+
<li>
38+
Android Developers:
39+
<a href="https://developer.android.com/guide/topics/manifest/intent-filter-element">The &lt;intent-filter&gt; element</a>.
40+
</li>
41+
<li>
42+
Android Developers:
43+
<a href="https://developer.android.com/guide/topics/manifest/activity-element#exported">The android:exported attribute</a>.
44+
</li>
45+
<li>
46+
Android Developers:
47+
<a href="https://developer.android.com/guide/topics/manifest/activity-element#prmsn">The android:permission attribute</a>.
48+
</li>
49+
<li>
50+
Android Developers:
51+
<a href="https://developer.android.com/about/versions/12/behavior-changes-12#exported">Safer component exporting</a>.
52+
</li>
53+
54+
</references>
55+
</qhelp>

‎java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.ql

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* @name Implicitly exported Android component
3+
* @description Android components with an '<intent-filter>' and no 'android:exported' attribute are implicitly exported, which can allow for improper access to the components themselves and to their data.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @security-severity 8.2
7+
* @id java/android/implicitly-exported-component
8+
* @tags security
9+
* external/cwe/cwe-926
10+
* @precision high
11+
*/
12+
13+
import java
14+
import semmle.code.java.security.ImplicitlyExportedAndroidComponent
15+
16+
from ImplicitlyExportedAndroidComponent impExpAndroidComp
17+
select impExpAndroidComp, "This component is implicitly exported."

‎java/ql/src/change-notes/2022-08-09-android-implicit-export.md

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/implicitly-exported-component`, to detect if components are implicitly exported in the Android manifest.

‎java/ql/test/query-tests/security/CWE-926/AndroidManifest.xml

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
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.happybirthday">
5+
6+
<application
7+
android:allowBackup="true"
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+
android:theme="@style/Theme.HappyBirthday"
15+
tools:targetApi="31">
16+
17+
<!-- $ hasImplicitExport --> <activity
18+
android:name=".Activity">
19+
<intent-filter>
20+
<action android:name="android.intent.action.VIEW" />
21+
</intent-filter>
22+
</activity>
23+
24+
<!-- $ hasImplicitExport --> <receiver
25+
android:name=".CheckInstall">
26+
<intent-filter>
27+
<action android:name="android.intent.action.PACKAGE_INSTALL"/>
28+
29+
</intent-filter>
30+
</receiver>
31+
32+
<!-- $ hasImplicitExport --> <service
33+
android:name=".backgroundService">
34+
<intent-filter>
35+
<action android:name="android.intent.action.START_BACKGROUND"/>
36+
37+
</intent-filter>
38+
</service>
39+
40+
<!-- $ hasImplicitExport --> <provider
41+
android:name=".MyCloudProvider">
42+
<intent-filter>
43+
<action android:name="android.intent.action.DOCUMENTS_PROVIDER"/>
44+
45+
</intent-filter>
46+
</provider>
47+
48+
<!-- Safe: 'android:exported' explicitly set --> <activity
49+
android:name=".Activity"
50+
android:exported="true">
51+
<intent-filter>
52+
<action android:name="android.intent.action.VIEW" />
53+
</intent-filter>
54+
</activity>
55+
56+
<!-- Safe: no intent filter --> <activity
57+
android:name=".Activity">
58+
</activity>
59+
60+
<!-- Safe: has 'permission' attribute --> <activity
61+
android:name=".Activity"
62+
android:permission=".Test">
63+
<intent-filter>
64+
<action android:name="android.intent.action.VIEW" />
65+
</intent-filter>
66+
</activity>
67+
68+
<!-- Safe: 'provider' with read and write permissions set --> <provider
69+
android:name=".MyCloudProvider"
70+
android:readPermission=".TestRead"
71+
android:writePermission=".TestWrite">
72+
<intent-filter>
73+
<action android:name="android.intent.action.DOCUMENTS_PROVIDER"/>
74+
75+
</intent-filter>
76+
</provider>
77+
78+
<!-- $ hasImplicitExport --> <provider
79+
android:name=".MyCloudProvider"
80+
android:readPermission=".TestRead">
81+
<intent-filter>
82+
<action android:name="android.intent.action.DOCUMENTS_PROVIDER"/>
83+
84+
</intent-filter>
85+
</provider>
86+
87+
<!-- $ hasImplicitExport --> <provider
88+
android:name=".MyCloudProvider"
89+
android:writePermission=".TestWrite">
90+
<intent-filter>
91+
<action android:name="android.intent.action.DOCUMENTS_PROVIDER"/>
92+
93+
</intent-filter>
94+
</provider>
95+
96+
<!-- Safe: has category 'android.intent.category.LAUNCHER' --> <activity
97+
android:name=".Activity">
98+
<intent-filter>
99+
<action android:name="android.intent.action.MAIN" />
100+
101+
<category android:name="android.intent.category.LAUNCHER" />
102+
</intent-filter>
103+
</activity>
104+
105+
<!-- Safe: has action 'android.intent.category.MAIN' --> <activity
106+
android:name=".Activity">
107+
<intent-filter>
108+
<action android:name="android.intent.action.MAIN" />
109+
</intent-filter>
110+
</activity>
111+
112+
</application>
113+
114+
</manifest>

‎java/ql/test/query-tests/security/CWE-926/ImplicitlyExportedAndroidComponentTest.expected

Whitespace-only changes.

0 commit comments

Comments
 (0)