diff --git a/F-Droid/src/org/fdroid/fdroid/Utils.java b/F-Droid/src/org/fdroid/fdroid/Utils.java index 7358104bd..f345a8113 100644 --- a/F-Droid/src/org/fdroid/fdroid/Utils.java +++ b/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; } diff --git a/F-Droid/src/org/fdroid/fdroid/compat/FileCompat.java b/F-Droid/src/org/fdroid/fdroid/compat/FileCompat.java new file mode 100644 index 000000000..9784bc8f2 --- /dev/null +++ b/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 ); + } + + } + +} diff --git a/F-Droid/src/org/fdroid/fdroid/data/SanitizedFile.java b/F-Droid/src/org/fdroid/fdroid/data/SanitizedFile.java new file mode 100644 index 000000000..8e60837e6 --- /dev/null +++ b/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.-_]", "")); + } +} diff --git a/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java b/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java index c8c6bcc15..e989da6d6 100644 --- a/F-Droid/src/org/fdroid/fdroid/net/ApkDownloader.java +++ b/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(); } diff --git a/F-Droid/test/src/org/fdroid/fdroid/SanitizedFileTest.java b/F-Droid/test/src/org/fdroid/fdroid/SanitizedFileTest.java new file mode 100644 index 000000000..e69de29bb