Fix duplicate permission privilege escalation

Duplicate permissions definition with different group allows
privilege permission escalation to a different permission group.

Android studio and gradle plugin does not allow duplicate permissions
with different attributes, these tools only allow if duplicate
permissions are exact copies.

Also platform stores permissions in map at multiple places with permission
name as key. This suggests that we can disallow duplicate permissions
during package install/update

Bug: 213323615
Test: AppSecurityTests
Change-Id: I9bc839836786a0876e67fd73c05f8944bb532249
Merged-In: I9bc839836786a0876e67fd73c05f8944bb532249

2 files changed, 29 insertions, 7 deletions

diff --git a/services/core/java/com/android/server/pm/pkg/component/ParsedPermissionUtils.java b/services/core/java/com/android/server/pm/pkg/component/ParsedPermissionUtils.java
index f2e2f4f009a9..281e1bd2824d 100644
--- a/services/core/java/com/android/server/pm/pkg/component/ParsedPermissionUtils.java
+++ b/services/core/java/com/android/server/pm/pkg/component/ParsedPermissionUtils.java
@@ -26,6 +26,7 @@ import android.content.res.Resources;
 import android.content.res.TypedArray;
 import android.content.res.XmlResourceParser;
 import android.util.ArrayMap;
+import android.util.EventLog;
 import android.util.Slog;
 
 import com.android.internal.R;
@@ -36,6 +37,7 @@ import org.xmlpull.v1.XmlPullParserException;
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * @hide
@@ -277,8 +279,28 @@ public class ParsedPermissionUtils {
     }
 
     /**
-     * @return {@code true} if the package declares duplicate permissions with different
-     * protection levels.
+     * Determines if a duplicate permission is malformed .i.e. defines different protection level
+     * or group.
+     */
+    private static boolean isMalformedDuplicate(ParsedPermission p1, ParsedPermission p2) {
+        // Since a permission tree is also added as a permission with normal protection
+        // level, we need to skip if the parsedPermission is a permission tree.
+        if (p1 == null || p2 == null || p1.isTree() || p2.isTree()) {
+            return false;
+        }
+
+        if (p1.getProtectionLevel() != p2.getProtectionLevel()) {
+            return true;
+        }
+        if (!Objects.equals(p1.getGroup(), p2.getGroup())) {
+            return true;
+        }
+
+        return false;
+    }
+
+    /**
+     * @return {@code true} if the package declares malformed duplicate permissions.
      */
     public static boolean declareDuplicatePermission(@NonNull ParsingPackage pkg) {
         final List<ParsedPermission> permissions = pkg.getPermissions();
@@ -289,10 +311,10 @@ public class ParsedPermissionUtils {
                 final ParsedPermission parsedPermission = permissions.get(i);
                 final String name = parsedPermission.getName();
                 final ParsedPermission perm = checkDuplicatePerm.get(name);
-                // Since a permission tree is also added as a permission with normal protection
-                // level, we need to skip if the parsedPermission is a permission tree.
-                if (perm != null && !(perm.isTree() || parsedPermission.isTree())
-                        && perm.getProtectionLevel() != parsedPermission.getProtectionLevel()) {
+                if (isMalformedDuplicate(parsedPermission, perm)) {
+                    // Fix for b/213323615
+                    EventLog.writeEvent(0x534e4554, "213323615",
+                            "The package " + pkg.getPackageName() + " seems malicious");
                     return true;
                 }
                 checkDuplicatePerm.put(name, parsedPermission);
diff --git a/services/core/java/com/android/server/pm/pkg/parsing/ParsingPackageUtils.java b/services/core/java/com/android/server/pm/pkg/parsing/ParsingPackageUtils.java
index 6ee9c66e328a..06a54a461d5e 100644
--- a/services/core/java/com/android/server/pm/pkg/parsing/ParsingPackageUtils.java
+++ b/services/core/java/com/android/server/pm/pkg/parsing/ParsingPackageUtils.java
@@ -968,7 +968,7 @@ public class ParsingPackageUtils {
         if (ParsedPermissionUtils.declareDuplicatePermission(pkg)) {
             return input.error(
                     INSTALL_PARSE_FAILED_MANIFEST_MALFORMED,
-                    "Declare duplicate permissions with different protection levels."
+                    "Found duplicate permission with a different attribute value."
             );
         }