Cache .apks in internal storage before installing.

This prevents an app with "write external storage" permission from
being able to switch the legit app with a dodgey one between F-Droid
requesting an install, and the package manager actually showing the
install dialog to the user.

In order to make the file in private internal storage readable by
the package manager, its parent directories need to be world-executable,
and the file itself needs to be world-readable. It seems that the
"/data/data/org.fdroid.fdroid/cache" dir provided by the Context is
already world executable, but the "apks" subdirectory does not default
to this.

Also, to be compatible with android-8, a Runtime.getRuntime().exec()
call was added for such devices, which invokes /system/bin/chmod.
The effect of this was to require some level of file sanitization to
be made available using the Java type system to prevent command injection
attacks from weird apk names (as people are free to download metadata
from random internet people).

F-Droid/src/org/fdroid/fdroid/Utils.java

@@ -29,7 +29,10 @@ import android.text.TextUtils;
 import android.util.DisplayMetrics;
 import android.util.Log;
 import com.nostra13.universalimageloader.utils.StorageUtils;
+import org.fdroid.fdroid.compat.FileCompat;
+import org.fdroid.fdroid.data.Apk;
 import org.fdroid.fdroid.data.Repo;
+import org.fdroid.fdroid.data.SanitizedFile;
 import org.xml.sax.XMLReader;
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
@@ -125,6 +128,21 @@ public final class Utils {
         }
     }
 
+    /**
+     * Read the input stream until it reaches the end, ignoring any exceptions.
+     */
+    public static void consumeStream(InputStream stream) {
+        final byte buffer[] = new byte[256];
+        try {
+            int read;
+            do {
+                read = stream.read(buffer);
+            } while (read != -1);
+        } catch (IOException e) {
+            // Ignore...
+        }
+    }
+
     public static boolean symlink(File inFile, File outFile) {
         int exitCode = -1;
         try {
@@ -311,12 +329,21 @@ public final class Utils {
         return b.build();
     }
 
+    /**
+     * The directory where .apk files are downloaded (and stored - if the relevant property is enabled).
+     * This must be on internal storage, to prevent other apps with "write external storage" from being
+     * able to change the .apk file between F-Droid requesting the Package Manger to install, and the
+     * Package Manager receiving that request.
+     */
     public static File getApkCacheDir(Context context) {
-        File apkCacheDir = new File(
-                StorageUtils.getCacheDirectory(context, true), "apks");
+        SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, false), "apks");
         if (!apkCacheDir.exists()) {
             apkCacheDir.mkdir();
         }
+
+        // All parent directories of the .apk file need to be executable for the package installer
+        // to be able to have permission to read our world-readable .apk files.
+        FileCompat.setExecutable(apkCacheDir, true, false);
         return apkCacheDir;
     }
 

F-Droid/src/org/fdroid/fdroid/compat/FileCompat.java

@@ -0,0 +1,70 @@
+package org.fdroid.fdroid.compat;
+
+import android.annotation.TargetApi;
+import android.util.Log;
+import org.fdroid.fdroid.Utils;
+import org.fdroid.fdroid.data.SanitizedFile;
+
+import java.io.IOException;
+
+public class FileCompat {
+
+    private static final String TAG = "org.fdroid.fdroid.compat.FileCompat";
+
+    @TargetApi(9)
+    public static boolean setReadable(SanitizedFile file, boolean readable, boolean ownerOnly) {
+
+        if (Compatibility.hasApi(9)) {
+            return file.setReadable(readable, ownerOnly);
+        } else {
+            String mode;
+            if (readable) {
+                mode = ownerOnly ? "0600" : "0644";
+            } else {
+                mode = "0000";
+            }
+            return setMode(file, mode);
+        }
+
+    }
+
+    private static boolean setMode(SanitizedFile file, String mode) {
+
+        // The "file" must be a sanitized file, and hence only contain A-Za-z0-9.-_ already,
+        // but it makes no assurances about the parent directory.
+        String[] args = {
+            "/system/bin/chmod",
+            mode,
+            file.getAbsolutePath()
+        };
+
+        try {
+            Log.d(TAG, "Executing following command: " + args[0] + " " + args[1] + " " + args[2]);
+            Process proc = Runtime.getRuntime().exec(args);
+            Utils.consumeStream(proc.getInputStream());
+            Utils.consumeStream(proc.getErrorStream());
+            return true;
+        } catch (IOException e) {
+            return false;
+        }
+
+    }
+
+    @TargetApi(9)
+    public static boolean setExecutable(SanitizedFile file, boolean readable, boolean ownerOnly) {
+
+        if (Compatibility.hasApi(9)) {
+            return file.setExecutable(readable, ownerOnly);
+        } else {
+            String mode;
+            if ( readable ) {
+                mode = ownerOnly ? "0700" : "0711";
+            } else {
+                mode = ownerOnly ? "0600" : "0600";
+            }
+            return setMode( file, mode );
+        }
+
+    }
+
+}

F-Droid/src/org/fdroid/fdroid/data/SanitizedFile.java

@@ -0,0 +1,15 @@
+package org.fdroid.fdroid.data;
+
+import java.io.File;
+
+/**
+ * File guaranteed to have a santitized name (though not a sanitized path to the parent dir).
+ * Useful so that we can use Java's type system to enforce that the file we are accessing
+ * doesn't contain illegal characters.
+ * Sanitized names are those which only have the following characters: [A-Za-z0-9.-_]
+ */
+public class SanitizedFile extends File {
+    public SanitizedFile(File parent, String name) {
+        super(parent, name.replaceAll("[^A-Za-z0-9.-_]", ""));
+    }
+}

F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java

@@ -25,7 +25,9 @@ import android.util.Log;
 
 import org.fdroid.fdroid.Hasher;
 import org.fdroid.fdroid.ProgressListener;
+import org.fdroid.fdroid.compat.FileCompat;
 import org.fdroid.fdroid.data.Apk;
+import org.fdroid.fdroid.data.SanitizedFile;
 
 import java.io.File;
 import java.io.IOException;
@@ -59,7 +61,7 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
 
     private Apk curApk;
     private String repoAddress;
-    private File localFile;
+    private SanitizedFile localFile;
 
     private ProgressListener listener;
     private AsyncDownloadWrapper dlWrapper = null;
@@ -80,13 +82,13 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
     public ApkDownloader(Apk apk, String repoAddress, File destDir) {
         curApk = apk;
         this.repoAddress = repoAddress;
-        localFile = new File(destDir, curApk.apkName);
+        localFile = new SanitizedFile(destDir, curApk.apkName);
     }
 
     /**
      * The downloaded APK. Valid only when getStatus() has returned STATUS.DONE.
      */
-    public File localFile() {
+    public SanitizedFile localFile() {
         return localFile;
     }
 
@@ -239,6 +241,13 @@ public class ApkDownloader implements AsyncDownloadWrapper.Listener {
             return;
         }
 
+        // Need the apk to be world readable, so that the installer is able to read it.
+        // Note that saving it into external storage for the purpose of letting the installer
+        // have access is insecure, because apps with permission to write to the external
+        // storage can overwrite the app between F-Droid asking for it to be installed and
+        // the installer actually installing it.
+        FileCompat.setReadable(localFile, true, false);
+
         Log.d("FDroid", "Download finished: " + localFile);
         sendCompleteMessage();
     }