From c69a2be9767b012bfd578308406e8856bb7b5a35 Mon Sep 17 00:00:00 2001
From: Ed Minnix <egregius313@github.com>
Date: Wed, 7 Sep 2022 12:08:25 -0400
Subject: [PATCH] Moved allowBackup query logic to allowsBackup pred

---
 .../lib/semmle/code/xml/AndroidManifest.qll   | 22 ++++++++++++++-----
 .../CWE-312/AllowBackupAttributeEnabled.ql    | 12 +---------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/java/ql/lib/semmle/code/xml/AndroidManifest.qll b/java/ql/lib/semmle/code/xml/AndroidManifest.qll
index 0965025b969..f53da67a650 100644
--- a/java/ql/lib/semmle/code/xml/AndroidManifest.qll
+++ b/java/ql/lib/semmle/code/xml/AndroidManifest.qll
@@ -79,10 +79,20 @@ class AndroidApplicationXmlElement extends XmlElement {
    * https://developer.android.com/guide/topics/data/autobackup
    */
   predicate allowsBackup() {
-    not exists(AndroidXmlAttribute attr |
-      this.getAnAttribute() = attr and
-      attr.getName() = "allowBackup" and
-      attr.getValue() = "false"
+    not this.getFile().(AndroidManifestXmlFile).isInBuildDirectory() and
+    (
+      // explicitly sets android:allowBackup="true"
+      this.allowsBackupExplicitly()
+      or
+      // Manifest providing the main intent for an application, and does not explicitly
+      // disallow the allowBackup attribute
+      this.providesMainIntent() and
+      // Check that android:allowBackup="false" is not present
+      not exists(AndroidXmlAttribute attr |
+        this.getAnAttribute() = attr and
+        attr.getName() = "allowBackup" and
+        attr.getValue() = "false"
+      )
     )
   }
 
@@ -91,7 +101,7 @@ class AndroidApplicationXmlElement extends XmlElement {
    *
    * https://developer.android.com/guide/topics/data/autobackup
    */
-  predicate allowsBackupExplicitly() {
+  private predicate allowsBackupExplicitly() {
     exists(AndroidXmlAttribute attr |
       this.getAnAttribute() = attr and
       attr.getName() = "allowBackup" and
@@ -103,7 +113,7 @@ class AndroidApplicationXmlElement extends XmlElement {
    * Holds if the application element contains a child element which provides the
    * `android.intent.action.MAIN` intent.
    */
-  predicate providesMainIntent() {
+  private predicate providesMainIntent() {
     exists(AndroidActivityXmlElement activity |
       activity = this.getAChild() and
       exists(AndroidIntentFilterXmlElement intentFilter |
diff --git a/java/ql/src/Security/CWE/CWE-312/AllowBackupAttributeEnabled.ql b/java/ql/src/Security/CWE/CWE-312/AllowBackupAttributeEnabled.ql
index 97f4ec82982..89421289765 100644
--- a/java/ql/src/Security/CWE/CWE-312/AllowBackupAttributeEnabled.ql
+++ b/java/ql/src/Security/CWE/CWE-312/AllowBackupAttributeEnabled.ql
@@ -14,15 +14,5 @@ import java
 import semmle.code.xml.AndroidManifest
 
 from AndroidApplicationXmlElement androidAppElem
-where
-  not androidAppElem.getFile().(AndroidManifestXmlFile).isInBuildDirectory() and
-  (
-    // explicitly sets android:allowBackup=true
-    androidAppElem.allowsBackupExplicitly()
-    or
-    // Manifest providing the main intent for an application, and does not explicitly
-    // disallow the allowBackup attribute
-    androidAppElem.providesMainIntent() and
-    androidAppElem.allowsBackup()
-  )
+where androidAppElem.allowsBackup()
 select androidAppElem, "The 'android:allowBackup' attribute is enabled."