From 5f7dc6c2c9aa4d41334117505efd46da06ef1353 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Pluta?= <bartlomiej.p.pluta@gmail.com>
Date: Fri, 5 Apr 2019 14:47:04 +0200
Subject: [PATCH] 10: Create PermissionsRaceConditionPlugin

---
 .../com/bartek/esa/core/di/PluginModule.java  |  7 ++++
 .../PermissionsRaceConditionPlugin.java       | 35 +++++++++++++++++++
 src/main/resources/description.properties     | 27 ++++++++------
 3 files changed, 58 insertions(+), 11 deletions(-)
 create mode 100644 src/main/java/com/bartek/esa/core/plugin/PermissionsRaceConditionPlugin.java

diff --git a/src/main/java/com/bartek/esa/core/di/PluginModule.java b/src/main/java/com/bartek/esa/core/di/PluginModule.java
index bdde37c..00e1a80 100644
--- a/src/main/java/com/bartek/esa/core/di/PluginModule.java
+++ b/src/main/java/com/bartek/esa/core/di/PluginModule.java
@@ -4,6 +4,7 @@ import com.bartek.esa.core.archetype.Plugin;
 import com.bartek.esa.core.plugin.AllowBackupPlugin;
 import com.bartek.esa.core.plugin.DebuggablePlugin;
 import com.bartek.esa.core.plugin.LoggingPlugin;
+import com.bartek.esa.core.plugin.PermissionsRaceConditionPlugin;
 import com.bartek.esa.core.xml.XmlHelper;
 import com.bartek.esa.file.matcher.GlobMatcher;
 import dagger.Module;
@@ -40,4 +41,10 @@ public class PluginModule {
     public Plugin allowBackupPlugin(GlobMatcher globMatcher, XmlHelper xmlHelper) {
         return new AllowBackupPlugin(globMatcher, xmlHelper);
     }
+
+    @Provides
+    @IntoSet
+    public Plugin permissionRaceConditionPlugin(GlobMatcher globMatcher, XmlHelper xmlHelper) {
+        return new PermissionsRaceConditionPlugin(globMatcher, xmlHelper);
+    }
 }
diff --git a/src/main/java/com/bartek/esa/core/plugin/PermissionsRaceConditionPlugin.java b/src/main/java/com/bartek/esa/core/plugin/PermissionsRaceConditionPlugin.java
new file mode 100644
index 0000000..f1e3478
--- /dev/null
+++ b/src/main/java/com/bartek/esa/core/plugin/PermissionsRaceConditionPlugin.java
@@ -0,0 +1,35 @@
+package com.bartek.esa.core.plugin;
+
+import com.bartek.esa.core.archetype.AndroidManifestPlugin;
+import com.bartek.esa.core.model.enumeration.Severity;
+import com.bartek.esa.core.xml.XmlHelper;
+import com.bartek.esa.file.matcher.GlobMatcher;
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+
+import javax.inject.Inject;
+import javax.xml.xpath.XPathConstants;
+
+import static java.lang.Integer.parseInt;
+
+public class PermissionsRaceConditionPlugin extends AndroidManifestPlugin {
+
+    @Inject
+    public PermissionsRaceConditionPlugin(GlobMatcher globMatcher, XmlHelper xmlHelper) {
+        super(globMatcher, xmlHelper);
+    }
+
+    @Override
+    protected void run(Document xml) {
+        boolean isAnyPermissionDefined = ((NodeList) xPath(xml, "/manifest/permission", XPathConstants.NODESET)).getLength() > 0;
+        if(isAnyPermissionDefined) {
+            Node usesSdkNode = (Node) xPath(xml, "/manifest/uses-sdk", XPathConstants.NODE);
+            Node minSdkVersionNode = usesSdkNode.getAttributes().getNamedItem("android:minSdkVersion");
+            int minSdkVersion = parseInt(minSdkVersionNode.getNodeValue());
+            if(minSdkVersion < 21) {
+                addIssue(Severity.VULNERABILITY, null, minSdkVersionNode.toString());
+            }
+        }
+    }
+}
diff --git a/src/main/resources/description.properties b/src/main/resources/description.properties
index d0b792b..e96b19e 100644
--- a/src/main/resources/description.properties
+++ b/src/main/resources/description.properties
@@ -26,18 +26,23 @@ com.bartek.esa.core.plugin.DebuggablePlugin.NO_ATTR=There is no android:debuggab
   For example: <application android:debuggable="false">
 
 com.bartek.esa.core.plugin.DebuggablePlugin.NO_FALSE=The android:debuggable is set to 'true'. Potential data leakage. \n\
-  The android:debuggable option in AndroidManifest.xml is set to 'true'. \n\
-  This will cause application to be debuggable and can result in \
-  security issues and data leakage on the production environment. \n\
-  Consider setting it to 'false'.
+The android:debuggable option in AndroidManifest.xml is set to 'true'. \n\
+This will cause application to be debuggable and can result in \
+security issues and data leakage on the production environment. \n\
+Consider setting it to 'false'.
 
 com.bartek.esa.core.plugin.AllowBackupPlugin.NO_ATTR=There is no android:allowBackup option. Potential data leakage. \n\
-  The android:allowBackup option was not found in the AndroidManifest.xml file. \n\
-  To avoid any potential data theft in the future, please explicitly set this flag to false. \n\
-  The attribute should be placed in <application> tag.\n\
-  For example: <application android:allowBackup="false">
+The android:allowBackup option was not found in the AndroidManifest.xml file. \n\
+To avoid any potential data theft in the future, please explicitly set this flag to false. \n\
+The attribute should be placed in <application> tag.\n\
+For example: <application android:allowBackup="false">
 
 com.bartek.esa.core.plugin.AllowBackupPlugin.NO_FALSE=The android:allowBackup is set to 'true'. Potential data leakage. \n\
-  The android:allowBackup option in AndroidManifest.xml is set to 'true'. \n\
-  This will allow accessing the backups via adb if device has USB debugging enabled.\n\
-  Consider setting it to 'false'.
\ No newline at end of file
+The android:allowBackup option in AndroidManifest.xml is set to 'true'. \n\
+This will allow accessing the backups via adb if device has USB debugging enabled.\n\
+Consider setting it to 'false'.
+
+com.bartek.esa.core.plugin.PermissionsRaceConditionPlugin=Potential permissions race condition vulnerability. \n\
+  There are declared custom permissions in AndroidManifest.xml and the minimal API version is set to less than 21.\n\
+  It means that declared permissions can be obtained by malicious application installed before and without need of having 1proper signature.\n\
+  Consider setting minimal API version to 21 at least.