From b72cdff522954c5bc80c9db4523fc645c506e90b Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Sun, 16 Oct 2016 20:28:19 +1100 Subject: [PATCH 1/2] Guard against null, and improve logging in ApkVerifier. --- .../java/org/fdroid/fdroid/installer/ApkVerifier.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ApkVerifier.java b/app/src/main/java/org/fdroid/fdroid/installer/ApkVerifier.java index ea06fe437..e59ba3678 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ApkVerifier.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ApkVerifier.java @@ -23,6 +23,7 @@ import android.content.Context; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.net.Uri; +import android.support.annotation.Nullable; import android.text.TextUtils; import android.util.Log; @@ -80,8 +81,6 @@ class ApkVerifier { } // verify permissions, important for unattended installer - Utils.debugLog(TAG, "localPermissions: " + TextUtils.join("\n", localApkInfo.requestedPermissions)); - Utils.debugLog(TAG, "expectedPermissions: " + TextUtils.join("\n", expectedApk.requestedPermissions)); if (!requestedPermissionsEqual(expectedApk.requestedPermissions, localApkInfo.requestedPermissions)) { throw new ApkPermissionUnequalException("Permissions in APK and index.xml do not match!"); } @@ -103,7 +102,11 @@ class ApkVerifier { * data format is {@link String} arrays but they are in effect sets. This is the * same data format as {@link android.content.pm.PackageInfo#requestedPermissions} */ - public static boolean requestedPermissionsEqual(String[] expected, String[] actual) { + public static boolean requestedPermissionsEqual(@Nullable String[] expected, @Nullable String[] actual) { + Utils.debugLog(TAG, "Checking permissions"); + Utils.debugLog(TAG, "Actual:\n " + (actual == null ? "None" : TextUtils.join("\n ", actual))); + Utils.debugLog(TAG, "Expected:\n " + (expected == null ? "None" : TextUtils.join("\n ", expected))); + if (expected == null && actual == null) { return true; } From 2b2958f89c5d099ad808c5a5d5e66c8aa6df93a5 Mon Sep 17 00:00:00 2001 From: Peter Serwylo Date: Sun, 16 Oct 2016 20:52:42 +1100 Subject: [PATCH 2/2] Added explicit test for null permissions. This wouldn'tve actually found the problem in the previous commit, due to the null happening before checking permissions while logging perms. However, still seems like a nice test to have so that the method itself handles nulls correctly. --- .../org/fdroid/fdroid/installer/ApkVerifierTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/src/androidTest/java/org/fdroid/fdroid/installer/ApkVerifierTest.java b/app/src/androidTest/java/org/fdroid/fdroid/installer/ApkVerifierTest.java index 28bfffdea..fabd5649b 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/installer/ApkVerifierTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/installer/ApkVerifierTest.java @@ -99,6 +99,15 @@ public class ApkVerifierTest { assertTrue(extendedPermsXml.exists()); } + @Test + public void testNulls() { + assertTrue(ApkVerifier.requestedPermissionsEqual(null, null)); + + String[] perms = new String[] {"Blah"}; + assertFalse(ApkVerifier.requestedPermissionsEqual(perms, null)); + assertFalse(ApkVerifier.requestedPermissionsEqual(null, perms)); + } + @Test public void testWithoutPrefix() { Apk apk = new Apk();