Merge pull request #16914 from owen-mc/java/android-app-detection

Java: Improve Android app detection

java/ql/lib/change-notes/2024-06-12-isandroid-deprecated.md

@@ -0,0 +1,4 @@
+---
+category: deprecated
+---
+* The predicate `isAndroid` from the module `semmle.code.java.security.AndroidCertificatePinningQuery` has been deprecated. Use `semmle.code.java.frameworks.android.Android::inAndroidApplication(File)` instead.

java/ql/lib/semmle/code/java/frameworks/android/Android.qll

@@ -5,6 +5,20 @@
 import java
 private import semmle.code.xml.AndroidManifest
 
+/**
+ * Holds if in `file`'s directory or some parent directory there is an `AndroidManifestXmlFile`
+ * that defines at least one activity, service or contest provider, suggesting this file is
+ * part of an android application.
+ */
+predicate inAndroidApplication(File file) {
+  file.isSourceFile() and
+  exists(AndroidManifestXmlFile amxf, Folder amxfDir |
+    amxf.definesAndroidApplication() and amxfDir = amxf.getParentContainer()
+  |
+    file.getParentContainer+() = amxfDir
+  )
+}
+
 /**
  * Gets a reflexive/transitive superType
  */

java/ql/lib/semmle/code/java/security/AndroidCertificatePinningQuery.qll

@@ -6,6 +6,7 @@ import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.frameworks.Networking
 import semmle.code.java.security.Encryption
 import semmle.code.java.security.HttpsUrls
+private import semmle.code.java.frameworks.android.Android
 
 /** An Android Network Security Configuration XML file. */
 class AndroidNetworkSecurityConfigFile extends XmlFile {
@@ -19,8 +20,12 @@ class AndroidNetworkSecurityConfigFile extends XmlFile {
   }
 }
 
-/** Holds if this database is of an Android application. */
-predicate isAndroid() { exists(AndroidManifestXmlFile m) }
+/**
+ * DEPRECATED. Use `semmle.code.java.frameworks.android.Android::inAndroidApplication` instead.
+ *
+ * Holds if this database contains an Android manifest file.
+ */
+deprecated predicate isAndroid() { exists(AndroidManifestXmlFile m) }
 
 /** Holds if the given domain name is trusted by the Network Security Configuration XML file. */
 private predicate trustedDomainViaXml(string domainName) {
@@ -122,7 +127,7 @@ private module UntrustedUrlFlow = TaintTracking::Global<UntrustedUrlConfig>;
 
 /** Holds if `node` is a network communication call for which certificate pinning is not implemented. */
 predicate missingPinning(MissingPinningSink node, string domain) {
-  isAndroid() and
+  inAndroidApplication(node.getLocation().getFile()) and
   exists(DataFlow::Node src | UntrustedUrlFlow::flow(src, node) |
     if trustedDomain(_) then domain = getDomain(src.asExpr()) else domain = ""
   )

java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll

@@ -6,16 +6,15 @@
 import java
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.security.CleartextStorageQuery
-import semmle.code.xml.AndroidManifest
 private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSinks
 private import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.frameworks.android.Android
 
 private class AndroidFilesystemCleartextStorageSink extends CleartextStorageSink {
   AndroidFilesystemCleartextStorageSink() {
     filesystemInput(_, this.asExpr()) and
-    // Make sure we are in an Android application.
-    exists(AndroidManifestXmlFile manifest)
+    inAndroidApplication(this.getLocation().getFile())
   }
 }
 

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

@@ -23,6 +23,21 @@ class AndroidManifestXmlFile extends XmlFile {
    * Holds if this Android manifest file is located in a build directory.
    */
   predicate isInBuildDirectory() { this.getFile().getRelativePath().matches("%build%") }
+
+  /**
+   * Holds if this file defines at least one activity, service or contest provider,
+   * and so it corresponds to an android application rather than a library.
+   */
+  predicate definesAndroidApplication() {
+    exists(AndroidComponentXmlElement acxe |
+      this.getManifestElement().getApplicationElement().getAComponentElement() = acxe and
+      (
+        acxe instanceof AndroidActivityXmlElement or
+        acxe instanceof AndroidServiceXmlElement or
+        acxe instanceof AndroidProviderXmlElement
+      )
+    )
+  }
 }
 
 /**

java/ql/src/change-notes/2024-07-07-android-application-heuristic-updated.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The heuristic to enable certain Android queries has been improved. Now it ignores Android Manifests which don't define an activity, content provider or service. We also only consider files which are under a folder containing such an Android Manifest for these queries. This should remove some false positive alerts.

java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test1/AndroidManifest.xml

@@ -5,6 +5,12 @@
     android:versionName="0.1" >
 
     <application android:networkSecurityConfig="@xml/NetworkSecurityConfig">
+        <activity android:name=".MainActivity">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
     </application>
 
-</manifest>
+</manifest>

java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test2/AndroidManifest.xml

@@ -5,6 +5,12 @@
     android:versionName="0.1" >
 
     <application android:networkSecurityConfig="@xml/NetworkSecurityConfig">
+        <activity android:name=".MainActivity">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
     </application>
 
-</manifest>
+</manifest>

java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test3/AndroidManifest.xml

@@ -5,6 +5,12 @@
     android:versionName="0.1" >
 
     <application android:networkSecurityConfig="@xml/NetworkSecurityConfig">
+        <activity android:name=".MainActivity">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
     </application>
 
-</manifest>
+</manifest>

java/ql/test/query-tests/security/CWE-295/AndroidMissingCertificatePinning/Test4/AndroidManifest.xml

@@ -5,6 +5,12 @@
     android:versionName="0.1" >
 
     <application android:networkSecurityConfig="@xml/NetworkSecurityConfig">
+        <activity android:name=".MainActivity">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
     </application>
 
-</manifest>
+</manifest>