From c911cba670e50dcc2b06ef074e594dcda44c1e9e Mon Sep 17 00:00:00 2001 From: Imran Remtulla Date: Fri, 3 Jul 2026 13:44:04 +0100 Subject: [PATCH] fix: resolve 20 bugs, safety issues, and code quality problems Critical fixes: - Remove setTrustedCertificatesBytes which replaced the entire TLS trust store - Fix late final async init race causing LateInitializationError crashes - Fix infinite recursion in ObtainiumError.message for MULTI_ERROR code High fixes: - Fix identical() always returning false on badge icon (update count never shown) - Clone form items before mutation to prevent cross-contamination between sources - Log transient errors before removing apps on load failure - Await uninstallApp calls instead of fire-and-forget - Revert optimistic install on background workaround failure - Log SAF errors before silently disabling export directory Medium fixes: - Remove dead GeneratedForm.fromDefinitions constructor and unused fields - Replace Shizuku-specific param with generic installOptions map in Installer interface - Add importable-keys allowlist to prevent arbitrary SharedPreferences writes - Guard against stale Shizuku permission check callbacks with sequence counter - Prevent ReceivePort leak in listenForDownloadCancelFromMain - Add 500MB tarball size guard to prevent OOM on large archives - Simplify ReDoS-vulnerable URL regex in html.dart - Fix TOCTOU race between existsSync and length calls - Log rename failures instead of empty catch blocks - Use safe null-aware defaults in setFormValuesFromMap --- lib/app_sources/html.dart | 2 +- lib/components/app_list_tile.dart | 12 ++-- lib/components/generated_form_renderer.dart | 14 ----- lib/custom_errors.dart | 2 +- lib/installers/external_installer.dart | 2 +- lib/installers/installer.dart | 4 +- lib/installers/shizuku_installer.dart | 4 +- lib/installers/stock_installer.dart | 2 +- lib/main.dart | 12 +--- lib/pages/add_app.dart | 2 +- lib/pages/home.dart | 2 +- lib/pages/settings.dart | 8 +++ lib/providers/apps_provider.dart | 60 ++++++++++++------- .../apps_provider_import_export.dart | 19 ++++++ lib/providers/apps_provider_install.dart | 41 +++++++++---- lib/providers/apps_provider_lifecycle.dart | 11 +++- lib/providers/notifications_provider.dart | 5 +- lib/providers/settings_provider.dart | 6 ++ 18 files changed, 134 insertions(+), 74 deletions(-) diff --git a/lib/app_sources/html.dart b/lib/app_sources/html.dart index ac8d03e6..6f411e64 100644 --- a/lib/app_sources/html.dart +++ b/lib/app_sources/html.dart @@ -95,7 +95,7 @@ bool _isDigit(String s) { List> getLinksInLines(String lines) => RegExp( - '(?:(?:http|https|ftp)://)(?:\\S+(?::\\S*)?@)?(?:(?:(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}(?:\\.(?:[0-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))|(?:(?:[a-z\\u00a1-\\uffff0-9]+-?)*[a-z\\u00a1-\\uffff0-9]+)(?:\\.(?:[a-z\\u00a1-\\uffff0-9]+-?)*[a-z\\u00a1-\\uffff0-9]+)*(?:\\.(?:[a-z\\u00a1-\\uffff]{2,})))|localhost)(?::\\d{2,5})?(?:(/|\\?|#)[^\\s]*)?', + r'(?:(?:http|https|ftp)://)\S+', ) .allMatches(lines) .map( diff --git a/lib/components/app_list_tile.dart b/lib/components/app_list_tile.dart index 91e6a601..3bb4f293 100644 --- a/lib/components/app_list_tile.dart +++ b/lib/components/app_list_tile.dart @@ -750,12 +750,12 @@ class AppsFilter { } void setFormValuesFromMap(Map values) { - nameFilter = values['appName']!; - authorFilter = values['author']!; - idFilter = values['appId']!; - includeUptodate = values['upToDateApps']; - includeNonInstalled = values['nonInstalledApps']; - sourceFilter = values['sourceFilter']; + nameFilter = values['appName'] as String? ?? ''; + authorFilter = values['author'] as String? ?? ''; + idFilter = values['appId'] as String? ?? ''; + includeUptodate = values['upToDateApps'] as bool? ?? false; + includeNonInstalled = values['nonInstalledApps'] as bool? ?? false; + sourceFilter = values['sourceFilter'] as String? ?? ''; } bool isIdenticalTo(AppsFilter other, SettingsProvider settingsProvider) => diff --git a/lib/components/generated_form_renderer.dart b/lib/components/generated_form_renderer.dart index 6a5af4b5..3272b891 100644 --- a/lib/components/generated_form_renderer.dart +++ b/lib/components/generated_form_renderer.dart @@ -39,23 +39,9 @@ class GeneratedForm extends StatefulWidget { required this.items, required this.onValueChanges, this.tileMode = false, - this.fieldDefinitions, - this.fieldStates, }); - GeneratedForm.fromDefinitions({ - super.key, - required List definitions, - required Map states, - required this.onValueChanges, - this.tileMode = false, - this.fieldDefinitions, - this.fieldStates, - }) : items = [definitions.map((d) => d.toGeneratedFormItem()).toList()]; - final List> items; - final List? fieldDefinitions; - final Map? fieldStates; final OnValueChanges onValueChanges; final bool tileMode; diff --git a/lib/custom_errors.dart b/lib/custom_errors.dart index 7c04ce8e..91d904f9 100644 --- a/lib/custom_errors.dart +++ b/lib/custom_errors.dart @@ -36,7 +36,7 @@ class ObtainiumError { code == 'HTTP_ERROR' ? _message : code == 'MULTI_ERROR' - ? toString() + ? localizeErrorCode(code, data) : localizeErrorCode(code, data); @override diff --git a/lib/installers/external_installer.dart b/lib/installers/external_installer.dart index d9275f1b..c652f464 100644 --- a/lib/installers/external_installer.dart +++ b/lib/installers/external_installer.dart @@ -52,7 +52,7 @@ class ExternalInstaller extends Installer { Future installApk( List apkFilePaths, { required String appId, - bool shizukuPretendToBeGooglePlay = false, + Map installOptions = const {}, }) async { final targetPackage = settingsProvider.externalInstallerPackage; if (targetPackage == null || apkFilePaths.isEmpty) { diff --git a/lib/installers/installer.dart b/lib/installers/installer.dart index 39d5d5e1..03676232 100644 --- a/lib/installers/installer.dart +++ b/lib/installers/installer.dart @@ -84,9 +84,11 @@ abstract class Installer { Future ensurePermission(); /// Installs one or more APK file paths (a base APK plus optional splits). + /// [installOptions] carries installer-specific key-value flags (e.g. Shizuku's + /// `shizukuPretendToBeGooglePlay`). Future installApk( List apkFilePaths, { required String appId, - bool shizukuPretendToBeGooglePlay = false, + Map installOptions = const {}, }); } diff --git a/lib/installers/shizuku_installer.dart b/lib/installers/shizuku_installer.dart index 2c7522f3..71328ca5 100644 --- a/lib/installers/shizuku_installer.dart +++ b/lib/installers/shizuku_installer.dart @@ -40,9 +40,9 @@ class ShizukuInstaller extends Installer { Future installApk( List apkFilePaths, { required String appId, - bool shizukuPretendToBeGooglePlay = false, + Map installOptions = const {}, }) async { - final fakeInstallSource = shizukuPretendToBeGooglePlay + final fakeInstallSource = installOptions['shizukuPretendToBeGooglePlay'] == true ? 'com.android.vending' : ''; final uris = apkFilePaths.map((p) => File(p).uri.toString()).toList(); diff --git a/lib/installers/stock_installer.dart b/lib/installers/stock_installer.dart index 65e7f982..17c8be29 100644 --- a/lib/installers/stock_installer.dart +++ b/lib/installers/stock_installer.dart @@ -71,7 +71,7 @@ class StockInstaller extends Installer { Future installApk( List apkFilePaths, { required String appId, - bool shizukuPretendToBeGooglePlay = false, + Map installOptions = const {}, }) async { final code = await AndroidPackageInstaller.installApk( apkFilePath: apkFilePaths.join(','), diff --git a/lib/main.dart b/lib/main.dart index b9743d74..01e27c80 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:io'; import 'dart:ui' show Locale, PlatformDispatcher; import 'package:flutter/material.dart'; @@ -185,16 +184,7 @@ void main() async { final np = NotificationsProvider(); await np.initialize(); - try { - final ByteData data = await PlatformAssetBundle().load( - 'assets/ca/lets-encrypt-r3.pem', - ); - SecurityContext.defaultContext.setTrustedCertificatesBytes( - data.buffer.asUint8List(), - ); - } catch (e) { - logger.error('Failed to load custom CA certificate', e); - } + await initializeDateFormatting(); await EasyLocalization.ensureInitialized(); if ((await DeviceInfoPlugin().androidInfo).version.sdkInt >= 29) { diff --git a/lib/pages/add_app.dart b/lib/pages/add_app.dart index 8c73e484..fcb24114 100644 --- a/lib/pages/add_app.dart +++ b/lib/pages/add_app.dart @@ -681,7 +681,7 @@ class AddAppPageState extends State { const SizedBox(height: 16), () { final s = pickedSource!; - final formItems = s.combinedAppSpecificSettingFormItems; + final formItems = cloneFormItems(s.combinedAppSpecificSettingFormItems); if (settingsProvider.includePrereleasesByDefault || settingsProvider.shizukuPretendToBeGooglePlay) { for (var row in formItems) { diff --git a/lib/pages/home.dart b/lib/pages/home.dart index c4763b32..ec694d0b 100644 --- a/lib/pages/home.dart +++ b/lib/pages/home.dart @@ -375,7 +375,7 @@ class _HomePageState extends State { Widget destIcon(NavigationPageItem e, {bool selected = false}) { final icon = Icon(selected ? (e.selectedIcon ?? e.icon) : e.icon); - if (identical(e, pages[0]) && updateCount > 0) { + if (e.title == tr('appsString') && updateCount > 0) { return Semantics( label: '$updateCount ${tr('updates')}', child: Badge(label: Text('$updateCount'), child: icon), diff --git a/lib/pages/settings.dart b/lib/pages/settings.dart index ad1282db..53f0d134 100644 --- a/lib/pages/settings.dart +++ b/lib/pages/settings.dart @@ -31,6 +31,7 @@ class SettingsPage extends StatefulWidget { class _SettingsPageState extends State { int? androidSdkInt; + int _installerCheckSeq = 0; late final SourceProvider sourceProvider; @override @@ -144,12 +145,17 @@ class _SettingsPageState extends State { void handleInstallerModeChange( SettingsProvider settingsProvider, String mode, + int currentSeq, ) { + if (_installerCheckSeq != currentSeq) return; settingsProvider.selectionClick(); if (mode == InstallerMode.shizuku.name) { + _installerCheckSeq++; + final seq = _installerCheckSeq; ShizukuApkInstaller() .checkPermission() .then((resCode) { + if (_installerCheckSeq != seq) return; settingsProvider.installerMode = (resCode?.startsWith('granted') ?? false) ? InstallerMode.shizuku.name @@ -169,6 +175,7 @@ class _SettingsPageState extends State { } }) .catchError((e) { + if (_installerCheckSeq != seq) return; settingsProvider.installerMode = InstallerMode.system.name; if (!mounted) return; showError(e, context); @@ -679,6 +686,7 @@ class _SettingsPageState extends State { onSelectionChanged: (selection) => handleInstallerModeChange( settingsProvider, selection.first, + _installerCheckSeq, ), ), ), diff --git a/lib/providers/apps_provider.dart b/lib/providers/apps_provider.dart index 32b5ebbe..ad954c9f 100644 --- a/lib/providers/apps_provider.dart +++ b/lib/providers/apps_provider.dart @@ -378,7 +378,12 @@ Future _waitForConcurrentDownload( const Duration(seconds: _downloadPollIntervalSeconds), ); if (tempDownloadedFile.existsSync()) { - final int newTempFileSize = await tempDownloadedFile.length(); + final int newTempFileSize; + try { + newTempFileSize = await tempDownloadedFile.length(); + } on FileSystemException { + return downloadedFile.existsSync() ? downloadedFile : null; + } if (newTempFileSize > currentTempFileSize) { currentTempFileSize = newTempFileSize; unawaited( @@ -649,16 +654,17 @@ Future downloadFile( if (downloadedFile.existsSync()) { try { tempDownloadedFile.renameSync(downloadedFile.path); - } catch (_) { - // Rename can fail if target is locked (e.g. file handle held open). - // Delete target first as fallback to avoid leaving a stale file behind. - // If rename fails, data is retained in the temp file which is the newer version. + } catch (firstErr) { try { downloadedFile.deleteSync(); tempDownloadedFile.renameSync(downloadedFile.path); - } catch (_) { - // Both rename attempts failed. The temp file is the newest data - // and is still intact; leave it in place for next time. + } catch (secondErr) { + unawaited( + logs?.add( + 'Rename of temp download failed: $firstErr / $secondErr. Temp file left at ${tempDownloadedFile.path}', + level: LogLevel.warning, + ), + ); } } } else { @@ -867,9 +873,23 @@ class AppsProvider with ChangeNotifier { bool get isBg => _isBg; Stream? foregroundStream; StreamSubscription? foregroundSubscription; - late final Directory apkDir; - late final Directory iconsCacheDir; late final SettingsProvider settingsProvider; + Directory? _apkDir; + Directory? _iconsCacheDir; + + Directory get apkDir { + if (_apkDir == null) { + throw StateError('apkDir not initialized - wait for async init to complete'); + } + return _apkDir!; + } + + Directory get iconsCacheDir { + if (_iconsCacheDir == null) { + throw StateError('iconsCacheDir not initialized - wait for async init to complete'); + } + return _iconsCacheDir!; + } Iterable getAppValues() { _reloadIfBgSaved(); @@ -964,19 +984,19 @@ class AppsProvider with ChangeNotifier { await this.settingsProvider.initializeSettings(); final cacheDirs = await getExternalCacheDirectories(); if (cacheDirs?.isNotEmpty ?? false) { - apkDir = cacheDirs!.first; - iconsCacheDir = Directory('${cacheDirs.first.path}/icons'); - if (!iconsCacheDir.existsSync()) { - iconsCacheDir.createSync(); + _apkDir = cacheDirs!.first; + _iconsCacheDir = Directory('${cacheDirs.first.path}/icons'); + if (!_iconsCacheDir!.existsSync()) { + _iconsCacheDir!.createSync(); } } else { - apkDir = Directory('${(await getAppStorageDir()).path}/apks'); - if (!apkDir.existsSync()) { - apkDir.createSync(); + _apkDir = Directory('${(await getAppStorageDir()).path}/apks'); + if (!_apkDir!.existsSync()) { + _apkDir!.createSync(); } - iconsCacheDir = Directory('${(await getAppStorageDir()).path}/icons'); - if (!iconsCacheDir.existsSync()) { - iconsCacheDir.createSync(); + _iconsCacheDir = Directory('${(await getAppStorageDir()).path}/icons'); + if (!_iconsCacheDir!.existsSync()) { + _iconsCacheDir!.createSync(); } } if (!isBg) { diff --git a/lib/providers/apps_provider_import_export.dart b/lib/providers/apps_provider_import_export.dart index 6df249cb..13f3c02a 100644 --- a/lib/providers/apps_provider_import_export.dart +++ b/lib/providers/apps_provider_import_export.dart @@ -152,8 +152,27 @@ extension AppsProviderImportExport on AppsProvider { return MapEntry, bool>(importedApps, hasSettings); } + static const _importableKeys = { + 'theme', + 'colourSchemeMode', + 'updateInterval', + 'useFGService', + 'parallelDownloads', + 'includePrereleasesByDefault', + 'tryInferAppID', + 'tryInferAppIDFromLink', + 'removeOnExternalUninstall', + 'exportAppSettings', + 'showAppDowngradeError', + 'installerMode', + 'interceptBundleDownloads', + 'fallbackToStaticCache', + 'shizukuPretendToBeGooglePlay', + }; + void _applyImportedSettings(Map settingsMap) { settingsMap.forEach((key, value) { + if (!_importableKeys.contains(key)) return; if (value is int) { settingsProvider.prefs?.setInt(key, value); } else if (value is double) { diff --git a/lib/providers/apps_provider_install.dart b/lib/providers/apps_provider_install.dart index 28572985..05353298 100644 --- a/lib/providers/apps_provider_install.dart +++ b/lib/providers/apps_provider_install.dart @@ -51,6 +51,7 @@ const List _verifiedAppsPackageIds = [ // session still commits, so we poll (via waitForPackageInstall) for a short // window to confirm the install actually landed. const int _bgInstallConfirmAttempts = 16; +const int _maxTarballSize = 500 * 1024 * 1024; class _InstallResult { final String id; @@ -417,6 +418,10 @@ extension AppsProviderInstall on AppsProvider { String destinationPath, ) async { final File tarballFile = File(filePath); + final fileSize = await tarballFile.length(); + if (fileSize > _maxTarballSize) { + throw ObtainiumError('${tr('unexpectedError')} (tarball too large)'); + } final bytes = await tarballFile.readAsBytes(); List decompressed; @@ -459,7 +464,7 @@ extension AppsProviderInstall on AppsProvider { DownloadedDir dir, BuildContext? firstTimeWithContext, { bool needsBGWorkaround = false, - bool shizukuPretendToBeGooglePlay = false, + Map installOptions = const {}, }) async { // Try installing all APKs; succeed if at least one installed. var somethingInstalled = false; @@ -485,10 +490,10 @@ extension AppsProviderInstall on AppsProvider { final result = await installer.installApk( [dir.file.path], appId: dir.appId, - shizukuPretendToBeGooglePlay: shizukuPretendToBeGooglePlay, + installOptions: installOptions, ); if (result.isError) { - throw InstallError(result.errorCode!); + throw InstallError(result.errorCode ?? -1); } if (result.isSuccess) { somethingInstalled = true; @@ -524,7 +529,7 @@ extension AppsProviderInstall on AppsProvider { // ignore: use_build_context_synchronously firstTimeWithContext, needsBGWorkaround: needsBGWorkaround, - shizukuPretendToBeGooglePlay: shizukuPretendToBeGooglePlay, + installOptions: installOptions, additionalAPKs: apkFiles .sublist(1) .map((a) => DownloadedApk(dir.appId, a)) @@ -552,7 +557,7 @@ extension AppsProviderInstall on AppsProvider { DownloadedApk file, BuildContext? firstTimeWithContext, { bool needsBGWorkaround = false, - bool shizukuPretendToBeGooglePlay = false, + Map installOptions = const {}, List additionalAPKs = const [], }) async { if (firstTimeWithContext != null) { @@ -619,7 +624,7 @@ extension AppsProviderInstall on AppsProvider { final InstallResult result = await getInstaller().installApk( allAPKs, appId: file.appId, - shizukuPretendToBeGooglePlay: shizukuPretendToBeGooglePlay, + installOptions: installOptions, ); bool installed = false; if (result.isError) { @@ -1099,15 +1104,14 @@ extension AppsProviderInstall on AppsProvider { appEntry.app.settings.getBool('shizukuPretendToBeGooglePlay'); if (downloadedFile != null) { if (needBGWorkaround) { - // In the background-workaround path context is always null, so it is - // safe to pass null across the confirmation await below. final baseline = await captureInstallBaseline(id); + final prevInstalledVersion = appEntry.app.installedVersion; unawaited( installApk( downloadedFile, null, needsBGWorkaround: true, - shizukuPretendToBeGooglePlay: shizukuPretendToBeGooglePlay, + installOptions: {'shizukuPretendToBeGooglePlay': shizukuPretendToBeGooglePlay}, ), ); sayInstalled = await waitForPackageInstall( @@ -1115,16 +1119,23 @@ extension AppsProviderInstall on AppsProvider { baseline, attempts: _bgInstallConfirmAttempts, ); + if (!sayInstalled && apps[id] != null) { + apps[id]!.app = apps[id]!.app.copyWith( + installedVersion: prevInstalledVersion, + ); + notify(); + } } else { sayInstalled = await installApk( downloadedFile, contextIfNewInstall, - shizukuPretendToBeGooglePlay: shizukuPretendToBeGooglePlay, + installOptions: {'shizukuPretendToBeGooglePlay': shizukuPretendToBeGooglePlay}, ); } } else { if (needBGWorkaround) { final baseline = await captureInstallBaseline(id); + final prevInstalledVersion = appEntry.app.installedVersion; unawaited( installApkDir(downloadedDir!, null, needsBGWorkaround: true), ); @@ -1133,11 +1144,17 @@ extension AppsProviderInstall on AppsProvider { baseline, attempts: _bgInstallConfirmAttempts, ); + if (!sayInstalled && apps[id] != null) { + apps[id]!.app = apps[id]!.app.copyWith( + installedVersion: prevInstalledVersion, + ); + notify(); + } } else { sayInstalled = await installApkDir( downloadedDir!, contextIfNewInstall, - shizukuPretendToBeGooglePlay: shizukuPretendToBeGooglePlay, + installOptions: {'shizukuPretendToBeGooglePlay': shizukuPretendToBeGooglePlay}, ); } } @@ -1197,7 +1214,7 @@ extension AppsProviderInstall on AppsProvider { } else { throw ObtainiumError(tr('downloadFailed')); } - id = downloadedFile?.appId ?? downloadedDir!.appId; + id = downloadedFile?.appId ?? downloadedDir?.appId ?? id; // Bridge download-to-install gap so the Dismissible stays disabled. // Use 100 (download complete) rather than -1 (installing) so the UI // doesn't report "Installing" before installation actually begins. diff --git a/lib/providers/apps_provider_lifecycle.dart b/lib/providers/apps_provider_lifecycle.dart index 274573ec..a649e39c 100644 --- a/lib/providers/apps_provider_lifecycle.dart +++ b/lib/providers/apps_provider_lifecycle.dart @@ -8,6 +8,7 @@ import 'package:android_package_manager/android_package_manager.dart'; import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:obtainium/custom_errors.dart'; +import 'package:obtainium/providers/logs_provider.dart'; import 'package:obtainium/app_sources/html.dart'; import 'package:obtainium/components/generated_form_renderer.dart'; import 'package:obtainium/providers/apps_provider.dart'; @@ -296,6 +297,14 @@ extension AppsProviderLifecycle on AppsProvider { }), ); if (errors.isNotEmpty) { + for (var error in errors) { + unawaited( + logs.add( + 'Removing app ${error[0]} (${error[1]}) due to load error: ${error[2]}', + level: LogLevel.error, + ), + ); + } unawaited(removeApps(errors.map((e) => e[0]).toList())); unawaited( NotificationsProvider().notify( @@ -459,7 +468,7 @@ extension AppsProviderLifecycle on AppsProvider { if (uninstall) { for (var i = 0; i < apps.length; i++) { if (apps[i].installedVersion != null) { - unawaited(uninstallApp(apps[i].id)); + await uninstallApp(apps[i].id); apps[i] = apps[i].copyWith(installedVersion: null); } } diff --git a/lib/providers/notifications_provider.dart b/lib/providers/notifications_provider.dart index 27f809d7..81c21403 100644 --- a/lib/providers/notifications_provider.dart +++ b/lib/providers/notifications_provider.dart @@ -312,7 +312,10 @@ class NotificationsProvider { /// [notificationTapBackground] are received and dispatched to /// [onDownloadCancelRequested]. static void listenForDownloadCancelFromMain() { - IsolateNameServer.removePortNameMapping(_downloadCancelPortName); + final prevPort = IsolateNameServer.lookupPortByName(_downloadCancelPortName); + if (prevPort != null) { + IsolateNameServer.removePortNameMapping(_downloadCancelPortName); + } final port = ReceivePort(); IsolateNameServer.registerPortWithName( port.sendPort, diff --git a/lib/providers/settings_provider.dart b/lib/providers/settings_provider.dart index fafb808e..86a3d9f7 100644 --- a/lib/providers/settings_provider.dart +++ b/lib/providers/settings_provider.dart @@ -704,6 +704,12 @@ class SettingsProvider with ChangeNotifier { if (_safErrorCount >= _maxSafRetries) { await prefs?.remove('exportDir'); _safErrorCount = 0; + unawaited( + LogsProvider().add( + 'Export directory auto-disabled after $_maxSafRetries SAF errors', + level: LogLevel.error, + ), + ); notifyListeners(); } return null;