From 521218a45c300953570a604bb1689d98f3d45434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 19 Jan 2015 15:45:23 +0100 Subject: [PATCH 1/4] Code cleanup and simplifications --- .../fdroid/installer/DefaultInstaller.java | 22 ++++++++--------- .../installer/DefaultInstallerSdk14.java | 24 +++++++++---------- .../fdroid/fdroid/installer/Installer.java | 6 +---- .../fdroid/installer/SystemInstaller.java | 12 ++++------ 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstaller.java b/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstaller.java index 48619f877..e6282eff7 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstaller.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstaller.java @@ -64,26 +64,24 @@ public class DefaultInstaller extends Installer { @Override protected void installPackageInternal(List 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); + + Uri uri = Uri.fromParts("package", pkgInfo.packageName, null); + Intent intent = new Intent(Intent.ACTION_DELETE, uri); + try { + mActivity.startActivityForResult(intent, REQUEST_CODE_DELETE); + } catch (ActivityNotFoundException e) { + throw new AndroidNotCompatibleException(e); + } } catch (NameNotFoundException e) { // already checked in super class } - - Uri uri = Uri.fromParts("package", pkgInfo.packageName, null); - Intent intent = new Intent(Intent.ACTION_DELETE, uri); - try { - mActivity.startActivityForResult(intent, REQUEST_CODE_DELETE); - } catch (ActivityNotFoundException e) { - throw new AndroidNotCompatibleException(e); - } } @Override diff --git a/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstallerSdk14.java b/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstallerSdk14.java index b45241b80..4815d410a 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstallerSdk14.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/DefaultInstallerSdk14.java @@ -76,27 +76,25 @@ public class DefaultInstallerSdk14 extends Installer { @Override protected void installPackageInternal(List 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); + + Uri uri = Uri.fromParts("package", pkgInfo.packageName, null); + Intent intent = new Intent(Intent.ACTION_UNINSTALL_PACKAGE, uri); + intent.putExtra(Intent.EXTRA_RETURN_RESULT, true); + try { + mActivity.startActivityForResult(intent, REQUEST_CODE_DELETE); + } catch (ActivityNotFoundException e) { + throw new AndroidNotCompatibleException(e); + } } 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); - intent.putExtra(Intent.EXTRA_RETURN_RESULT, true); - try { - mActivity.startActivityForResult(intent, REQUEST_CODE_DELETE); - } catch (ActivityNotFoundException e) { - throw new AndroidNotCompatibleException(e); - } } @Override diff --git a/F-Droid/src/org/fdroid/fdroid/installer/Installer.java b/F-Droid/src/org/fdroid/fdroid/installer/Installer.java index 5fd7c57ec..65b36187e 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/Installer.java +++ b/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 true; - } else { - return false; - } + return permissionsGranted; } public void installPackage(File apkFile) throws AndroidNotCompatibleException { diff --git a/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java b/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java index c3af08249..686ff3107 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java +++ b/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[] { - packageURI, mInstallObserver, INSTALL_REPLACE_EXISTING, null - }); + mInstallMethod.invoke(mPm, packageURI, mInstallObserver, + 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 apkFiles) throws AndroidNotCompatibleException { - // TODO Auto-generated method stub - + // not used } @Override protected void deletePackageInternal(String packageName) throws AndroidNotCompatibleException { try { - mDeleteMethod.invoke(mPm, new Object[] { - packageName, mDeleteObserver, 0 - }); + mDeleteMethod.invoke(mPm, packageName, mDeleteObserver, 0); } catch (Exception e) { throw new AndroidNotCompatibleException(e); } From 1f2fe25cd4e1d13dbd4e5ba479c27ee920c188b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 19 Jan 2015 15:48:39 +0100 Subject: [PATCH 2/4] RootInstaller: Put quotes around filenames and package names --- F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java b/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java index 4334b2222..4d9dabb44 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java @@ -130,7 +130,7 @@ public class RootInstaller extends Installer { } private void addInstallCommand(File apkFile) { - rootSession.addCommand("pm install -r " + apkFile.getAbsolutePath(), 0, + rootSession.addCommand("pm install -r \"" + apkFile.getAbsolutePath() + "\"", 0, new Shell.OnCommandResultListener() { public void onCommandResult(int commandCode, int exitCode, List output) { // close su shell @@ -151,7 +151,7 @@ public class RootInstaller extends Installer { List commands = new ArrayList(); String pm = "pm install -r "; for (File apkFile : apkFiles) { - commands.add(pm + apkFile.getAbsolutePath()); + commands.add(pm + "\"" + apkFile.getAbsolutePath() + "\""); } rootSession.addCommand(commands, 0, @@ -174,7 +174,7 @@ public class RootInstaller extends Installer { } private void addDeleteCommand(String packageName) { - rootSession.addCommand("pm uninstall " + packageName, 0, + rootSession.addCommand("pm uninstall \"" + packageName + "\"", 0, new Shell.OnCommandResultListener() { public void onCommandResult(int commandCode, int exitCode, List output) { // close su shell From d941ac5eb0f9d17fe617ccfb6995987a88b27155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 19 Jan 2015 16:11:46 +0100 Subject: [PATCH 3/4] Only accept valid package names as parameter for 'pm uninstall' --- .../fdroid/installer/RootInstaller.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java b/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java index 4d9dabb44..34e38f781 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java +++ b/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 @@ -174,6 +176,13 @@ public class RootInstaller extends Installer { } private void addDeleteCommand(String packageName) { + 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 output) { @@ -196,6 +205,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 * --key --iv ] [--originating-uri From 239250763422110c073988f4b5dd9d809ebe523d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 19 Jan 2015 16:19:31 +0100 Subject: [PATCH 4/4] Accept same set of characters for apk file names for 'pm install' like for package names --- .../fdroid/installer/RootInstaller.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java b/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java index 34e38f781..91dc44254 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/RootInstaller.java @@ -132,6 +132,16 @@ public class RootInstaller extends Installer { } private void addInstallCommand(File apkFile) { + // 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 output) { @@ -153,6 +163,14 @@ public class RootInstaller extends Installer { List commands = new ArrayList(); String pm = "pm install -r "; for (File apkFile : apkFiles) { + // 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() + "\""); } @@ -177,7 +195,8 @@ public class RootInstaller extends Installer { private void addDeleteCommand(String packageName) { if (!isValidPackageName(packageName)) { - Log.e(TAG, "Package name is not valid (contains characters other than letters, numbers, dots, or underscore): " + 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;