From f9a9dc720c151ce9d8a2bba6f5400f5a8f759b2d Mon Sep 17 00:00:00 2001 From: Manjeet Rulhania Date: Thu, 28 Apr 2022 21:50:25 +0000 Subject: 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: I1910dca44104e35a57eba4acfa8188cd9b8626ac Merged-Id: I34120fff2ec2a158dfa55779d2afd4bbd49487ff Merged-In: I9bc839836786a0876e67fd73c05f8944bb532249 --- .../content/pm/parsing/ParsingPackageUtils.java | 8 ++++ .../parsing/component/ParsedPermissionUtils.java | 49 ++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/core/java/android/content/pm/parsing/ParsingPackageUtils.java b/core/java/android/content/pm/parsing/ParsingPackageUtils.java index a56a3ea9b2f6..e4fd59c01eac 100644 --- a/core/java/android/content/pm/parsing/ParsingPackageUtils.java +++ b/core/java/android/content/pm/parsing/ParsingPackageUtils.java @@ -21,6 +21,7 @@ import static android.content.pm.ActivityInfo.RESIZE_MODE_UNRESIZEABLE; import static android.content.pm.PackageManager.FEATURE_WATCH; import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_BAD_MANIFEST; import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_INCONSISTENT_CERTIFICATES; +import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_MANIFEST_MALFORMED; import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_ONLY_COREAPP_ALLOWED; import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_RESOURCES_ARSC_COMPRESSED; import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_UNEXPECTED_EXCEPTION; @@ -778,6 +779,13 @@ public class ParsingPackageUtils { ); } + if (ParsedPermissionUtils.declareDuplicatePermission(pkg)) { + return input.error( + INSTALL_PARSE_FAILED_MANIFEST_MALFORMED, + "Found duplicate permission with a different attribute value." + ); + } + convertNewPermissions(pkg); convertSplitPermissions(pkg); diff --git a/core/java/android/content/pm/parsing/component/ParsedPermissionUtils.java b/core/java/android/content/pm/parsing/component/ParsedPermissionUtils.java index 1884a1e27832..9a6742c68ab4 100644 --- a/core/java/android/content/pm/parsing/component/ParsedPermissionUtils.java +++ b/core/java/android/content/pm/parsing/component/ParsedPermissionUtils.java @@ -22,6 +22,8 @@ import android.content.pm.parsing.ParsingPackage; 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; @@ -32,6 +34,8 @@ import android.content.pm.parsing.result.ParseResult; import org.xmlpull.v1.XmlPullParserException; import java.io.IOException; +import java.util.List; +import java.util.Objects; /** @hide */ public class ParsedPermissionUtils { @@ -207,4 +211,49 @@ public class ParsedPermissionUtils { return ComponentParseUtils.parseAllMetaData(pkg, res, parser, tag, permissionGroup, input); } + + /** + * 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 permissions = pkg.getPermissions(); + final int size = permissions.size(); + if (size > 0) { + final ArrayMap checkDuplicatePerm = new ArrayMap<>(size); + for (int i = 0; i < size; i++) { + final ParsedPermission parsedPermission = permissions.get(i); + final String name = parsedPermission.getName(); + final ParsedPermission perm = checkDuplicatePerm.get(name); + if (isMalformedDuplicate(parsedPermission, perm)) { + // Fix for b/213323615 + EventLog.writeEvent(0x534e4554, "213323615", + "The package " + pkg.getPackageName() + " seems malicious"); + return true; + } + checkDuplicatePerm.put(name, parsedPermission); + } + } + return false; + } } -- cgit v1.2.3-59-g8ed1b