added predicates in the AndroidManifest library and adjusted tests

Author: Jami Cogswell

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

@@ -18,6 +18,16 @@ class AndroidManifestXmlFile extends XMLFile {
    * Gets the top-level `<manifest>` element in this Android manifest file.
    */
   AndroidManifestXmlElement getManifestElement() { result = this.getAChild() }
+
+  /**
+   * Holds if this Android manifest file is located in a build directory.
+   */
+  predicate isInBuildDirectory() {
+    exists(AndroidManifestXmlFile file |
+      file = this.getFile() and
+      file.getRelativePath().matches("%build%")
+    )
+  }
 }
 
 /**
@@ -51,6 +61,23 @@ class AndroidApplicationXmlElement extends XMLElement {
    * Gets a component child element of this `<application>` element.
    */
   AndroidComponentXmlElement getAComponentElement() { result = this.getAChild() }
+
+  /**
+   * Holds if this application element has the attribute `android:debuggable` set to `true`.
+   */
+  predicate isDebuggable() {
+    exists(AndroidXmlAttribute attr |
+      this.getAnAttribute() = attr and
+      attr.getName() = "debuggable" and
+      attr.getValue() = "true"
+    )
+  }
+
+  /**
+   * Overrides the getFile() predicate of the XMLElement class to get the
+   * AndroidManifest.xml file itself.
+   */
+  override AndroidManifestXmlFile getFile() { result = super.getFile() }
 }
 
 /**

java/ql/src/Security/CWE/CWE-489/DebuggableAttributeEnabled.ql

@@ -13,9 +13,8 @@
 import java
 import semmle.code.xml.AndroidManifest
 
-from AndroidXmlAttribute androidXmlAttr
+from AndroidApplicationXmlElement androidAppElem
 where
-  androidXmlAttr.getName() = "debuggable" and
-  androidXmlAttr.getValue() = "true" and
-  not androidXmlAttr.getLocation().getFile().getRelativePath().matches("%build%")
-select androidXmlAttr, "The 'android:debuggable' attribute is enabled."
+  androidAppElem.isDebuggable() and
+  not androidAppElem.getFile().isInBuildDirectory()
+select androidAppElem.getAttribute("debuggable"), "The 'android:debuggable' attribute is enabled."

java/ql/test/query-tests/security/CWE-489/TestTrue.xml renamed to java/ql/test/query-tests/security/CWE-489/AndroidManifest.xml

@@ -2,20 +2,19 @@ import java
 import semmle.code.xml.AndroidManifest
 import TestUtilities.InlineExpectationsTest
 
-class DebuggableAttributeTrueTest extends InlineExpectationsTest {
-  DebuggableAttributeTrueTest() { this = "DebuggableAttributeEnabledTest" }
+class DebuggableAttributeEnabledTest extends InlineExpectationsTest {
+  DebuggableAttributeEnabledTest() { this = "DebuggableAttributeEnabledTest" }
 
   override string getARelevantTag() { result = "hasDebuggableAttributeEnabled" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasDebuggableAttributeEnabled" and
-    exists(AndroidXmlAttribute androidXmlAttr |
-      androidXmlAttr.getName() = "debuggable" and
-      androidXmlAttr.getValue() = "true" and
-      not androidXmlAttr.getLocation().getFile().getRelativePath().matches("%build%")
+    exists(AndroidApplicationXmlElement androidAppElem |
+      androidAppElem.isDebuggable() and
+      not androidAppElem.getFile().isInBuildDirectory()
     |
-      androidXmlAttr.getLocation() = location and
-      element = androidXmlAttr.toString() and
+      androidAppElem.getAttribute("debuggable").getLocation() = location and
+      element = androidAppElem.getAttribute("debuggable").toString() and
       value = ""
     )
   }

java/ql/test/query-tests/security/CWE-489/DebuggableAttributeEnabledTest.ql

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    package="com.example.happybirthday">
+
+    <!-- $ hasDebuggableAttributeEnabled --> <application
+        android:debuggable="true"
+        android:allowBackup="true"
+        android:dataExtractionRules="@xml/data_extraction_rules"
+        android:fullBackupContent="@xml/backup_rules"
+        android:icon="@mipmap/ic_launcher"
+        android:label="@string/app_name"
+        android:roundIcon="@mipmap/ic_launcher_round"
+        android:supportsRtl="true"
+        android:theme="@style/Theme.HappyBirthday"
+        tools:targetApi="31">
+        <activity
+            android:name=".MainActivity"
+            android:exported="true">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>

java/ql/test/query-tests/security/CWE-489/TestFalse.xml renamed to java/ql/test/query-tests/security/CWE-489/TestFalse/AndroidManifest.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="utf-8"?>
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
+    package="com.example.happybirthday">
+
+    <!-- Safe: manifest file located in build directory --> <application
+        android:debuggable="true"
+        android:allowBackup="true"
+        android:dataExtractionRules="@xml/data_extraction_rules"
+        android:fullBackupContent="@xml/backup_rules"
+        android:icon="@mipmap/ic_launcher"
+        android:label="@string/app_name"
+        android:roundIcon="@mipmap/ic_launcher_round"
+        android:supportsRtl="true"
+        android:theme="@style/Theme.HappyBirthday"
+        tools:targetApi="31">
+        <activity
+            android:name=".MainActivity"
+            android:exported="true">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>