From 0d1e00b6cf5dabdd44610534a00ca8028664efc0 Mon Sep 17 00:00:00 2001
From: Peter Serwylo <peter@serwylo.com>
Date: Fri, 28 Apr 2017 08:33:58 +1000
Subject: [PATCH] Don't accidentally return Apk instances from the wrong repo

There may be multiple apk files with the same hash. Although it is not a
security issue to install one or the other (they are exactly the same
binary), they may have different metadata to display in the client.
Thus, it may result in weirdness if one has a different
name/description/summary etc).

This change takes each of the matching Apk objects from the database,
then asks them where they expect to be downloaded. It matches this
against the File that we are looking at and only returns if they match.
---
 .../fdroid/fdroid/AppUpdateStatusService.java | 51 +++++++++++++------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java
index 0b41dbfa7..2bb2991bc 100644
--- a/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java
+++ b/app/src/main/java/org/fdroid/fdroid/AppUpdateStatusService.java
@@ -5,6 +5,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
+import android.net.Uri;
 import android.support.annotation.Nullable;
 import android.util.Log;
 
@@ -68,33 +69,20 @@ public class AppUpdateStatusService extends IntentService {
             return null;
         }
 
-        // NOTE: This presumes SHA256 is the only supported hash. It seems like that is an assumption
-        // in more than one place in the F-Droid client. If this becomes a problem in the future, we
-        // can query the Apk table for `SELECT DISTINCT hashType FROM fdroid_apk` and then we can just
-        // try each of the hash types that have been specified in the metadata. Seems a bit overkill
-        // at the time of writing though.
         Utils.debugLog(TAG, "Found package for " + downloadedInfo.packageName + ", checking its hash to see if it downloaded correctly.");
-        String hash = Utils.getBinaryHash(apkPath, "sha256");
-
-        List<Apk> apksMatchingHash = ApkProvider.Helper.findApksByHash(this, hash);
-        Utils.debugLog(TAG, "Found " + apksMatchingHash.size() + " apk(s) matching the hash " + hash);
-
-        if (apksMatchingHash.size() == 0) {
+        Apk downloadedApk = findApkMatchingHash(apkPath);
+        if (downloadedApk ==  null) {
             Utils.debugLog(TAG, "Either the apk wasn't downloaded fully, or the repo it came from has been disabled. Either way, not notifying the user about it.");
             return null;
         }
 
-        // It makes zero difference which apk we get from this list. By definition they all have
-        // the exact same hash, and are thus the same binary.
-        Apk downloadedApk = apksMatchingHash.get(0);
-
         PackageInfo installedInfo = null;
         try {
             installedInfo = getPackageManager().getPackageInfo(downloadedApk.packageName, PackageManager.GET_META_DATA);
         } catch (PackageManager.NameNotFoundException ignored) { }
 
         if (installedInfo == null) {
-            if (AppUpdateStatusManager.getInstance(this).isPendingInstall(hash)) {
+            if (AppUpdateStatusManager.getInstance(this).isPendingInstall(downloadedApk.hash)) {
                 Utils.debugLog(TAG, downloadedApk.packageName + " is not installed, so presuming we need to notify the user about installing it.");
                 return downloadedApk;
             } else {
@@ -112,4 +100,35 @@ public class AppUpdateStatusService extends IntentService {
 
         return downloadedApk;
     }
+
+    /**
+     * There could be multiple apks with the same hash, provided by different repositories.
+     * This method looks for all matching records in the database. It then asks each of these
+     * {@link Apk} instances where they expect to be downloaded. If they expect to be downloaded
+     * to {@param apkPath} then that instance is returned.
+     *
+     * If no files have a matching hash, or only those which don't belong to the correct repo, then
+     * this will return null.
+     */
+    @Nullable
+    private Apk findApkMatchingHash(File apkPath) {
+
+        // NOTE: This presumes SHA256 is the only supported hash. It seems like that is an assumption
+        // in more than one place in the F-Droid client. If this becomes a problem in the future, we
+        // can query the Apk table for `SELECT DISTINCT hashType FROM fdroid_apk` and then we can just
+        // try each of the hash types that have been specified in the metadata. Seems a bit overkill
+        // at the time of writing though.
+        String hash = Utils.getBinaryHash(apkPath, "sha256");
+
+        List<Apk> apksMatchingHash = ApkProvider.Helper.findApksByHash(this, hash);
+        Utils.debugLog(TAG, "Found " + apksMatchingHash.size() + " apk(s) matching the hash " + hash);
+
+        for (Apk apk : apksMatchingHash) {
+            if (apkPath.equals(ApkCache.getApkDownloadPath(this, Uri.parse(apk.getUrl())))) {
+                return apk;
+            }
+        }
+
+        return null;
+    }
 }
\ No newline at end of file