Merge branch 'superfdroid-fixes' into 'master'

Superfdroid fixes

Package names and apk file names should only contain letters, numbers, dots,
and underscores. This is now checked in RootInstaller before executing 'pm
install' or 'pm uninstall'.

See merge request !48  https://gitlab.com/fdroid/fdroidclient/merge_requests/48

F-Droid/src/org/fdroid/fdroid/installer/DefaultInstaller.java

@@ -64,18 +64,13 @@ public class DefaultInstaller extends Installer {
 
     @Override
     protected void installPackageInternal(List<File> apkFiles) throws AndroidNotCompatibleException {
-        // TODO Auto-generated method stub
+        // not used
-
     }
 
     @Override
     protected void deletePackageInternal(String packageName) throws AndroidNotCompatibleException {
-        PackageInfo pkgInfo = null;
         try {
-            pkgInfo = mPm.getPackageInfo(packageName, 0);
+            PackageInfo pkgInfo = mPm.getPackageInfo(packageName, 0);
-        } catch (NameNotFoundException e) {
-            // already checked in super class
-        }
 
             Uri uri = Uri.fromParts("package", pkgInfo.packageName, null);
             Intent intent = new Intent(Intent.ACTION_DELETE, uri);
@@ -84,6 +79,9 @@ public class DefaultInstaller extends Installer {
             } catch (ActivityNotFoundException e) {
                 throw new AndroidNotCompatibleException(e);
             }
+        } catch (NameNotFoundException e) {
+            // already checked in super class
+        }
     }
 
     @Override

F-Droid/src/org/fdroid/fdroid/installer/DefaultInstallerSdk14.java

@@ -76,18 +76,13 @@ public class DefaultInstallerSdk14 extends Installer {
 
     @Override
     protected void installPackageInternal(List<File> apkFiles) throws AndroidNotCompatibleException {
-        // TODO Auto-generated method stub
+        // not used
-
     }
 
     @Override
     protected void deletePackageInternal(String packageName) throws AndroidNotCompatibleException {
-        PackageInfo pkgInfo = null;
         try {
-            pkgInfo = mPm.getPackageInfo(packageName, 0);
+            PackageInfo pkgInfo = mPm.getPackageInfo(packageName, 0);
-        } catch (NameNotFoundException e) {
-            // already checked in super class
-        }
 
             Uri uri = Uri.fromParts("package", pkgInfo.packageName, null);
             Intent intent = new Intent(Intent.ACTION_UNINSTALL_PACKAGE, uri);
@@ -97,6 +92,9 @@ public class DefaultInstallerSdk14 extends Installer {
             } catch (ActivityNotFoundException e) {
                 throw new AndroidNotCompatibleException(e);
             }
+        } catch (NameNotFoundException e) {
+            // already checked in super class
+        }
     }
 
     @Override

F-Droid/src/org/fdroid/fdroid/installer/Installer.java

@@ -190,11 +190,7 @@ abstract public class Installer {
                 (checkInstallPermission == PackageManager.PERMISSION_GRANTED
                 && checkDeletePermission == PackageManager.PERMISSION_GRANTED);
 
-        if (permissionsGranted) {
+        return permissionsGranted;
-            return true;
-        } else {
-            return false;
-        }
     }
 
     public void installPackage(File apkFile) throws AndroidNotCompatibleException {

F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java

@@ -28,6 +28,8 @@ import eu.chainfire.libsuperuser.Shell;
 import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Installer using a root shell and "pm install", "pm uninstall" commands
@@ -130,7 +132,17 @@ public class RootInstaller extends Installer {
     }
 
     private void addInstallCommand(File apkFile) {
-        rootSession.addCommand("pm install -r " + apkFile.getAbsolutePath(), 0,
+        // Like package names, apk files should also only contain letters, numbers, dots, or underscore,
+        // e.g., org.fdroid.fdroid_9.apk
+        if (!isValidPackageName(apkFile.getName())) {
+            Log.e(TAG, "File name is not valid (contains characters other than letters, numbers, dots, or underscore): "
+                    + apkFile.getName());
+            mCallback.onError(InstallerCallback.OPERATION_DELETE,
+                    InstallerCallback.ERROR_CODE_OTHER);
+            return;
+        }
+
+        rootSession.addCommand("pm install -r \"" + apkFile.getAbsolutePath() + "\"", 0,
                 new Shell.OnCommandResultListener() {
                     public void onCommandResult(int commandCode, int exitCode, List<String> output) {
                         // close su shell
@@ -151,7 +163,15 @@ public class RootInstaller extends Installer {
         List<String> commands = new ArrayList<String>();
         String pm = "pm install -r ";
         for (File apkFile : apkFiles) {
-            commands.add(pm + apkFile.getAbsolutePath());
+            // see addInstallCommand()
+            if (!isValidPackageName(apkFile.getName())) {
+                Log.e(TAG, "File name is not valid (contains characters other than letters, numbers, dots, or underscore): "
+                        + apkFile.getName());
+                mCallback.onError(InstallerCallback.OPERATION_DELETE,
+                        InstallerCallback.ERROR_CODE_OTHER);
+                return;
+            }
+            commands.add(pm + "\"" + apkFile.getAbsolutePath() + "\"");
         }
 
         rootSession.addCommand(commands, 0,
@@ -174,7 +194,15 @@ public class RootInstaller extends Installer {
     }
 
     private void addDeleteCommand(String packageName) {
-        rootSession.addCommand("pm uninstall " + packageName, 0,
+        if (!isValidPackageName(packageName)) {
+            Log.e(TAG, "Package name is not valid (contains characters other than letters, numbers, dots, or underscore): "
+                    + packageName);
+            mCallback.onError(InstallerCallback.OPERATION_DELETE,
+                    InstallerCallback.ERROR_CODE_OTHER);
+            return;
+        }
+
+        rootSession.addCommand("pm uninstall \"" + packageName + "\"", 0,
                 new Shell.OnCommandResultListener() {
                     public void onCommandResult(int commandCode, int exitCode, List<String> output) {
                         // close su shell
@@ -196,6 +224,20 @@ public class RootInstaller extends Installer {
         return true;
     }
 
+    private static final Pattern PACKAGE_NAME_BLACKLIST = Pattern.compile("[^a-zA-Z0-9\\.\\_]");
+
+    /**
+     * Package names should only contain letters, numbers, dots, and underscores!
+     * Prevent injection attacks with app names like ";touch $'\057data\057injected'"
+     *
+     * @param packageName
+     * @return
+     */
+    private boolean isValidPackageName(String packageName) {
+        Matcher matcher = PACKAGE_NAME_BLACKLIST.matcher(packageName);
+        return !matcher.find();
+    }
+
     /**
      * pm install [-l] [-r] [-t] [-i INSTALLER_PACKAGE_NAME] [-s] [-f] [--algo
      * <algorithm name> --key <key-in-hex> --iv <IV-in-hex>] [--originating-uri

F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java

@@ -128,9 +128,8 @@ public class SystemInstaller extends Installer {
     protected void installPackageInternal(File apkFile) throws AndroidNotCompatibleException {
         Uri packageURI = Uri.fromFile(apkFile);
         try {
-            mInstallMethod.invoke(mPm, new Object[] {
+            mInstallMethod.invoke(mPm, packageURI, mInstallObserver,
-                    packageURI, mInstallObserver, INSTALL_REPLACE_EXISTING, null
+                    INSTALL_REPLACE_EXISTING, null);
-            });
         } catch (Exception e) {
             throw new AndroidNotCompatibleException(e);
         }
@@ -138,16 +137,13 @@ public class SystemInstaller extends Installer {
 
     @Override
     protected void installPackageInternal(List<File> apkFiles) throws AndroidNotCompatibleException {
-        // TODO Auto-generated method stub
+        // not used
-
     }
 
     @Override
     protected void deletePackageInternal(String packageName) throws AndroidNotCompatibleException {
         try {
-            mDeleteMethod.invoke(mPm, new Object[] {
+            mDeleteMethod.invoke(mPm, packageName, mDeleteObserver, 0);
-                    packageName, mDeleteObserver, 0
-            });
         } catch (Exception e) {
             throw new AndroidNotCompatibleException(e);
         }