diff --git a/java/ql/lib/change-notes/2022-08-09-android-manifest-new-class-and-predicates.md b/java/ql/lib/change-notes/2022-08-09-android-manifest-new-class-and-predicates.md new file mode 100644 index 00000000000..95a1d8997cb --- /dev/null +++ b/java/ql/lib/change-notes/2022-08-09-android-manifest-new-class-and-predicates.md @@ -0,0 +1,8 @@ +--- +category: feature +--- +* 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. +* Added a new predicate, `hasAnIntentFilterElement`, in the `AndroidComponentXmlElement` class to detect if a component contains an intent filter element. +* Added a new predicate, `hasExportedAttribute`, in the `AndroidComponentXmlElement` class to detect if a component has an `android:exported` attribute. +* Added a new class, `AndroidCategoryXmlElement`, to represent a category element in an Android manifest file. +* Added a new predicate, `getACategoryElement`, in the `AndroidIntentFilterXmlElement` class to get a category element of an intent filter. diff --git a/java/ql/lib/semmle/code/java/security/ImplicitlyExportedAndroidComponent.qll b/java/ql/lib/semmle/code/java/security/ImplicitlyExportedAndroidComponent.qll new file mode 100644 index 00000000000..4aa21c4a260 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/ImplicitlyExportedAndroidComponent.qll @@ -0,0 +1,27 @@ +/** Provides a class to identify implicitly exported Android components. */ + +private import semmle.code.xml.AndroidManifest + +/** + * An Android component without an `exported` attribute explicitly set + * that also has an `intent-filter` element. + */ +class ImplicitlyExportedAndroidComponent extends AndroidComponentXmlElement { + ImplicitlyExportedAndroidComponent() { + this.hasAnIntentFilterElement() and + not this.hasExportedAttribute() and + // Components with category LAUNCHER or with action MAIN need to be exported since + // they are entry-points to the application. As a result, we do not consider these + // components to be implicitly exported since the developer intends them to be exported anyways. + not this.getAnIntentFilterElement().getACategoryElement().getCategoryName() = + "android.intent.category.LAUNCHER" and + not this.getAnIntentFilterElement().getAnActionElement().getActionName() = + "android.intent.action.MAIN" and + // The `permission` attribute can be used to limit components' exposure to other applications. + // As a result, we do not consider components with an explicitly set `permission` attribute to be + // implicitly exported since the developer has already limited access to such components. + not this.requiresPermissions() and + not this.getParent().(AndroidApplicationXmlElement).requiresPermissions() and + not this.getFile().(AndroidManifestXmlFile).isInBuildDirectory() + } +} diff --git a/java/ql/lib/semmle/code/xml/AndroidManifest.qll b/java/ql/lib/semmle/code/xml/AndroidManifest.qll index 7d414cabe63..b5c79efa853 100644 --- a/java/ql/lib/semmle/code/xml/AndroidManifest.qll +++ b/java/ql/lib/semmle/code/xml/AndroidManifest.qll @@ -67,6 +67,11 @@ class AndroidApplicationXmlElement extends XmlElement { attr.getValue() = "true" ) } + + /** + * Holds if this application element has explicitly set a value for its `android:permission` attribute. + */ + predicate requiresPermissions() { this.getAnAttribute().(AndroidPermissionXmlAttribute).isFull() } } /** @@ -108,7 +113,7 @@ class AndroidProviderXmlElement extends AndroidComponentXmlElement { * `android:permission` attribute or its `android:readPermission` and `android:writePermission` * attributes. */ - predicate requiresPermissions() { + override predicate requiresPermissions() { this.getAnAttribute().(AndroidPermissionXmlAttribute).isFull() or this.getAnAttribute().(AndroidPermissionXmlAttribute).isWrite() and @@ -170,6 +175,11 @@ class AndroidComponentXmlElement extends XmlElement { */ AndroidIntentFilterXmlElement getAnIntentFilterElement() { result = this.getAChild() } + /** + * Holds if this component element has an `` child element. + */ + predicate hasAnIntentFilterElement() { exists(this.getAnIntentFilterElement()) } + /** * Gets the value of the `android:name` attribute of this component element. */ @@ -220,6 +230,16 @@ class AndroidComponentXmlElement extends XmlElement { * Holds if the `android:exported` attribute of this component element is explicitly set to `false`. */ predicate isNotExported() { this.getExportedAttributeValue() = "false" } + + /** + * Holds if this component element has an `android:exported` attribute. + */ + predicate hasExportedAttribute() { exists(this.getExportedAttributeValue()) } + + /** + * Holds if this component element has explicitly set a value for its `android:permission` attribute. + */ + predicate requiresPermissions() { this.getAnAttribute().(AndroidPermissionXmlAttribute).isFull() } } /** @@ -234,6 +254,11 @@ class AndroidIntentFilterXmlElement extends XmlElement { * Gets an `` child element of this `` element. */ AndroidActionXmlElement getAnActionElement() { result = this.getAChild() } + + /** + * Gets a `` child element of this `` element. + */ + AndroidCategoryXmlElement getACategoryElement() { result = this.getAChild() } } /** @@ -257,3 +282,25 @@ class AndroidActionXmlElement extends XmlElement { ) } } + +/** + * A `` element in an Android manifest file. + */ +class AndroidCategoryXmlElement extends XmlElement { + AndroidCategoryXmlElement() { + this.getFile() instanceof AndroidManifestXmlFile and this.getName() = "category" + } + + /** + * Gets the name of this category. + */ + string getCategoryName() { + exists(XmlAttribute attr | + attr = this.getAnAttribute() and + attr.getNamespace().getPrefix() = "android" and + attr.getName() = "name" + | + result = attr.getValue() + ) + } +} diff --git a/java/ql/src/Security/CWE/CWE-926/ExampleBad.xml b/java/ql/src/Security/CWE/CWE-926/ExampleBad.xml new file mode 100644 index 00000000000..ec60406c460 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-926/ExampleBad.xml @@ -0,0 +1,11 @@ + + + + android:name=".Activity"> + + + + + + diff --git a/java/ql/src/Security/CWE/CWE-926/ExampleGood.xml b/java/ql/src/Security/CWE/CWE-926/ExampleGood.xml new file mode 100644 index 00000000000..f19184b8c13 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-926/ExampleGood.xml @@ -0,0 +1,12 @@ + + + + android:name=".Activity"> + android:exported="false" + + + + + + diff --git a/java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.qhelp b/java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.qhelp new file mode 100644 index 00000000000..f9efb0d631b --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.qhelp @@ -0,0 +1,55 @@ + + + + +

The Android manifest file defines configuration settings for Android applications. +In this file, components can be declared with intent filters which specify what the components can do and what types +of intents the components can respond to. If the android:exported attribute is omitted from the component +when an intent filter is included, then the component will be implicitly exported.

+ +

An implicitly exported component could allow for improper access to the component and its data.

+ + + + +

Explicitly set the android:exported attribute for every component or use permissions to limit access to the component.

+ + + + +

In the example below, the android:exported attribute is omitted when an intent filter is used.

+ + + +

A corrected version sets the android:exported attribute to false.

+ + + + + + +
  • + Android Developers: + App Manifest Overview. +
    • +
  • + Android Developers: + The <intent-filter> element. +
    • +
  • + Android Developers: + The android:exported attribute. +
    • +
  • + Android Developers: + The android:permission attribute. +
    • +
  • + Android Developers: + Safer component exporting. +
    • + +     +     diff --git a/java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.ql b/java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.ql new file mode 100644 index 00000000000..9796a3f59e2 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-926/ImplicitlyExportedAndroidComponent.ql @@ -0,0 +1,17 @@ +/** + * @name Implicitly exported Android component + * @description Android components with an '' and no 'android:exported' attribute are implicitly exported, which can allow for improper access to the components themselves and to their data. + * @kind problem + * @problem.severity warning + * @security-severity 8.2 + * @id java/android/implicitly-exported-component + * @tags security + * external/cwe/cwe-926 + * @precision high + */ + +import java +import semmle.code.java.security.ImplicitlyExportedAndroidComponent + +from ImplicitlyExportedAndroidComponent impExpAndroidComp +select impExpAndroidComp, "This component is implicitly exported." diff --git a/java/ql/src/change-notes/2022-08-09-android-implicit-export.md b/java/ql/src/change-notes/2022-08-09-android-implicit-export.md new file mode 100644 index 00000000000..beea9a8d3bf --- /dev/null +++ b/java/ql/src/change-notes/2022-08-09-android-implicit-export.md @@ -0,0 +1,4 @@ +--- +category: newQuery +--- +* Added a new query, `java/android/implicitly-exported-component`, to detect if components are implicitly exported in the Android manifest. diff --git a/java/ql/test/query-tests/security/CWE-926/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-926/AndroidManifest.xml new file mode 100644 index 00000000000..210c97b26a2 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-926/AndroidManifest.xml @@ -0,0 +1,114 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java/ql/test/query-tests/security/CWE-926/ImplicitlyExportedAndroidComponentTest.expected b/java/ql/test/query-tests/security/CWE-926/ImplicitlyExportedAndroidComponentTest.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/java/ql/test/query-tests/security/CWE-926/ImplicitlyExportedAndroidComponentTest.ql b/java/ql/test/query-tests/security/CWE-926/ImplicitlyExportedAndroidComponentTest.ql new file mode 100644 index 00000000000..fbda52d36ab --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-926/ImplicitlyExportedAndroidComponentTest.ql @@ -0,0 +1,18 @@ +import java +import semmle.code.java.security.ImplicitlyExportedAndroidComponent +import TestUtilities.InlineExpectationsTest + +class ImplicitlyExportedAndroidComponentTest extends InlineExpectationsTest { + ImplicitlyExportedAndroidComponentTest() { this = "ImplicitlyExportedAndroidComponentTest" } + + override string getARelevantTag() { result = "hasImplicitExport" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasImplicitExport" and + exists(ImplicitlyExportedAndroidComponent impExpAndroidComp | + impExpAndroidComp.getLocation() = location and + element = impExpAndroidComp.toString() and + value = "" + ) + } +} diff --git a/java/ql/test/query-tests/security/CWE-926/Test.java b/java/ql/test/query-tests/security/CWE-926/Test.java new file mode 100644 index 00000000000..4566fbca2ad --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-926/Test.java @@ -0,0 +1,3 @@ +public class Test { + +} diff --git a/java/ql/test/query-tests/security/CWE-926/TestApplicationPermission/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-926/TestApplicationPermission/AndroidManifest.xml new file mode 100644 index 00000000000..de229fb9be9 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-926/TestApplicationPermission/AndroidManifest.xml @@ -0,0 +1,108 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java/ql/test/query-tests/security/CWE-926/Testbuild/AndroidManifest.xml b/java/ql/test/query-tests/security/CWE-926/Testbuild/AndroidManifest.xml new file mode 100644 index 00000000000..336734a565f --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-926/Testbuild/AndroidManifest.xml @@ -0,0 +1,107 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/java/ql/test/query-tests/security/CWE-926/options b/java/ql/test/query-tests/security/CWE-926/options new file mode 100644 index 00000000000..dacd3cb21df --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-926/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/google-android-9.0.0