diff --git a/app/src/main/java/org/lsposed/manager/adapters/ScopeAdapter.java b/app/src/main/java/org/lsposed/manager/adapters/ScopeAdapter.java index 5e131cb2f..454c5d139 100644 --- a/app/src/main/java/org/lsposed/manager/adapters/ScopeAdapter.java +++ b/app/src/main/java/org/lsposed/manager/adapters/ScopeAdapter.java @@ -69,6 +69,7 @@ import org.lsposed.manager.R; import org.lsposed.manager.databinding.ItemMasterSwitchBinding; import org.lsposed.manager.databinding.ItemModuleBinding; +import org.lsposed.manager.databinding.ItemScopeFooterBinding; import org.lsposed.manager.ui.dialog.BlurBehindDialogBuilder; import org.lsposed.manager.ui.fragment.AppListFragment; import org.lsposed.manager.ui.fragment.CompileDialogFragment; @@ -126,6 +127,28 @@ public int getItemCount() { } }; + /** + * staticScope says the module's scope list is the whole of it, so the list above shows only the + * apps it claims and this rules it off. Nothing follows for a module that does not claim one. + */ + public RecyclerView.Adapter footerAdaptor = new RecyclerView.Adapter<>() { + @NonNull + @Override + public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) { + return new RecyclerView.ViewHolder(ItemScopeFooterBinding.inflate(activity.getLayoutInflater(), parent, false).getRoot()) { + }; + } + + @Override + public void onBindViewHolder(@NonNull RecyclerView.ViewHolder holder, int position) { + } + + @Override + public int getItemCount() { + return module.staticScope ? 1 : 0; + } + }; + private final OnMainSwitchChangeListener switchBarOnCheckedChangeListener = new OnMainSwitchChangeListener() { @Override public void onSwitchChanged(Switch view, boolean isChecked) { @@ -531,6 +554,11 @@ public void refresh(boolean force) { synchronized (tmpRecList) { tmpRecList.add(application); } + } else if (module.staticScope && !tmpChkList.contains(application)) { + // The module says its scope list is the whole of it, so nothing outside that + // list is offered. An app already enabled outside it still has to be shown, or + // it would keep the module loaded from a row nobody can see to switch off. + return; } else if (shouldHideApp(info, application, tmpChkList)) { return; } diff --git a/app/src/main/java/org/lsposed/manager/ui/fragment/AppListFragment.java b/app/src/main/java/org/lsposed/manager/ui/fragment/AppListFragment.java index f06468f1d..e00a0b621 100644 --- a/app/src/main/java/org/lsposed/manager/ui/fragment/AppListFragment.java +++ b/app/src/main/java/org/lsposed/manager/ui/fragment/AppListFragment.java @@ -93,6 +93,7 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c var concatAdapter = new ConcatAdapter(); concatAdapter.addAdapter(scopeAdapter.switchAdaptor); concatAdapter.addAdapter(scopeAdapter); + concatAdapter.addAdapter(scopeAdapter.footerAdaptor); binding.recyclerView.setAdapter(concatAdapter); binding.recyclerView.setHasFixedSize(true); binding.recyclerView.setLayoutManager(new LinearLayoutManager(requireActivity())); diff --git a/app/src/main/java/org/lsposed/manager/ui/fragment/ModulesFragment.java b/app/src/main/java/org/lsposed/manager/ui/fragment/ModulesFragment.java index 3df1a1c3a..a44afb6b3 100644 --- a/app/src/main/java/org/lsposed/manager/ui/fragment/ModulesFragment.java +++ b/app/src/main/java/org/lsposed/manager/ui/fragment/ModulesFragment.java @@ -34,6 +34,7 @@ import android.text.SpannableStringBuilder; import android.text.TextUtils; import android.text.style.ForegroundColorSpan; +import android.text.style.RelativeSizeSpan; import android.text.style.StyleSpan; import android.text.style.TypefaceSpan; import android.util.SparseArray; @@ -51,6 +52,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.appcompat.widget.SearchView; +import androidx.appcompat.widget.TooltipCompat; import androidx.constraintlayout.widget.ConstraintLayout; import androidx.coordinatorlayout.widget.CoordinatorLayout; import androidx.core.view.MenuProvider; @@ -558,6 +560,7 @@ public void onLoadCleared(@Nullable Drawable placeholder) { sb = new SpannableStringBuilder(); int installXposedVersion = ConfigManager.getXposedApiVersion(); + bindApiBadge(holder.apiBadge, item, installXposedVersion); String warningText = null; if (item.minVersion == 0) { warningText = getString(R.string.no_min_version_specified); @@ -748,12 +751,84 @@ public boolean isLoaded() { return isLoaded && moduleUtil.isModulesLoaded(); } + /** + * The API version a module targets, printed under its icon. The word stays in the ordinary + * secondary colour and the version itself carries the colour, so the eye lands on the one + * part that differs between modules. The whole statement goes in the content description + * and the tooltip, so the colour is never the only thing carrying it. + */ + private void bindApiBadge(TextView badge, ModuleUtil.InstalledModule item, int frameworkApi) { + final String label; + final String status; + final String version; + final int color; + + if (item.legacy) { + // A legacy module reports the original Xposed API, which is numbered separately + // from the one this framework implements, so it is never compared against it. + version = item.minVersion > 0 ? String.valueOf(item.minVersion) : null; + label = version != null + ? getString(R.string.module_api_badge_legacy, item.minVersion) + : getString(R.string.module_api_badge_legacy_unknown); + status = getString(R.string.module_api_status_legacy); + color = android.R.attr.textColorSecondary; + } else if (item.minVersion <= 0 || item.targetVersion <= 0 || frameworkApi <= 0) { + // Both keys are required of a module. Reporting the one it did declare would put a + // version on screen for a module that never stated which API it was built for. + version = null; + label = getString(R.string.module_api_badge_unknown); + status = getString(R.string.module_api_status_unknown); + color = android.R.attr.textColorSecondary; + } else { + version = String.valueOf(item.targetVersion); + label = getString(R.string.module_api_badge, item.targetVersion); + if (item.minVersion > frameworkApi) { + // It declares it needs more than we have, whatever it targets. + status = getString(R.string.module_api_status_unsupported, item.minVersion, frameworkApi); + color = com.google.android.material.R.attr.colorError; + } else if (item.targetVersion == frameworkApi) { + status = getString(R.string.module_api_status_current); + color = androidx.appcompat.R.attr.colorPrimary; + } else { + status = getString(item.targetVersion > frameworkApi + ? R.string.module_api_status_newer + : R.string.module_api_status_older, + item.targetVersion, frameworkApi); + color = android.R.attr.textColorSecondary; + } + } + + final int resolved = ResourceUtils.resolveColor(requireActivity().getTheme(), color); + final SpannableStringBuilder text = new SpannableStringBuilder(label); + final int at = version == null ? -1 : label.lastIndexOf(version); + if (at > 0) { + // Small caps: the capitals of the word set smaller than the version beside them, so + // the word reads as a label and the version reads as the value. + text.setSpan(new RelativeSizeSpan(0.78f), 0, at, Spannable.SPAN_INCLUSIVE_EXCLUSIVE); + text.setSpan(new StyleSpan(Typeface.BOLD), at, label.length(), + Spannable.SPAN_INCLUSIVE_INCLUSIVE); + text.setSpan(new ForegroundColorSpan(resolved), at, label.length(), + Spannable.SPAN_INCLUSIVE_INCLUSIVE); + } else { + text.setSpan(new RelativeSizeSpan(0.78f), 0, label.length(), + Spannable.SPAN_INCLUSIVE_INCLUSIVE); + text.setSpan(new ForegroundColorSpan(resolved), 0, label.length(), + Spannable.SPAN_INCLUSIVE_INCLUSIVE); + } + + badge.setText(text); + badge.setContentDescription(status); + TooltipCompat.setTooltipText(badge, status); + badge.setVisibility(View.VISIBLE); + } + static class ViewHolder extends RecyclerView.ViewHolder { ConstraintLayout root; ImageView appIcon; TextView appName; TextView appDescription; TextView appVersion; + TextView apiBadge; TextView hint; MaterialCheckBox checkBox; @@ -764,6 +839,7 @@ static class ViewHolder extends RecyclerView.ViewHolder { appName = binding.appName; appDescription = binding.description; appVersion = binding.versionName; + apiBadge = binding.apiBadge; hint = binding.hint; checkBox = binding.checkbox; } diff --git a/app/src/main/java/org/lsposed/manager/util/ModuleUtil.java b/app/src/main/java/org/lsposed/manager/util/ModuleUtil.java index 1205b37c5..1fc50843d 100644 --- a/app/src/main/java/org/lsposed/manager/util/ModuleUtil.java +++ b/app/src/main/java/org/lsposed/manager/util/ModuleUtil.java @@ -86,6 +86,9 @@ public static synchronized ModuleUtil getInstance() { } public static int extractIntPart(String str) { + // minApiVersion and targetApiVersion are required of a module, but a third-party module + // omitting one must not take down the module list. + if (str == null) return 0; int result = 0, length = str.length(); for (int offset = 0; offset < length; offset++) { char c = str.charAt(offset); @@ -295,11 +298,20 @@ private InstalledModule(PackageInfo pkg, ZipFile modernModuleApk) { try (modernModuleApk) { var propEntry = modernModuleApk.getEntry("META-INF/xposed/module.prop"); if (propEntry != null) { - var prop = new Properties(); - prop.load(modernModuleApk.getInputStream(propEntry)); - minVersion = extractIntPart(prop.getProperty("minApiVersion")); - targetVersion = extractIntPart(prop.getProperty("targetApiVersion")); - staticScope = TextUtils.equals(prop.getProperty("staticScope"), "true"); + try { + var prop = new Properties(); + prop.load(modernModuleApk.getInputStream(propEntry)); + minVersion = extractIntPart(prop.getProperty("minApiVersion")); + targetVersion = extractIntPart(prop.getProperty("targetApiVersion")); + staticScope = TextUtils.equals(prop.getProperty("staticScope"), "true"); + } catch (IllegalArgumentException e) { + // Properties.load rejects a malformed unicode escape, and nothing read + // out of the file before that point can be trusted either, so the + // module reports no version rather than the defaults above. + minVersion = 0; + targetVersion = 0; + Log.e(App.TAG, "Malformed module.prop in " + pkg.packageName, e); + } } var scopeEntry = modernModuleApk.getEntry("META-INF/xposed/scope.list"); if (scopeEntry != null) { @@ -309,8 +321,11 @@ private InstalledModule(PackageInfo pkg, ZipFile modernModuleApk) { } else { scopeList = Collections.emptyList(); } - } catch (IOException | OutOfMemoryError e) { - Log.e(App.TAG, "Error while closing modern module APK", e); + } catch (IOException | OutOfMemoryError | IllegalArgumentException e) { + // Properties.load is documented to throw IllegalArgumentException for a + // malformed unicode escape. Uncaught, one such module.prop leaves the user + // with an empty module list instead of one unreadable entry. + Log.e(App.TAG, "Error while reading modern module APK", e); } this.minVersion = minVersion; this.targetVersion = targetVersion; diff --git a/app/src/main/res/layout/item_module.xml b/app/src/main/res/layout/item_module.xml index 33980f66a..6c0c59f8a 100644 --- a/app/src/main/res/layout/item_module.xml +++ b/app/src/main/res/layout/item_module.xml @@ -40,12 +40,32 @@ android:id="@+id/app_icon" android:layout_width="@dimen/app_icon_size" android:layout_height="@dimen/app_icon_size" - app:layout_constraintBottom_toBottomOf="parent" + app:layout_constraintBottom_toTopOf="@id/api_badge" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" + app:layout_constraintVertical_chainStyle="packed" tools:ignore="ContentDescription" tools:srcCompat="@tools:sample/backgrounds/scenic" /> + + + + + + + + + + + + + + + diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8d3c4dbb2..469a63577 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -105,6 +105,17 @@ This module requires a newer Xposed version (%d) and thus cannot be activated This module is designed for a newer Xposed version (%d) and thus some functionalities may not work This module does not specify the Xposed version it needs. + API %1$d + API ? + Xposed %1$d + Xposed ? + Targets the API this framework implements + Targets API %1$d, older than the API this framework implements (%2$d) + Targets API %1$d, newer than the API this framework implements (%2$d) + Needs API %1$d, more than this framework implements (%2$d) + Uses the original Xposed API through the legacy bridge + The targeted API version is unknown + Static Scope This module was created for Xposed version %1$d, but due to incompatible changes in version %2$d, it has been disabled This module cannot be loaded because it\'s installed on the SD card, please move it to internal storage Uninstall diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt index 1306ff550..48747e111 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt @@ -396,7 +396,8 @@ object VectorService : IDaemonService.Stub() { } } } - .onFailure { runCatching { iCallback.onScopeRequestFailed(it.message) } } + // onScopeRequestFailed declares @NonNull, and Throwable.message is frequently null. + .onFailure { runCatching { iCallback.onScopeRequestFailed(it.message ?: it.toString()) } } NotificationManager.cancelNotification( NotificationManager.SCOPE_CHANNEL_ID, packageName, userId) diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt index cfeb9d107..2c34d7d74 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt @@ -34,6 +34,13 @@ object ConfigCache { val dbHelper = Database() // Kept public for PreferenceStore and ModuleDatabase + // Module package -> the packages it claims, for modules whose module.prop fixes the scope. + // Absent means the module places no restriction on its scope. + @Volatile private var staticScopes: Map> = emptyMap() + + /** The packages [modulePackage] claims, or null when it does not fix its scope. */ + fun staticScopeOf(modulePackage: String): Set? = staticScopes[modulePackage] + private val cacheUpdateChannel = Channel(Channel.CONFLATED) init { @@ -123,6 +130,7 @@ object ConfigCache { val oldState = state val newModules = mutableMapOf() + val newStaticScopes = mutableMapOf>() val obsoleteModules = mutableSetOf() val obsoletePaths = mutableMapOf() @@ -166,6 +174,9 @@ object ConfigCache { File(appInfo.sourceDir).parent == File(apkPath).parent) { if (oldModule.appId == -1) oldModule.applicationInfo = appInfo + // This path skips re-reading the APK, so what the module claims has to be carried + // over; the new map replaces the old one wholesale and would otherwise lose it. + staticScopes[pkgName]?.let { newStaticScopes[pkgName] = it } newModules[pkgName] = oldModule continue } @@ -180,6 +191,8 @@ object ConfigCache { obsoletePaths[pkgName] = realApkPath } + FileSystem.readStaticScope(apkPath)?.let { newStaticScopes[pkgName] = it } + val preLoadedApk = FileSystem.loadModule(apkPath, state.isDexObfuscateEnabled) if (preLoadedApk != null) { val module = @@ -204,6 +217,17 @@ object ConfigCache { obsoletePaths.forEach { (pkg, path) -> ModuleDatabase.updateModuleApkPath(pkg, path, true) } } + staticScopes = newStaticScopes + // Rows can predate the module declaring a fixed scope, or come from an older build that let + // them in. Dropping them here is what makes the scope actually fixed rather than merely + // unreachable through the manager, and it runs before the scope table is read below. + newStaticScopes.forEach { (modulePkg, claimed) -> + val dropped = ModuleDatabase.pruneScopeToClaimed(modulePkg, claimed) + if (dropped > 0) { + Log.i(TAG, "Dropped $dropped app(s) outside the static scope of $modulePkg") + } + } + val newScopes = mutableMapOf>() db.query( "scope INNER JOIN modules ON scope.mid = modules.mid", diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/FileSystem.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/FileSystem.kt index 5eeb7a1b9..bcbeb3ecc 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/FileSystem.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/FileSystem.kt @@ -27,6 +27,7 @@ import java.nio.file.attribute.PosixFilePermissions import java.time.Instant import java.time.ZoneId import java.time.format.DateTimeFormatter +import java.util.Properties import java.util.zip.ZipEntry import java.util.zip.ZipFile import java.util.zip.ZipOutputStream @@ -160,6 +161,34 @@ object FileSystem { return memory } + /** + * The packages a module claims, when its module.prop fixes the scope. Null when it does not, so + * a caller can tell "claims nothing" from "claims no restriction". + * + * staticScope is documented as "the module scope is fixed and users should not apply the module + * on apps outside the scope list". Enforcing that in the manager alone leaves the socket CLI, a + * backup restore and the module's own requestScope walking straight past it, so the daemon has + * to know about it too. + */ + fun readStaticScope(apkPath: String): Set? = + runCatching { + ZipFile(File(apkPath)).use { zip -> + val props = + Properties().apply { + zip.getEntry("META-INF/xposed/module.prop")?.let { entry -> + runCatching { zip.getInputStream(entry).use { load(it) } } + } + } + if (!props.getProperty("staticScope").toBoolean()) return@use null + val entry = zip.getEntry("META-INF/xposed/scope.list") ?: return@use emptySet() + zip.getInputStream(entry).bufferedReader().useLines { lines -> + lines.filter { it.isNotEmpty() }.toSet() + } + } + } + .onFailure { Log.w(TAG, "Cannot read the scope list of $apkPath", it) } + .getOrNull() + /** Parses the module APK, extracts init lists, and loads DEXes into SharedMemory. */ fun loadModule(apkPath: String, obfuscate: Boolean): PreLoadedApk? { val file = File(apkPath) @@ -170,23 +199,32 @@ object FileSystem { val moduleClassNames = mutableListOf() val moduleLibraryNames = mutableListOf() var isLegacy = false + var exceptionPassthrough = false runCatching { ZipFile(file).use { zip -> - // Parse module.prop to get targetApiVersion + // module.prop is specified as Java Properties format. Parsing it by hand mishandles + // ':' as a separator, '!' comments, escapes and line continuations, and the manager + // app already reads the same file with Properties.load. val props = - zip.getEntry("META-INF/xposed/module.prop")?.let { entry -> - zip.getInputStream(entry).bufferedReader().useLines { lines -> - lines - .filter { it.contains("=") } - .associate { - val parts = it.split("=", limit = 2) - parts[0].trim() to parts[1].trim() - } + Properties().apply { + zip.getEntry("META-INF/xposed/module.prop")?.let { entry -> + // Properties.load rejects a malformed \uXXXX escape, which the old hand-rolled + // parser tolerated. Keep that tolerance: a bad module.prop must not make the + // APK unloadable, not least because a legacy module is selected by + // assets/xposed_init and needs no module.prop at all. + runCatching { zip.getInputStream(entry).use { load(it) } } + .onFailure { Log.w(TAG, "Malformed module.prop in $apkPath", it) } } - } ?: emptyMap() + } + + val targetApi = props.getProperty("targetApiVersion")?.trim()?.toIntOrNull() ?: 0 + // The module-wide mode ExceptionMode.DEFAULT resolves to. Anything that is not + // "passthrough" - absent, misspelled, or an explicit "protective" - keeps the + // protective default the API specifies. + exceptionPassthrough = + props.getProperty("exceptionMode")?.trim().equals("passthrough", true) - val targetApi = props["targetApiVersion"]?.toIntOrNull() ?: 0 val hasLegacyFile = zip.getEntry("assets/xposed_init") != null // Determine Loading Strategy based on Priority: API 101+ > Legacy > API 100 @@ -263,6 +301,7 @@ object FileSystem { this.moduleClassNames = moduleClassNames this.moduleLibraryNames = moduleLibraryNames this.legacy = isLegacy + this.exceptionPassthrough = exceptionPassthrough } return preLoadedApk diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt index 40210aace..134c80ea8 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt @@ -48,6 +48,15 @@ object ModuleDatabase { } fun setModuleScope(packageName: String, scope: MutableList): Boolean { + // Last line of defence for staticScope. The manager, the socket CLI, a backup restore and a + // module's own requestScope all end up here, so refusing here covers every one of them. + ConfigCache.staticScopeOf(packageName)?.let { claimed -> + val beyond = scope.map { it.packageName }.distinct().filterNot { claimed.contains(it) } + if (beyond.isNotEmpty()) { + Log.w(TAG, "$packageName fixes its scope; refusing to add ${beyond.joinToString()}") + return false + } + } enableModule(packageName) val db = ConfigCache.dbHelper.writableDatabase db.beginTransaction() @@ -76,6 +85,33 @@ object ModuleDatabase { return true } + /** + * Drops every scope row of [packageName] outside [claimed]. Called from within a cache update, so + * it deliberately does not ask for another one. + * + * @return how many rows went. + */ + fun pruneScopeToClaimed(packageName: String, claimed: Set): Int { + val db = ConfigCache.dbHelper.writableDatabase + return runCatching { + val mid = + db.compileStatement("SELECT mid FROM modules WHERE module_pkg_name = ?") + .apply { bindString(1, packageName) } + .simpleQueryForLong() + val placeholders = claimed.joinToString(",") { "?" } + if (claimed.isEmpty()) { + db.delete("scope", "mid = ?", arrayOf(mid.toString())) + } else { + db.delete( + "scope", + "mid = ? AND app_pkg_name NOT IN ($placeholders)", + arrayOf(mid.toString()) + claimed.toTypedArray()) + } + } + .onFailure { Log.e(TAG, "Failed to prune the scope of $packageName", it) } + .getOrDefault(0) + } + fun removeModuleScope(packageName: String, scopePackageName: String, userId: Int): Boolean { if (packageName == "lspd" || (scopePackageName == "system" && userId != 0)) return false val db = ConfigCache.dbHelper.writableDatabase diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt index c71715102..16341343d 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt @@ -106,6 +106,18 @@ object CliHandler { val modulePkg = request.targets[0] val apps = request.targets.drop(1) + // staticScope fixes the scope, so the CLI may still read it and take apps out of it, but not + // put new ones in. Checked here as well as in the database so the answer names the packages. + fun rejectBeyondStaticScope(apps: List) { + val claimed = ConfigCache.staticScopeOf(modulePkg) ?: return + val beyond = apps.map { it.substringBefore('/') }.filterNot { claimed.contains(it) } + if (beyond.isNotEmpty()) { + throw IllegalArgumentException( + "$modulePkg fixes its scope in module.prop, so ${beyond.joinToString()} cannot be " + + "added. It claims: ${claimed.sorted().joinToString().ifEmpty { "nothing" }}.") + } + } + return when (request.action) { "ls" -> { val scope = @@ -115,6 +127,7 @@ object CliHandler { } "add" -> { if (apps.isEmpty()) throw IllegalArgumentException("No target apps provided.") + rejectBeyondStaticScope(apps) val scope = ConfigCache.getModuleScope(modulePkg) ?: mutableListOf() apps.forEach { appStr -> @@ -135,6 +148,7 @@ object CliHandler { "set" -> { if (apps.isEmpty()) throw IllegalArgumentException("No target apps provided for scope overwrite.") + rejectBeyondStaticScope(apps) val scope = mutableListOf() apps.forEach { appStr -> val parts = appStr.split("/") diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/InjectedModuleService.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/InjectedModuleService.kt index cff900d01..3765b7940 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/InjectedModuleService.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/InjectedModuleService.kt @@ -19,8 +19,11 @@ private const val TAG = "VectorInjectedModuleService" class InjectedModuleService(private val packageName: String) : ILSPInjectedModuleService.Stub() { - // Tracks active RemotePreferenceCallbacks linked by config group - private val callbacks = ConcurrentHashMap>() + // Tracks active RemotePreferenceCallbacks linked by config group. Preferences are stored per + // Android user, so a registration is only interested in updates made by its own user. + private data class Subscriber(val userId: Int, val callback: IRemotePreferenceCallback) + + private val callbacks = ConcurrentHashMap>() override fun getFrameworkProperties(): Long { var prop = IXposedService.PROP_CAP_SYSTEM or IXposedService.PROP_CAP_REMOTE @@ -41,21 +44,26 @@ class InjectedModuleService(private val packageName: String) : ILSPInjectedModul if (callback != null) { val groupCallbacks = callbacks.getOrPut(group) { ConcurrentHashMap.newKeySet() } - groupCallbacks.add(callback) - runCatching { callback.asBinder().linkToDeath({ groupCallbacks.remove(callback) }, 0) } + val subscriber = Subscriber(userId, callback) + groupCallbacks.add(subscriber) + runCatching { callback.asBinder().linkToDeath({ groupCallbacks.remove(subscriber) }, 0) } .onFailure { Log.w(TAG, "requestRemotePreferences linkToDeath failed", it) } } return bundle } - override fun openRemoteFile(path: String): ParcelFileDescriptor { - FileSystem.ensureModuleFilePath(path) + override fun openRemoteFile(path: String): ParcelFileDescriptor? { + // XposedInterface#openRemoteFile documents FileNotFoundException for a missing *or* forbidden + // path. Returning null lets VectorContext raise exactly that; throwing here surfaced a + // RemoteException for a missing file and an IllegalArgumentException for a rejected path. val userId = Binder.getCallingUid() / PER_USER_RANGE return runCatching { + FileSystem.ensureModuleFilePath(path) val dir = FileSystem.resolveModuleDir(packageName, "files", userId, -1) ParcelFileDescriptor.open(dir.resolve(path).toFile(), ParcelFileDescriptor.MODE_READ_ONLY) } - .getOrElse { throw RemoteException(it.message) } + .onFailure { Log.w(TAG, "Cannot open remote file $path for $packageName: ${it.message}") } + .getOrNull() } override fun getRemoteFileList(): Array { @@ -67,11 +75,13 @@ class InjectedModuleService(private val packageName: String) : ILSPInjectedModul .getOrElse { throw RemoteException(it.message) } } - // Called by ModuleService when prefs are updated globally - fun onUpdateRemotePreferences(group: String, diff: Bundle) { + // Called by ModuleService when the module app has changed the group for one Android user. + fun onUpdateRemotePreferences(group: String, userId: Int, diff: Bundle) { val groupCallbacks = callbacks[group] ?: return - for (callback in groupCallbacks) { - runCatching { callback.onUpdate(diff) }.onFailure { groupCallbacks.remove(callback) } + for (subscriber in groupCallbacks) { + if (subscriber.userId != userId) continue + runCatching { subscriber.callback.onUpdate(diff) } + .onFailure { groupCallbacks.remove(subscriber) } } } } diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleService.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleService.kt index 26d0a25bb..056580728 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleService.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleService.kt @@ -117,12 +117,30 @@ class ModuleService(private val loadedModule: Module) : IXposedService.Stub() { override fun getScope(): List { ensureModule() - return ConfigCache.getModuleScope(loadedModule.packageName)?.map { it.packageName } + // The scope table has one row per (app, user), so a module enabled for several users saw the + // same package repeatedly. A scope is a set of package names. + return ConfigCache.getModuleScope(loadedModule.packageName)?.map { it.packageName }?.distinct() ?: emptyList() } override fun requestScope(packages: List, callback: IXposedScopeCallback) { val userId = ensureModule() + if (packages.isEmpty()) { + // Nothing was asked for, so the request is trivially satisfied. Returning without touching + // the callback would leave the module waiting forever. + callback.onScopeRequestApproved(emptyList()) + return + } + // A module that fixed its own scope in module.prop does not get to ask for more of it at + // runtime. Prompting the user here would make "fixed" mean nothing. + ConfigCache.staticScopeOf(loadedModule.packageName)?.let { claimed -> + val beyond = packages.filterNot { claimed.contains(it) } + if (beyond.isNotEmpty()) { + callback.onScopeRequestFailed( + "This module declares a static scope, so ${beyond.joinToString()} cannot be added") + return + } + } if (!PreferenceStore.isScopeRequestBlocked(loadedModule.packageName)) { packages.forEach { pkg -> NotificationManager.requestModuleScope(loadedModule.packageName, userId, pkg, callback) @@ -154,6 +172,12 @@ class ModuleService(private val loadedModule: Module) : IXposedService.Stub() { val userId = ensureModule() val values = mutableMapOf() + // RemotePreferences.Editor always writes this key, and sets it for edit().clear(). Ignoring it + // left every key the module app just cleared in place. + if (diff.getBoolean("clear", false)) { + PreferenceStore.deleteModulePrefs(loadedModule.packageName, userId, group) + } + diff.getSerializable("delete")?.let { deletes -> (deletes as Set<*>).forEach { values[it as String] = null } } @@ -163,13 +187,19 @@ class ModuleService(private val loadedModule: Module) : IXposedService.Stub() { runCatching { PreferenceStore.updateModulePrefs(loadedModule.packageName, userId, group, values) - (loadedModule.service as? InjectedModuleService)?.onUpdateRemotePreferences(group, diff) + (loadedModule.service as? InjectedModuleService) + ?.onUpdateRemotePreferences(group, userId, diff) } .getOrElse { throw RemoteException(it.message) } } override fun deleteRemotePreferences(group: String) { - PreferenceStore.deleteModulePrefs(loadedModule.packageName, ensureModule(), group) + val userId = ensureModule() + PreferenceStore.deleteModulePrefs(loadedModule.packageName, userId, group) + // Hooked processes hold an in-process cache of the group; without this they keep serving the + // deleted values until their process restarts. + (loadedModule.service as? InjectedModuleService) + ?.onUpdateRemotePreferences(group, userId, Bundle().apply { putBoolean("clear", true) }) } override fun listRemoteFiles(): Array { diff --git a/legacy/src/main/java/de/robv/android/xposed/XposedInit.java b/legacy/src/main/java/de/robv/android/xposed/XposedInit.java index 9c23e3380..ba197f5df 100644 --- a/legacy/src/main/java/de/robv/android/xposed/XposedInit.java +++ b/legacy/src/main/java/de/robv/android/xposed/XposedInit.java @@ -216,7 +216,14 @@ public static void loadLegacyModules() { }); } + private static final AtomicBoolean modulesLoaded = new AtomicBoolean(false); + public static void loadModules(ActivityThread at) { + // A late-injected system server calls this directly because its ActivityThread.attach + // already ran; guard so the normal path cannot load a second generation on top. + if (!modulesLoaded.compareAndSet(false, true)) { + return; + } var packages = (ArrayMap) XposedHelpers.getObjectField(at, "mPackages"); VectorServiceClient.INSTANCE.getModulesList().forEach(module -> { loadedModules.put(module.packageName, Optional.empty()); diff --git a/native/src/jni/hook_bridge.cpp b/native/src/jni/hook_bridge.cpp index 5bc19975f..2cbae6777 100644 --- a/native/src/jni/hook_bridge.cpp +++ b/native/src/jni/hook_bridge.cpp @@ -1,9 +1,14 @@ #include #include +#include +#include +#include #include +#include #include #include +#include #include "jni/jni_bridge.h" #include "jni/jni_hooks.h" @@ -554,23 +559,114 @@ VECTOR_DEF_NATIVE_METHOD(jobjectArray, HookBridge, callbackSnapshot, jclass call } /** - * @brief Retrieves the static initializer () of a class as a Method object. - * @param target_class The class to inspect. - * @return A Method object for the static initializer, or null if it doesn't exist. + * @brief Reports whether the pages spanning [addr, addr + len) are mapped. + * + * msync on an unmapped range fails with ENOMEM, which turns a read that would raise SIGSEGV into + * an answer. Used for the one candidate below that cannot be bracketed by known-good members. */ -VECTOR_DEF_NATIVE_METHOD(jobject, HookBridge, getStaticInitializer, jclass target_class) { - // is the internal name for a static initializer. - // Its signature is always ()V (no arguments, void return). - jmethodID mid = env->GetStaticMethodID(target_class, "", "()V"); - if (!mid) { - // If GetStaticMethodID fails, it throws an exception. - // We clear it and return null to let the Java side handle it gracefully. - env->ExceptionClear(); - return nullptr; +static bool IsMapped(uintptr_t addr, size_t len) { + static const size_t page = static_cast(sysconf(_SC_PAGESIZE)); + if (page == 0) return false; + const uintptr_t start = addr & ~(page - 1); + const size_t span = ((addr + len) - start + page - 1) & ~(page - 1); + return msync(reinterpret_cast(start), span, MS_ASYNC) == 0; +} + +/** + * @brief Finds a class's static initializer without initializing the class. + * + * GetStaticMethodID cannot be used: JNI specifies that resolving a method id initializes the + * class, which is exactly the event a hook exists to observe. Java reflection cannot be + * used either, because it hides entirely. + * + * ART stores a class's ArtMethods in one contiguous array: direct methods, then declared virtual + * methods, then methods copied in from interfaces. Reflection reports every one of the first two + * groups except , so the addresses the caller passes are a run of evenly spaced slots with + * missing from it. Finding the hole finds the method, and a hole is bracketed by two + * members that are known to be inside the array, so nothing has to be assumed about where the + * array begins. + * + * The hole is only at the very start - below every address the caller can see - when no declared + * direct method sorts ahead of "". Dex method ids are ordered by name, and while "" + * does sort after it, '$' and '-' do not: an enum's $values, and the -$$Nest$ accessors javac + * emits for nestmates, both take the first slot instead. So the slot below the run is one + * possibility among several rather than the answer, and it is the only one that can fall outside + * the array, which is what a class with no static initializer looks like. It is checked last and + * only once its page is known to be mapped. + * + * Every candidate is then confirmed by two plain word reads before anything dereferences it: its + * declaring class must match the run's, and its access flags must say static constructor. + * + * The caller passes ArtMethod addresses read from java.lang.reflect.Executable.artMethod rather + * than jmethodIDs, because a Java-debuggable process hands out index based ids instead of + * pointers. + * + * @return The static initializer as a reflected object, or nullptr if the class has none or the + * layout is not what this relies on. + */ +VECTOR_DEF_NATIVE_METHOD(jobject, HookBridge, findStaticInitializer, jclass target_class, + jlongArray art_methods, jlong art_method_size) { + const jsize count = art_methods ? env->GetArrayLength(art_methods) : 0; + // One member is enough to anchor the run; the element size is a property of the runtime, so + // the caller derives it once elsewhere. A class whose only members are and an + // implicit constructor leaves exactly one member visible to reflection, and that is the + // commonest shape for wanting this hook. + if (count < 1) return nullptr; + + std::vector ids(count); + { + std::vector raw(count); + env->GetLongArrayRegion(art_methods, 0, count, raw.data()); + for (jsize i = 0; i < count; ++i) { + auto id = static_cast(raw[i]); + if (id < 0x1000 || (id % alignof(void *)) != 0) return nullptr; + ids[i] = id; + } + } + + std::sort(ids.begin(), ids.end()); + const auto stride = static_cast(art_method_size); + // An ArtMethod is a few dozen bytes on every supported release; refuse rather than guess when + // the size is not one a contiguous method array could have. + if (stride < 16 || stride > 128 || (stride % alignof(void *)) != 0) return nullptr; + + // Slots the caller cannot see. Interior ones come first because each is bracketed by a member + // on either side, so it is inside the array whatever the class turns out to look like. A class + // may have more than one hole: reflection also hides members the hidden API policy blocks. + constexpr size_t kMaxCandidates = 64; + std::vector candidates; + for (size_t i = 1; i < ids.size(); ++i) { + const uintptr_t delta = ids[i] - ids[i - 1]; + // Uneven spacing means these are not one run of ArtMethods and none of this holds. + if (delta == 0 || (delta % stride) != 0) return nullptr; + for (uintptr_t slot = ids[i - 1] + stride; slot < ids[i]; slot += stride) { + if (candidates.size() >= kMaxCandidates) return nullptr; + candidates.push_back(slot); + } + } + // The slot below the run, which may be outside the array altogether. + const uintptr_t below = ids.front() - stride; + if (IsMapped(below, 2 * sizeof(uint32_t))) candidates.push_back(below); + + // ArtMethod starts with GcRoot declaring_class_ followed by uint32_t + // access_flags_, so both live in the first eight bytes of a slot. + const auto declaring_of = [](uintptr_t m) { return *reinterpret_cast(m); }; + const auto flags_of = [](uintptr_t m) { + return *reinterpret_cast(m + sizeof(uint32_t)); + }; + + constexpr uint32_t kAccStatic = 0x0008; + constexpr uint32_t kAccConstructor = 0x00010000; + const uint32_t declaring = declaring_of(ids.front()); + + for (const uintptr_t candidate : candidates) { + if (declaring_of(candidate) != declaring) continue; + const uint32_t flags = flags_of(candidate); + if ((flags & kAccStatic) == 0 || (flags & kAccConstructor) == 0) continue; + return env->ToReflectedMethod(target_class, reinterpret_cast(candidate), + JNI_TRUE); } - // Convert the method ID to a java.lang.reflect.Method object. - // The last parameter must be JNI_TRUE because it's a static method. - return env->ToReflectedMethod(target_class, mid, JNI_TRUE); + return nullptr; } // Array of native method descriptors for JNI registration. @@ -593,8 +689,8 @@ static JNINativeMethod gMethods[] = { VECTOR_NATIVE_METHOD(HookBridge, callbackSnapshot, "(Ljava/lang/Class;Ljava/lang/reflect/" "Executable;)[[Ljava/lang/Object;"), - VECTOR_NATIVE_METHOD(HookBridge, getStaticInitializer, - "(Ljava/lang/Class;)Ljava/lang/reflect/Method;"), + VECTOR_NATIVE_METHOD(HookBridge, findStaticInitializer, + "(Ljava/lang/Class;[JJ)Ljava/lang/reflect/Executable;"), }; /** diff --git a/services/daemon-service/src/main/aidl/org/lsposed/lspd/models/PreLoadedApk.aidl b/services/daemon-service/src/main/aidl/org/lsposed/lspd/models/PreLoadedApk.aidl index a9b5b9ef3..45a96ce3b 100644 --- a/services/daemon-service/src/main/aidl/org/lsposed/lspd/models/PreLoadedApk.aidl +++ b/services/daemon-service/src/main/aidl/org/lsposed/lspd/models/PreLoadedApk.aidl @@ -5,4 +5,7 @@ parcelable PreLoadedApk { List moduleClassNames; List moduleLibraryNames; boolean legacy; + // module.prop 'exceptionMode', normalised by the daemon. false, the value an absent key + // parses to, is PROTECTIVE - what ExceptionMode.DEFAULT is specified to fall back to. + boolean exceptionPassthrough; } diff --git a/services/daemon-service/src/main/aidl/org/lsposed/lspd/service/ILSPInjectedModuleService.aidl b/services/daemon-service/src/main/aidl/org/lsposed/lspd/service/ILSPInjectedModuleService.aidl index c77ece391..71751c3a9 100644 --- a/services/daemon-service/src/main/aidl/org/lsposed/lspd/service/ILSPInjectedModuleService.aidl +++ b/services/daemon-service/src/main/aidl/org/lsposed/lspd/service/ILSPInjectedModuleService.aidl @@ -7,7 +7,7 @@ interface ILSPInjectedModuleService { Bundle requestRemotePreferences(String group, IRemotePreferenceCallback callback); - ParcelFileDescriptor openRemoteFile(String path); + @nullable ParcelFileDescriptor openRemoteFile(String path); String[] getRemoteFileList(); } diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/VectorContext.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/VectorContext.kt index d92faabb6..efb4da78f 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/VectorContext.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/VectorContext.kt @@ -4,11 +4,14 @@ import android.content.SharedPreferences import android.content.pm.ApplicationInfo import android.os.ParcelFileDescriptor import io.github.libxposed.api.XposedInterface +import io.github.libxposed.api.XposedInterface.ExceptionMode import io.github.libxposed.api.XposedModuleInterface.* import java.io.FileNotFoundException import java.lang.reflect.Constructor import java.lang.reflect.Executable +import java.lang.reflect.Field import java.lang.reflect.Method +import java.lang.reflect.Modifier import java.util.concurrent.ConcurrentHashMap import org.lsposed.lspd.service.ILSPInjectedModuleService import org.lsposed.lspd.util.Utils.Log @@ -17,6 +20,38 @@ import org.matrix.vector.impl.hooks.VectorHookBuilder import org.matrix.vector.impl.hooks.VectorMethodInvoker import org.matrix.vector.nativebridge.HookBridge +private const val TAG = "VectorContext" + +/** Smallest positive gap between the sorted addresses, i.e. one ArtMethod. */ +private fun strideOf(addresses: LongArray): Long { + val sorted = addresses.sorted() + var stride = Long.MAX_VALUE + for (i in 1 until sorted.size) { + val delta = sorted[i] - sorted[i - 1] + if (delta in 1 until stride) stride = delta + } + return if (stride == Long.MAX_VALUE) 0L else stride +} + +/** + * The size of one ArtMethod, which is a property of the runtime rather than of any class. Derived + * once from a class with plenty of members so that a class showing only one member to reflection + * can still be measured against it. + */ +private val artMethodSize: Long by lazy { + val field = artMethodField ?: return@lazy 0L + runCatching { strideOf(Any::class.java.declaredMethods.map { field.getLong(it) }.toLongArray()) } + .getOrDefault(0L) +} + +/** ART keeps the ArtMethod address of a reflected member in this field. */ +private val artMethodField: Field? by lazy { + runCatching { + Executable::class.java.getDeclaredField("artMethod").apply { isAccessible = true } + } + .getOrNull() +} + /** * Main framework context implementation. Provides modules with capabilities to hook executables, * request invokers, and interact with the system. @@ -25,6 +60,8 @@ class VectorContext( private val packageName: String, private val applicationInfo: ApplicationInfo, private val service: ILSPInjectedModuleService, + // What ExceptionMode.DEFAULT resolves to for this module, from module.prop. + private val defaultExceptionMode: ExceptionMode = ExceptionMode.PROTECTIVE, ) : XposedInterface { private val remotePrefs = ConcurrentHashMap() @@ -40,16 +77,63 @@ class VectorContext( } override fun hook(origin: Executable): XposedInterface.HookBuilder { - return VectorHookBuilder(origin) + return VectorHookBuilder(origin, defaultExceptionMode) } override fun hookClassInitializer(origin: Class<*>): XposedInterface.HookBuilder { val clinit = - HookBridge.getStaticInitializer(origin) + findStaticInitializer(origin) ?: throw IllegalArgumentException("Class ${origin.name} has no static initializer") - return VectorHookBuilder(clinit) + return VectorHookBuilder(asSyntheticMethod(clinit), defaultExceptionMode) + } + + /** + * hookClassInitializer documents Chain#getExecutable as "a synthetic Method representing the + * static initializer", but ART reflects as a Constructor because it carries + * ACC_CONSTRUCTOR. A Method is allocated without running a constructor and given the same + * Executable state, so it names the same ArtMethod while presenting the documented type. + */ + private fun asSyntheticMethod(clinit: Executable): Executable { + if (clinit is Method) return clinit + return runCatching { + val method = HookBridge.allocateObject(Method::class.java) + for (field in Executable::class.java.declaredFields) { + if (Modifier.isStatic(field.modifiers)) continue + field.isAccessible = true + field.set(method, field.get(clinit)) + } + method as Executable + } + .onFailure { + // Falling back keeps the hook working, at the cost of handing the hooker the type + // ART produced instead of the one the interface promises. + Log.w(TAG, "Cannot present of ${'$'}{clinit.declaringClass.name} as a Method", it) + } + .getOrDefault(clinit) } + /** + * Resolving through JNI runs the class's static initializer, which is the event a hook + * on it exists to observe, so locate it from the method layout instead. Reflection over + * declared members does not initialize the class, and ArtMethod addresses are read from the + * reflected objects rather than through jmethodIDs, which a debuggable process hands out as + * indices. + */ + private fun findStaticInitializer(origin: Class<*>): Executable? = + runCatching { + val field = artMethodField ?: return null + val members = ArrayList() + members.addAll(origin.declaredConstructors) + members.addAll(origin.declaredMethods) + val addresses = LongArray(members.size) { field.getLong(members[it]) } + // Prefer the class's own spacing; fall back to the runtime-wide size for a class + // that shows fewer than two members. + val stride = strideOf(addresses).takeIf { it > 0 } ?: artMethodSize + HookBridge.findStaticInitializer(origin, addresses, stride) + } + .onFailure { Log.w(TAG, "Static initializer lookup failed for ${origin.name}", it) } + .getOrNull() + override fun deoptimize(executable: Executable): Boolean { return HookBridge.deoptimizeMethod(executable) } diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/VectorRemotePreferences.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/VectorRemotePreferences.kt index db50b0cc6..2c66241c9 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/VectorRemotePreferences.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/VectorRemotePreferences.kt @@ -33,6 +33,13 @@ internal class VectorRemotePreferences(service: ILSPInjectedModuleService, group override fun onUpdate(bundle: Bundle) { val changes = ArraySet() + // Sent for edit().clear() and for deleteRemotePreferences. Without this the cache + // keeps serving values the module app already removed. + if (bundle.getBoolean("clear", false)) { + changes.addAll(map.keys) + map.clear() + } + if (bundle.containsKey("delete")) { val deletes = bundle.getSerializableCompat>("delete") as? Set diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorModuleManager.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorModuleManager.kt index a5afba016..05fda8b70 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorModuleManager.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorModuleManager.kt @@ -2,6 +2,7 @@ package org.matrix.vector.impl.core import android.os.Build import android.os.Process +import io.github.libxposed.api.XposedInterface.ExceptionMode import io.github.libxposed.api.XposedModule import io.github.libxposed.api.XposedModuleInterface.ModuleLoadedParam import java.io.File @@ -62,6 +63,9 @@ object VectorModuleManager { packageName = module.packageName, applicationInfo = module.applicationInfo, service = module.service, // Our IPC client + defaultExceptionMode = + if (module.file.exceptionPassthrough) ExceptionMode.PASSTHROUGH + else ExceptionMode.PROTECTIVE, ) // Instantiate the module entry classes diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorStartup.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorStartup.kt index 4020d5e63..7614a4866 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorStartup.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/core/VectorStartup.kt @@ -1,9 +1,12 @@ package org.matrix.vector.impl.core +import android.app.ActivityThread import android.os.Build import android.os.IBinder import dalvik.system.DexFile +import org.lsposed.lspd.util.Utils import org.lsposed.lspd.service.ILSPApplicationService +import org.matrix.vector.impl.di.VectorBootstrap import org.matrix.vector.impl.hookers.* import org.matrix.vector.impl.hooks.VectorHookBuilder @@ -82,6 +85,27 @@ object VectorStartup { if (classLoader != null) { // Manually trigger the routines that the hooks normally would HandleSystemServerProcessHooker.initSystemServer(classLoader, isLate = true) + + // ActivityThread.attach ran before we were injected, so AppAttachHooker will + // never fire here and nothing else instantiates modules. Without this, + // dispatchSystemServerLoaded below would notify an empty module set. + val activityThread = ActivityThread.currentActivityThread() + if (activityThread != null) { + VectorBootstrap.withLegacy { it.loadModules(activityThread) } + } else { + Utils.logW( + "Late system server injection: no current ActivityThread, modules cannot be loaded" + ) + } + + // Say plainly what this is. onSystemServerStarting is documented as "system + // server is ready to start critical services", which is no longer true here - + // the services are already running. Modules that key off it should treat a + // late dispatch as best effort. + Utils.logW( + "Late system server injection: dispatching onSystemServerStarting after " + + "system server has already started; critical services are live" + ) StartBootstrapServicesHooker.dispatchSystemServerLoaded(classLoader) } } diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/hookers/LoadedApkHookers.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/hookers/LoadedApkHookers.kt index 33c691988..bac9d9cc2 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/hookers/LoadedApkHookers.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/hookers/LoadedApkHookers.kt @@ -39,25 +39,42 @@ private object PackageContextHelper { data class ContextInfo( val packageName: String, + val legacyPackageName: String, val processName: String, val isFirstPackage: Boolean, ) fun resolve(loadedApk: Any, apkPackageName: String): ContextInfo { - var packageName = currentPkgMethod.invoke(null) as? String - var processName = currentProcMethod.invoke(null) as? String + val boundPackage = currentPkgMethod.invoke(null) as? String + val boundProcess = currentProcMethod.invoke(null) as? String val isFirstPackage = - packageName != null && processName != null && packageName == apkPackageName - - if (!isFirstPackage) { + boundPackage != null && boundProcess != null && boundPackage == apkPackageName + + val packageName: String + val processName: String + if (isFirstPackage) { + packageName = boundPackage!! + processName = boundProcess!! + } else { packageName = apkPackageName - processName = currentPkgMethod.invoke(null) as? String ?: apkPackageName - } else if (packageName == "android") { - packageName = "system" + processName = boundPackage ?: apkPackageName } - return ContextInfo(packageName, processName, isFirstPackage) + // The two APIs name this process differently, so keep both names. + // + // de.robv modules identify system server by lpparam.packageName == "android", so the + // android:ui process has always been handed "system" to keep the two apart. The modern + // API says the reverse: "system" is the virtual name for system server, while "android" + // stays a real scope target precisely because some of its components declare + // android:process=":ui". Reporting android:ui as "system" there hid the real package. + // + // In system server itself currentPackageName() is null, so isFirstPackage is false and + // both names are already "android"; only android:ui is affected. + val legacyPackageName = + if (isFirstPackage && packageName == "android") "system" else packageName + + return ContextInfo(packageName, legacyPackageName, processName, isFirstPackage) } } @@ -172,7 +189,7 @@ object LoadedApkCreateCLHooker : XposedInterface.Hooker { VectorBootstrap.withLegacy { delegate -> delegate.onPackageLoaded( LegacyPackageInfo( - ctx.packageName, + ctx.legacyPackageName, ctx.processName, classLoader, appInfo, diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/BaseInvoker.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/BaseInvoker.kt index c800fd89a..a8f8e2564 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/BaseInvoker.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/BaseInvoker.kt @@ -28,16 +28,16 @@ internal abstract class BaseInvoker, U : Executable>( /** Resolves the current [type] and executes the underlying method. */ protected fun proceedInvocation(thisObject: Any?, args: Array): Any? { return when (val currentType = type) { - is Invoker.Type.Origin -> { - try { - HookBridge.invokeOriginalMethod(executable, thisObject, *args) - } catch (e: InvocationTargetException) { - throw e.cause ?: e - } - } + // Both paths below already report a target exception wrapped and their own argument + // failures unwrapped, which is what Invoker#invoke documents. + is Invoker.Type.Origin -> dispatchOriginal(thisObject, args) is Invoker.Type.Chain -> { val snapshots = HookBridge.callbackSnapshot(VectorHookRecord::class.java, executable) + // The executable carries no hooks, so there is no chain to enter. Invokers + // default to Type.Chain.FULL, so this is the ordinary case for a module + // that obtains an invoker for a method it has not hooked. + ?: return dispatchOriginal(thisObject, args) @Suppress("UNCHECKED_CAST") val allModernHooks = snapshots[0] as Array @@ -51,20 +51,66 @@ internal abstract class BaseInvoker, U : Executable>( val delegate = VectorBootstrap.delegate if (legacyHooks.isNotEmpty() && delegate != null) { delegate.processLegacyHook(executable, tObj, tArgs, legacyHooks) { - HookBridge.invokeOriginalMethod(executable, tObj, *tArgs) + invokeOriginal(tObj, tArgs) } } else { - HookBridge.invokeOriginalMethod(executable, tObj, *tArgs) + invokeOriginal(tObj, tArgs) } } val chain = VectorChain(executable, thisObject, arrayOf(*args), filteredHooks, 0, terminal) - chain.proceed() + // Chain#proceed is documented to hand hookers the exception itself, while + // Invoker#invoke is documented against Method#invoke, which reports it wrapped, so + // the wrapping belongs at this boundary rather than inside the chain. The paths + // that skip the chain get this from the dispatch itself and are not re-wrapped. + try { + chain.proceed() + } catch (e: InvocationTargetException) { + throw e + } catch (e: Throwable) { + throw InvocationTargetException(e) + } } } } + /** Invokes the original executable, reporting a target exception as Method#invoke does. */ + private fun dispatchOriginal(thisObject: Any?, args: Array): Any? { + // invokeOriginalMethod dispatches through a cached Method.invoke id. For a hooked + // executable that is applied to lsplant's backup Method, which is correct. For an + // executable with no hook item at all it is applied to the reflected object we passed in + // — and if that is a Constructor, the id belongs to a different class. Route those + // through the non-virtual path instead, which is what invokeSpecial already uses. + if ( + executable is Constructor<*> && + HookBridge.callbackSnapshot(VectorHookRecord::class.java, executable) == null + ) { + requireNotNull(thisObject) { + "A constructor invoked as a method needs a receiver: $executable" + } + return HookBridge.invokeSpecialMethod( + executable, + getExecutableShorty(), + executable.declaringClass, + thisObject, + *args, + ) + } + return HookBridge.invokeOriginalMethod(executable, thisObject, *args) + } + + /** + * The chain terminal. Chain#proceed is documented to throw whatever the original executable + * threw, so the reflective wrapper comes off here rather than at the public boundary. + */ + private fun invokeOriginal(thisObject: Any?, args: Array): Any? = + try { + dispatchOriginal(thisObject, args) + } catch (e: InvocationTargetException) { + throw e.cause ?: e + } + /** Helper to generate the JNI shorty for non-virtual special invocations. */ protected fun getExecutableShorty(): CharArray { val parameterTypes = executable.parameterTypes diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorChain.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorChain.kt index 4a423c108..8867c52b8 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorChain.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorChain.kt @@ -4,6 +4,7 @@ import io.github.libxposed.api.XposedInterface import io.github.libxposed.api.XposedInterface.Chain import io.github.libxposed.api.XposedInterface.ExceptionMode import java.lang.reflect.Executable +import java.util.Collections import org.lsposed.lspd.util.Utils /** Represents a registered hook configuration, stored natively by [HookBridge]. */ @@ -38,7 +39,10 @@ class VectorChain( override fun getThisObject(): Any? = thisObj - override fun getArgs(): List = args.toList() + // Immutable, and a snapshot rather than a view: the chain rewrites this array in place when a + // hooker calls proceed(args) and when a legacy hook edits its arguments, which would otherwise + // change a list a hooker is still holding. + override fun getArgs(): List = Collections.unmodifiableList(args.toMutableList()) override fun getArg(index: Int): Any? = args[index] @@ -66,20 +70,31 @@ class VectorChain( return try { executeDownstream { record.hooker.intercept(nextChain) } } catch (t: Throwable) { - handleInterceptorException(t, record, nextChain, thisObject, currentArgs) + // Recording the recovery keeps this node's cached state consistent: once the hooker's + // exception has been suppressed, parent nodes must observe the recovered outcome and + // not the exception we just swallowed. + executeDownstream { + handleInterceptorException(t, record, nextChain, thisObject, currentArgs) + } } } /** * Executes the block and caches the downstream state so parent chains can recover it if the * current interceptor crashes during post-processing. + * + * Exactly one of [downstreamResult] and [downstreamThrowable] is meaningful after this returns, + * so both are always written; leaving a stale value behind would let a parent node resurrect an + * exception this node already handled. */ private inline fun executeDownstream(block: () -> Any?): Any? { return try { val result = block() downstreamResult = result + downstreamThrowable = null result } catch (t: Throwable) { + downstreamResult = null downstreamThrowable = t throw t } diff --git a/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorNativeHooker.kt b/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorNativeHooker.kt index cbfdb7506..c8b65fe78 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorNativeHooker.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/impl/hooks/VectorNativeHooker.kt @@ -6,6 +6,7 @@ import io.github.libxposed.api.XposedInterface.HookBuilder import io.github.libxposed.api.XposedInterface.HookHandle import io.github.libxposed.api.XposedInterface.Hooker import io.github.libxposed.api.error.HookFailedError +import java.lang.reflect.Constructor import java.lang.reflect.Executable import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method @@ -15,7 +16,12 @@ import org.matrix.vector.impl.di.VectorBootstrap import org.matrix.vector.nativebridge.HookBridge /** Builder for configuring and registering hooks. */ -class VectorHookBuilder(private val origin: Executable) : HookBuilder { +class VectorHookBuilder( + private val origin: Executable, + // Framework-internal hooks have no module.prop, and must stay protective: letting one of + // them propagate would take the boot path down with it. + private val defaultExceptionMode: ExceptionMode = ExceptionMode.PROTECTIVE, +) : HookBuilder { private var priority = XposedInterface.PRIORITY_DEFAULT private var exceptionMode = ExceptionMode.DEFAULT @@ -37,9 +43,22 @@ class VectorHookBuilder(private val origin: Executable) : HookBuilder { origin.name == "invoke" ) { throw IllegalArgumentException("Cannot hook Method.invoke") + } else if ( + origin is Method && + origin.declaringClass == Constructor::class.java && + origin.name == "newInstance" + ) { + // Named alongside Method.invoke by the API: the framework reflects through both, so + // hooking either recurses into the hook dispatch. + throw IllegalArgumentException("Cannot hook Constructor.newInstance") } - val record = VectorHookRecord(hooker, priority, exceptionMode) + // Resolve DEFAULT here rather than at throw time: the record is stored natively and + // reaches VectorChain with no way back to the module, and module.prop cannot change + // for the life of the process. + val resolvedMode = + if (exceptionMode == ExceptionMode.DEFAULT) defaultExceptionMode else exceptionMode + val record = VectorHookRecord(hooker, priority, resolvedMode) // Register natively. HookBridge now stores VectorHookRecord instead of HookerCallback. if ( @@ -72,8 +91,11 @@ class VectorNativeHooker(private val method: T) { val thisObject = if (isStatic) null else args[0] val actualArgs = if (isStatic) args else args.sliceArray(1 until args.size) - // Retrieve the hook snapshots - val snapshots = HookBridge.callbackSnapshot(VectorHookRecord::class.java, method) + // Retrieve the hook snapshots. Null means every hook was removed after this trampoline was + // entered, which is indistinguishable from having none. + val snapshots = + HookBridge.callbackSnapshot(VectorHookRecord::class.java, method) + ?: return invokeOriginalSafely(thisObject, actualArgs) @Suppress("UNCHECKED_CAST") val modernHooks = snapshots[0] as Array val legacyHooks = snapshots[1] diff --git a/xposed/src/main/kotlin/org/matrix/vector/nativebridge/HookBridge.kt b/xposed/src/main/kotlin/org/matrix/vector/nativebridge/HookBridge.kt index ea2a5bfd6..d469eb4a5 100644 --- a/xposed/src/main/kotlin/org/matrix/vector/nativebridge/HookBridge.kt +++ b/xposed/src/main/kotlin/org/matrix/vector/nativebridge/HookBridge.kt @@ -54,8 +54,27 @@ object HookBridge { @JvmStatic @FastNative external fun setTrusted(cookie: Any?): Boolean + /** Returns null when [method] carries no hooks at all. */ @JvmStatic - external fun callbackSnapshot(hooker_callback: Class<*>, method: Executable): Array> + external fun callbackSnapshot( + hooker_callback: Class<*>, + method: Executable, + ): Array>? - @JvmStatic external fun getStaticInitializer(clazz: Class<*>): Method? + /** + * Locates a class's static initializer without initializing it. + * [artMethods] must be the ArtMethod addresses of the class's declared constructors and + * methods, which reflection can supply without triggering initialization, and [artMethodSize] + * the size of one ArtMethod. One member is enough, which matters because a class whose only + * members are the static initializer and an implicit constructor shows just one to reflection. + * + * Returns null when the class has no static initializer or the method layout is not the one + * this relies on. + */ + @JvmStatic + external fun findStaticInitializer( + clazz: Class<*>, + artMethods: LongArray, + artMethodSize: Long, + ): Executable? }