From e6c1dd532d2910285d503f271183839446495360 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Thu, 14 Jun 2018 04:30:24 +0800 Subject: [PATCH] Re-implement duplicate Magisk Manager logic Starting from the next Magisk release, it will no longer prefer the package name com.topjohnwu.magisk over a hidden manager; it will always be aware whether the hidden manager exists, so when a package named com.topjohnwu.magisk is installed alongside with the hidden manager, com.topjohnwu.magisk will not have root access by default. This will prevent malware from using the package name com.topjohnwu.magisk to gain root access when a user is using a hidden manager. To support this new behavior, several changes has to be done: - Never grant com.topjohnwu.magisk in Magisk Manager (if it IS the actual manager, MagiskSU will grant it by default) - While hidden, remove com.topjohnwu.magisk if exists - Restore Magisk Manager (unhide) has to be done with root - Upgrading Magisk Manager should preserve package name (implemented in a949641) --- .../com/topjohnwu/magisk/MagiskManager.java | 27 +++++++----------- .../topjohnwu/magisk/SettingsActivity.java | 28 +++++++++++-------- .../topjohnwu/magisk/asyncs/HideManager.java | 2 +- .../magisk/receivers/ManagerUpdate.java | 1 - .../magisk/superuser/RequestActivity.java | 4 +++ .../com/topjohnwu/magisk/utils/Const.java | 2 +- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/full/java/com/topjohnwu/magisk/MagiskManager.java b/src/full/java/com/topjohnwu/magisk/MagiskManager.java index 15f95d9bf..0a73dc616 100644 --- a/src/full/java/com/topjohnwu/magisk/MagiskManager.java +++ b/src/full/java/com/topjohnwu/magisk/MagiskManager.java @@ -3,7 +3,6 @@ package com.topjohnwu.magisk; import android.app.job.JobInfo; import android.app.job.JobScheduler; import android.content.ComponentName; -import android.content.Intent; import android.content.SharedPreferences; import android.content.pm.PackageManager; import android.content.res.Configuration; @@ -131,25 +130,19 @@ public class MagiskManager extends Application implements Shell.Container { }); prefs = PreferenceManager.getDefaultSharedPreferences(this); - - // Handle duplicate package - if (!getPackageName().equals(Const.ORIG_PKG_NAME)) { - try { - getPackageManager().getApplicationInfo(Const.ORIG_PKG_NAME, 0); - Intent intent = getPackageManager().getLaunchIntentForPackage(Const.ORIG_PKG_NAME); - intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); - startActivity(intent); - return; - } catch (PackageManager.NameNotFoundException ignored) { /* Expected */ } - } - mDB = MagiskDatabaseHelper.getInstance(this); - String pkg = mDB.getStrings(Const.Key.SU_REQUESTER, Const.ORIG_PKG_NAME); - if (getPackageName().equals(Const.ORIG_PKG_NAME) && !pkg.equals(Const.ORIG_PKG_NAME)) { - mDB.setStrings(Const.Key.SU_REQUESTER, null); + String pkg = mDB.getStrings(Const.Key.SU_MANAGER, null); + if (pkg != null && getPackageName().equals(Const.ORIG_PKG_NAME)) { + mDB.setStrings(Const.Key.SU_MANAGER, null); RootUtils.uninstallPkg(pkg); - mDB = MagiskDatabaseHelper.getInstance(this); + } + if (TextUtils.equals(pkg, getPackageName())) { + try { + // We are the manager, remove com.topjohnwu.magisk as it could be malware + getPackageManager().getApplicationInfo(Const.ORIG_PKG_NAME, 0); + RootUtils.uninstallPkg(Const.ORIG_PKG_NAME); + } catch (PackageManager.NameNotFoundException ignored) {} } setLocale(); diff --git a/src/full/java/com/topjohnwu/magisk/SettingsActivity.java b/src/full/java/com/topjohnwu/magisk/SettingsActivity.java index 257af77bb..fd69d9942 100644 --- a/src/full/java/com/topjohnwu/magisk/SettingsActivity.java +++ b/src/full/java/com/topjohnwu/magisk/SettingsActivity.java @@ -1,8 +1,8 @@ package com.topjohnwu.magisk; -import android.Manifest; -import android.content.Intent; +import android.content.Context; import android.content.SharedPreferences; +import android.net.Uri; import android.os.Build; import android.os.Bundle; import android.support.v14.preference.SwitchPreference; @@ -23,9 +23,10 @@ import android.widget.Toast; import com.topjohnwu.magisk.asyncs.CheckUpdates; import com.topjohnwu.magisk.asyncs.HideManager; import com.topjohnwu.magisk.components.Activity; -import com.topjohnwu.magisk.receivers.ManagerUpdate; +import com.topjohnwu.magisk.receivers.DownloadReceiver; import com.topjohnwu.magisk.utils.Const; import com.topjohnwu.magisk.utils.FingerprintHelper; +import com.topjohnwu.magisk.utils.RootUtils; import com.topjohnwu.magisk.utils.Topic; import com.topjohnwu.magisk.utils.Utils; import com.topjohnwu.superuser.Shell; @@ -166,15 +167,18 @@ public class SettingsActivity extends Activity implements Topic.Subscriber { } else { if (Utils.checkNetworkStatus()) { restoreManager.setOnPreferenceClickListener((pref) -> { - runWithPermission(getActivity(), - new String[] { Manifest.permission.WRITE_EXTERNAL_STORAGE }, () -> { - Intent intent = new Intent(mm, ManagerUpdate.class); - intent.putExtra(Const.Key.INTENT_SET_LINK, mm.managerLink); - intent.putExtra(Const.Key.INTENT_SET_FILENAME, - Utils.fmt("MagiskManager-v%s(%d).apk", - mm.remoteManagerVersionString, mm.remoteManagerVersionCode)); - mm.sendBroadcast(intent); - }); + Utils.dlAndReceive( + getActivity(), new DownloadReceiver() { + @Override + public void onDownloadDone(Context context, Uri uri) { + mm.dumpPrefs(); + if (RootUtils.cmdResult("pm install " + uri.getPath())) + RootUtils.uninstallPkg(context.getPackageName()); + } + }, + mm.managerLink, + Utils.fmt("MagiskManager-v%s.apk", mm.remoteManagerVersionString) + ); return true; }); } else { diff --git a/src/full/java/com/topjohnwu/magisk/asyncs/HideManager.java b/src/full/java/com/topjohnwu/magisk/asyncs/HideManager.java index 9c821a7c0..999d51654 100644 --- a/src/full/java/com/topjohnwu/magisk/asyncs/HideManager.java +++ b/src/full/java/com/topjohnwu/magisk/asyncs/HideManager.java @@ -77,7 +77,7 @@ public class HideManager extends ParallelTask { repack.delete(); - mm.mDB.setStrings(Const.Key.SU_REQUESTER, pkg); + mm.mDB.setStrings(Const.Key.SU_MANAGER, pkg); mm.dumpPrefs(); RootUtils.uninstallPkg(Const.ORIG_PKG_NAME); diff --git a/src/full/java/com/topjohnwu/magisk/receivers/ManagerUpdate.java b/src/full/java/com/topjohnwu/magisk/receivers/ManagerUpdate.java index 33e2f82ce..5dd8e520f 100644 --- a/src/full/java/com/topjohnwu/magisk/receivers/ManagerUpdate.java +++ b/src/full/java/com/topjohnwu/magisk/receivers/ManagerUpdate.java @@ -5,7 +5,6 @@ import android.content.Context; import android.content.Intent; import android.net.Uri; import android.os.AsyncTask; -import android.os.Handler; import com.topjohnwu.magisk.utils.Const; import com.topjohnwu.magisk.utils.PatchAPK; diff --git a/src/full/java/com/topjohnwu/magisk/superuser/RequestActivity.java b/src/full/java/com/topjohnwu/magisk/superuser/RequestActivity.java index bf7b11765..10bb42c68 100644 --- a/src/full/java/com/topjohnwu/magisk/superuser/RequestActivity.java +++ b/src/full/java/com/topjohnwu/magisk/superuser/RequestActivity.java @@ -277,6 +277,10 @@ public class RequestActivity extends Activity { if (policy == null) { policy = new Policy(uid, pm); } + + /* Never allow com.topjohnwu.magisk (could be malware) */ + if (TextUtils.equals(policy.packageName, Const.ORIG_PKG_NAME)) + return false; } catch (Exception e) { e.printStackTrace(); return false; diff --git a/src/main/java/com/topjohnwu/magisk/utils/Const.java b/src/main/java/com/topjohnwu/magisk/utils/Const.java index 4bfbf266f..5081d03d0 100644 --- a/src/main/java/com/topjohnwu/magisk/utils/Const.java +++ b/src/main/java/com/topjohnwu/magisk/utils/Const.java @@ -94,7 +94,7 @@ public class Const { public static final String ROOT_ACCESS = "root_access"; public static final String SU_MULTIUSER_MODE = "multiuser_mode"; public static final String SU_MNT_NS = "mnt_ns"; - public static final String SU_REQUESTER = "requester"; + public static final String SU_MANAGER = "requester"; public static final String SU_REQUEST_TIMEOUT = "su_request_timeout"; public static final String SU_AUTO_RESPONSE = "su_auto_response"; public static final String SU_NOTIFICATION = "su_notification";