From 46a4070f841b2d2463cdc683aa17dd546285b610 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Thu, 26 Jan 2017 13:46:54 +0800 Subject: [PATCH] Prevent shell response crashes --- .../java/com/topjohnwu/magisk/Global.java | 33 ++++++++++--- .../com/topjohnwu/magisk/InstallFragment.java | 6 ++- .../com/topjohnwu/magisk/LogFragment.java | 32 +++++++------ .../topjohnwu/magisk/SettingsActivity.java | 4 +- .../com/topjohnwu/magisk/SplashActivity.java | 2 +- .../com/topjohnwu/magisk/StatusFragment.java | 46 ++++++------------- .../magisk/receivers/RepoDlReceiver.java | 2 +- .../com/topjohnwu/magisk/utils/Async.java | 11 +++-- .../magisk/utils/ByteArrayInOutStream.java | 18 ++++++++ .../com/topjohnwu/magisk/utils/Shell.java | 8 +++- .../com/topjohnwu/magisk/utils/Utils.java | 35 +++++++------- .../com/topjohnwu/magisk/utils/ZipUtils.java | 2 - 12 files changed, 118 insertions(+), 81 deletions(-) create mode 100644 app/src/main/java/com/topjohnwu/magisk/utils/ByteArrayInOutStream.java diff --git a/app/src/main/java/com/topjohnwu/magisk/Global.java b/app/src/main/java/com/topjohnwu/magisk/Global.java index 00a1162f7..8bb5488f6 100644 --- a/app/src/main/java/com/topjohnwu/magisk/Global.java +++ b/app/src/main/java/com/topjohnwu/magisk/Global.java @@ -3,22 +3,26 @@ package com.topjohnwu.magisk; import android.content.Context; import android.content.SharedPreferences; import android.preference.PreferenceManager; +import android.text.TextUtils; import com.topjohnwu.magisk.module.Module; import com.topjohnwu.magisk.module.Repo; import com.topjohnwu.magisk.utils.CallbackHandler; +import com.topjohnwu.magisk.utils.Shell; +import com.topjohnwu.magisk.utils.Utils; import com.topjohnwu.magisk.utils.ValueSortedMap; import java.util.List; public class Global { + public static class Constant { // No global constants now } public static class Info { public static double magiskVersion; - public static double remoteMagiskVersion = -1; public static String magiskVersionString = "(none)"; + public static double remoteMagiskVersion = -1; public static String magiskLink; public static String releaseNoteLink; public static int SNCheckResult = -1; @@ -43,11 +47,28 @@ public class Global { public static boolean shellLogging; public static boolean devLogging; - public static void init(Context context) { - SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); - isDarkTheme = prefs.getBoolean("dark_theme", false); - devLogging = prefs.getBoolean("developer_logging", false); - shellLogging = prefs.getBoolean("shell_logging", false); + } + + public static void init(Context context) { + SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context); + Configs.isDarkTheme = prefs.getBoolean("dark_theme", false); + Configs.devLogging = prefs.getBoolean("developer_logging", false); + Configs.shellLogging = prefs.getBoolean("shell_logging", false); + updateMagiskInfo(); + } + + static void updateMagiskInfo() { + List ret = Shell.sh("getprop magisk.version"); + if (Utils.isValidShellResponse(ret)) { + Info.magiskVersion = -1; + } else { + try { + Info.magiskVersionString = ret.get(0); + Info.magiskVersion = Double.parseDouble(ret.get(0)); + } catch (NumberFormatException e) { + // Custom version don't need to receive updates + Info.magiskVersion = Double.POSITIVE_INFINITY; + } } } diff --git a/app/src/main/java/com/topjohnwu/magisk/InstallFragment.java b/app/src/main/java/com/topjohnwu/magisk/InstallFragment.java index 46c639493..85ec9b1ef 100644 --- a/app/src/main/java/com/topjohnwu/magisk/InstallFragment.java +++ b/app/src/main/java/com/topjohnwu/magisk/InstallFragment.java @@ -46,7 +46,11 @@ public class InstallFragment extends Fragment implements CallbackHandler.EventLi currentVersionTitle.setText(getString(R.string.current_magisk_title, Global.Info.magiskVersionString)); installTitle.setText(getString(R.string.install_magisk_title, Global.Info.remoteMagiskVersion)); flashButton.setOnClickListener(v1 -> { - String bootImage = Global.Info.bootBlock; + String bootImage; + if (spinner.getSelectedItemPosition() > 0) + bootImage = Global.Data.blockList.get(spinner.getSelectedItemPosition() - 1); + else + bootImage = Global.Info.bootBlock; if (bootImage == null) { bootImage = Global.Data.blockList.get(spinner.getSelectedItemPosition() - 1); } diff --git a/app/src/main/java/com/topjohnwu/magisk/LogFragment.java b/app/src/main/java/com/topjohnwu/magisk/LogFragment.java index c985f8485..eeb4d083e 100644 --- a/app/src/main/java/com/topjohnwu/magisk/LogFragment.java +++ b/app/src/main/java/com/topjohnwu/magisk/LogFragment.java @@ -14,6 +14,7 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.design.widget.Snackbar; import android.support.v4.app.ActivityCompat; +import android.text.TextUtils; import android.view.LayoutInflater; import android.view.Menu; import android.view.MenuInflater; @@ -143,12 +144,14 @@ public class LogFragment extends Fragment { case 0: List logList = Utils.readFile(MAGISK_LOG); - StringBuilder llog = new StringBuilder(15 * 10 * 1024); - for (String s : logList) { - llog.append(s).append("\n"); + if (Utils.isValidShellResponse(logList)) { + StringBuilder llog = new StringBuilder(15 * 10 * 1024); + for (String s : logList) { + llog.append(s).append("\n"); + } + return llog.toString(); } - - return llog.toString(); + return ""; case 1: Shell.su("echo > " + MAGISK_LOG); @@ -182,21 +185,24 @@ public class LogFragment extends Fragment { List in = Utils.readFile(MAGISK_LOG); - try (FileWriter out = new FileWriter(targetFile)) { - for (String line : in) { - out.write(line + "\n"); + if (Utils.isValidShellResponse(in)) { + try (FileWriter out = new FileWriter(targetFile)) { + for (String line : in) + out.write(line + "\n"); + return true; + } catch (IOException e) { + e.printStackTrace(); + return false; } - return true; - } catch (IOException e) { - e.printStackTrace(); - return false; } + return false; } return null; } @Override protected void onPostExecute(Object o) { + if (o == null) return; boolean bool; String llog; switch (mode) { @@ -204,7 +210,7 @@ public class LogFragment extends Fragment { case 1: llog = (String) o; progressBar.setVisibility(View.GONE); - if (llog.length() == 0) + if (TextUtils.isEmpty(llog)) txtLog.setText(R.string.log_is_empty); else txtLog.setText(llog); diff --git a/app/src/main/java/com/topjohnwu/magisk/SettingsActivity.java b/app/src/main/java/com/topjohnwu/magisk/SettingsActivity.java index 0093155a5..00d5da9c2 100644 --- a/app/src/main/java/com/topjohnwu/magisk/SettingsActivity.java +++ b/app/src/main/java/com/topjohnwu/magisk/SettingsActivity.java @@ -115,8 +115,8 @@ public class SettingsActivity extends AppCompatActivity { } @Override - public void onStop() { - super.onStop(); + public void onPause() { + super.onPause(); prefs.unregisterOnSharedPreferenceChangeListener(this); } diff --git a/app/src/main/java/com/topjohnwu/magisk/SplashActivity.java b/app/src/main/java/com/topjohnwu/magisk/SplashActivity.java index 625f41082..d19486cf5 100644 --- a/app/src/main/java/com/topjohnwu/magisk/SplashActivity.java +++ b/app/src/main/java/com/topjohnwu/magisk/SplashActivity.java @@ -17,7 +17,7 @@ public class SplashActivity extends AppCompatActivity { super.onCreate(savedInstanceState); SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(getApplicationContext()); - Global.Configs.init(getApplicationContext()); + Global.init(getApplicationContext()); if (Global.Configs.isDarkTheme) { setTheme(R.style.AppTheme_dh); diff --git a/app/src/main/java/com/topjohnwu/magisk/StatusFragment.java b/app/src/main/java/com/topjohnwu/magisk/StatusFragment.java index ef388fcf2..ecda1fc7c 100644 --- a/app/src/main/java/com/topjohnwu/magisk/StatusFragment.java +++ b/app/src/main/java/com/topjohnwu/magisk/StatusFragment.java @@ -59,10 +59,6 @@ public class StatusFragment extends Fragment implements CallbackHandler.EventLis @BindColor(android.R.color.transparent) int trans; int defaultColor; - static { - checkMagiskInfo(); - } - private AlertDialog updateMagisk; @Nullable @@ -162,25 +158,10 @@ public class StatusFragment extends Fragment implements CallbackHandler.EventLis unbinder.unbind(); } - private static void checkMagiskInfo() { - List ret = Shell.sh("getprop magisk.version"); - if (ret.get(0).length() == 0) { - Global.Info.magiskVersion = -1; - } else { - try { - Global.Info.magiskVersionString = ret.get(0); - Global.Info.magiskVersion = Double.parseDouble(ret.get(0)); - } catch (NumberFormatException e) { - // Custom version don't need to receive updates - Global.Info.magiskVersion = Double.POSITIVE_INFINITY; - } - } - } - private void updateUI() { int image, color; - checkMagiskInfo(); + Global.updateMagiskInfo(); if (Global.Info.magiskVersion < 0) { magiskVersionText.setText(R.string.magisk_version_error); @@ -188,23 +169,26 @@ public class StatusFragment extends Fragment implements CallbackHandler.EventLis magiskVersionText.setText(getString(R.string.magisk_version, Global.Info.magiskVersionString)); } - if (Shell.rootStatus == 1) { - color = colorOK; - image = R.drawable.ic_check_circle; - rootStatusText.setText(R.string.proper_root); - rootInfoText.setText(Shell.sh("su -v").get(0)); - - } else { - rootInfoText.setText(R.string.root_info_warning); - if (Shell.rootStatus == 0) { + switch (Shell.rootStatus) { + case 0: color = colorBad; image = R.drawable.ic_cancel; rootStatusText.setText(R.string.not_rooted); - } else { + break; + case 1: + List stats = Shell.sh("su -v"); + if (Utils.isValidShellResponse(stats)) { + color = colorOK; + image = R.drawable.ic_check_circle; + rootStatusText.setText(R.string.proper_root); + rootInfoText.setText(stats.get(0)); + break; + } + case -1: + default: color = colorNeutral; image = R.drawable.ic_help; rootStatusText.setText(R.string.root_error); - } } rootStatusContainer.setBackgroundColor(color); rootStatusText.setTextColor(color); diff --git a/app/src/main/java/com/topjohnwu/magisk/receivers/RepoDlReceiver.java b/app/src/main/java/com/topjohnwu/magisk/receivers/RepoDlReceiver.java index b9830a45d..212f12f1b 100644 --- a/app/src/main/java/com/topjohnwu/magisk/receivers/RepoDlReceiver.java +++ b/app/src/main/java/com/topjohnwu/magisk/receivers/RepoDlReceiver.java @@ -4,7 +4,7 @@ import android.net.Uri; import com.topjohnwu.magisk.R; import com.topjohnwu.magisk.utils.Async; -import com.topjohnwu.magisk.utils.Utils.ByteArrayInOutStream; +import com.topjohnwu.magisk.utils.ByteArrayInOutStream; import com.topjohnwu.magisk.utils.ZipUtils; import java.io.OutputStream; diff --git a/app/src/main/java/com/topjohnwu/magisk/utils/Async.java b/app/src/main/java/com/topjohnwu/magisk/utils/Async.java index ba1a5f634..5989c324e 100644 --- a/app/src/main/java/com/topjohnwu/magisk/utils/Async.java +++ b/app/src/main/java/com/topjohnwu/magisk/utils/Async.java @@ -218,8 +218,9 @@ public class Async { protected boolean unzipAndCheck() { ZipUtils.unzip(mCachedFile, mCachedFile.getParentFile(), "META-INF/com/google/android"); - return Utils.readFile(mCachedFile.getParent() + "/META-INF/com/google/android/updater-script") - .get(0).contains("#MAGISK"); + List ret; + ret = Utils.readFile(mCachedFile.getParent() + "/META-INF/com/google/android/updater-script"); + return Utils.isValidShellResponse(ret) && ret.get(0).contains("#MAGISK"); } @Override @@ -252,6 +253,7 @@ public class Async { "/META-INF/com/google/android/update-binary dummy 1 " + mCachedFile.getPath(), "if [ $? -eq 0 ]; then echo true; else echo false; fi" ); + if (!Utils.isValidShellResponse(ret)) return -1; Logger.dev("FlashZip: Console log:"); for (String line : ret) { Logger.dev(line); @@ -293,7 +295,7 @@ public class Async { Utils.getAlertDialogBuilder(mContext) .setTitle(R.string.reboot_title) .setMessage(R.string.reboot_msg) - .setPositiveButton(R.string.reboot, (dialogInterface1, i) -> Shell.sh("su -c reboot")) + .setPositiveButton(R.string.reboot, (dialogInterface, i) -> Shell.sh("su -c reboot")) .setNegativeButton(R.string.no_thanks, null) .show(); } @@ -323,9 +325,8 @@ public class Async { protected Void doInBackground(Void... params) { if (Shell.rootAccess()) { Global.Data.blockList = Shell.su("ls /dev/block | grep mmc"); - if (Global.Info.bootBlock == null) { + if (Global.Info.bootBlock == null) Global.Info.bootBlock = Utils.detectBootImage(); - } } return null; } diff --git a/app/src/main/java/com/topjohnwu/magisk/utils/ByteArrayInOutStream.java b/app/src/main/java/com/topjohnwu/magisk/utils/ByteArrayInOutStream.java new file mode 100644 index 000000000..5424b985a --- /dev/null +++ b/app/src/main/java/com/topjohnwu/magisk/utils/ByteArrayInOutStream.java @@ -0,0 +1,18 @@ +package com.topjohnwu.magisk.utils; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; + +public class ByteArrayInOutStream extends ByteArrayOutputStream { + public ByteArrayInputStream getInputStream() { + ByteArrayInputStream in = new ByteArrayInputStream(buf, 0, count); + count = 0; + buf = new byte[32]; + return in; + } + + public void setBuffer(byte[] buffer) { + buf = buffer; + count = buffer.length; + } +} diff --git a/app/src/main/java/com/topjohnwu/magisk/utils/Shell.java b/app/src/main/java/com/topjohnwu/magisk/utils/Shell.java index 75dc4080b..7d0ae8cae 100644 --- a/app/src/main/java/com/topjohnwu/magisk/utils/Shell.java +++ b/app/src/main/java/com/topjohnwu/magisk/utils/Shell.java @@ -170,6 +170,7 @@ public class Shell { try { // Process terminated, it means the interactive shell has some issues process.exitValue(); + rootStatus = -1; return null; } catch (IllegalThreadStateException e) { // Process still running, gobble output until done @@ -183,17 +184,22 @@ public class Shell { break; } } - try { STDOUT.join(100); } catch (InterruptedException err) { return null; } + try { STDOUT.join(100); } catch (InterruptedException err) { + rootStatus = -1; + return null; + } } } } } catch (IOException e) { if (!e.getMessage().contains("EPIPE")) { Logger.dev("Shell: Root shell error..."); + rootStatus = -1; return null; } } catch(InterruptedException e) { Logger.dev("Shell: Root shell error..."); + rootStatus = -1; return null; } diff --git a/app/src/main/java/com/topjohnwu/magisk/utils/Utils.java b/app/src/main/java/com/topjohnwu/magisk/utils/Utils.java index d6fe74ccd..3848b0099 100644 --- a/app/src/main/java/com/topjohnwu/magisk/utils/Utils.java +++ b/app/src/main/java/com/topjohnwu/magisk/utils/Utils.java @@ -31,29 +31,32 @@ public class Utils { public static boolean itemExist(boolean root, String path) { String command = "if [ -e " + path + " ]; then echo true; else echo false; fi"; + List ret; if (Shell.rootAccess() && root) { - return Boolean.parseBoolean(Shell.su(command).get(0)); + ret = Shell.su(command); + return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0)); } else { return new File(path).exists(); } } public static boolean commandExists(String s) { - List ret; String command = "if [ -z $(which " + s + ") ]; then echo false; else echo true; fi"; - ret = Shell.sh(command); - return Boolean.parseBoolean(ret.get(0)); + List ret = Shell.sh(command); + return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0)); } public static boolean createFile(String path) { String folder = path.substring(0, path.lastIndexOf('/')); String command = "mkdir -p " + folder + " 2>/dev/null; touch " + path + " 2>/dev/null; if [ -f \"" + path + "\" ]; then echo true; else echo false; fi"; - return Shell.rootAccess() && Boolean.parseBoolean(Shell.su(command).get(0)); + List ret = Shell.su(command); + return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0)); } public static boolean removeItem(String path) { String command = "rm -rf " + path + " 2>/dev/null; if [ -e " + path + " ]; then echo false; else echo true; fi"; - return Shell.rootAccess() && Boolean.parseBoolean(Shell.su(command).get(0)); + List ret = Shell.su(command); + return isValidShellResponse(ret) && Boolean.parseBoolean(ret.get(0)); } public static List getModList(String path) { @@ -118,9 +121,8 @@ public class Utils { "echo \"${BOOTIMAGE##*/}\"" }; List ret = Shell.su(commands); - if (!ret.isEmpty()) { + if (isValidShellResponse(ret)) return ret.get(0); - } return null; } @@ -136,17 +138,14 @@ public class Utils { return !TextUtils.isEmpty(string) && string.toString().toLowerCase().contains(nonNullLowercaseSearch); } - public static class ByteArrayInOutStream extends ByteArrayOutputStream { - public ByteArrayInputStream getInputStream() { - ByteArrayInputStream in = new ByteArrayInputStream(buf, 0, count); - count = 0; - buf = new byte[32]; - return in; - } - public void setBuffer(byte[] buffer) { - buf = buffer; - count = buffer.length; + public static boolean isValidShellResponse(List list) { + if (list != null && list.size() != 0) { + // Check if all empty + for (String res : list) { + if (!TextUtils.isEmpty(res)) return true; + } } + return false; } } \ No newline at end of file diff --git a/app/src/main/java/com/topjohnwu/magisk/utils/ZipUtils.java b/app/src/main/java/com/topjohnwu/magisk/utils/ZipUtils.java index c5f4300d7..3164105a2 100644 --- a/app/src/main/java/com/topjohnwu/magisk/utils/ZipUtils.java +++ b/app/src/main/java/com/topjohnwu/magisk/utils/ZipUtils.java @@ -3,8 +3,6 @@ package com.topjohnwu.magisk.utils; import android.content.Context; import android.util.Pair; -import com.topjohnwu.magisk.utils.Utils.ByteArrayInOutStream; - import org.spongycastle.asn1.ASN1InputStream; import org.spongycastle.asn1.ASN1ObjectIdentifier; import org.spongycastle.asn1.DEROutputStream;