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
2026-08-03 02:51:21 -04:00 · 2026-07-03 13:44:04 +01:00
parent 10b3af22cc
commit c911cba670
18 changed files with 134 additions and 74 deletions
--- a/lib/app_sources/html.dart
+++ b/lib/app_sources/html.dart
@@ -95,7 +95,7 @@ bool _isDigit(String s) {

 List<MapEntry<String, String>> 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(
--- a/lib/components/app_list_tile.dart
+++ b/lib/components/app_list_tile.dart
@@ -750,12 +750,12 @@ class AppsFilter {
  }

  void setFormValuesFromMap(Map<String, dynamic> 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) =>
--- 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<FormFieldDefinition> definitions,
-    required Map<String, GeneratedFormFieldState> states,
-    required this.onValueChanges,
-    this.tileMode = false,
-    this.fieldDefinitions,
-    this.fieldStates,
-  }) : items = [definitions.map((d) => d.toGeneratedFormItem()).toList()];
-
  final List<List<GeneratedFormItem>> items;
-  final List<FormFieldDefinition>? fieldDefinitions;
-  final Map<String, GeneratedFormFieldState>? fieldStates;
  final OnValueChanges onValueChanges;

  final bool tileMode;
--- 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
--- a/lib/installers/external_installer.dart
+++ b/lib/installers/external_installer.dart
@@ -52,7 +52,7 @@ class ExternalInstaller extends Installer {
  Future<InstallResult> installApk(
    List<String> apkFilePaths, {
    required String appId,
-    bool shizukuPretendToBeGooglePlay = false,
+    Map<String, dynamic> installOptions = const {},
  }) async {
    final targetPackage = settingsProvider.externalInstallerPackage;
    if (targetPackage == null || apkFilePaths.isEmpty) {
--- a/lib/installers/installer.dart
+++ b/lib/installers/installer.dart
@@ -84,9 +84,11 @@ abstract class Installer {
  Future<void> 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<InstallResult> installApk(
    List<String> apkFilePaths, {
    required String appId,
-    bool shizukuPretendToBeGooglePlay = false,
+    Map<String, dynamic> installOptions = const {},
  });
 }
--- a/lib/installers/shizuku_installer.dart
+++ b/lib/installers/shizuku_installer.dart
@@ -40,9 +40,9 @@ class ShizukuInstaller extends Installer {
  Future<InstallResult> installApk(
    List<String> apkFilePaths, {
    required String appId,
-    bool shizukuPretendToBeGooglePlay = false,
+    Map<String, dynamic> installOptions = const {},
  }) async {
-    final fakeInstallSource = shizukuPretendToBeGooglePlay
+    final fakeInstallSource = installOptions['shizukuPretendToBeGooglePlay'] == true
        ? 'com.android.vending'
        : '';
    final uris = apkFilePaths.map((p) => File(p).uri.toString()).toList();
--- a/lib/installers/stock_installer.dart
+++ b/lib/installers/stock_installer.dart
@@ -71,7 +71,7 @@ class StockInstaller extends Installer {
  Future<InstallResult> installApk(
    List<String> apkFilePaths, {
    required String appId,
-    bool shizukuPretendToBeGooglePlay = false,
+    Map<String, dynamic> installOptions = const {},
  }) async {
    final code = await AndroidPackageInstaller.installApk(
      apkFilePath: apkFilePaths.join(','),
--- 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) {
--- a/lib/pages/add_app.dart
+++ b/lib/pages/add_app.dart
@@ -681,7 +681,7 @@ class AddAppPageState extends State<AddAppPage> {
      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) {
--- a/lib/pages/home.dart
+++ b/lib/pages/home.dart
@@ -375,7 +375,7 @@ class _HomePageState extends State<HomePage> {

    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),
--- a/lib/pages/settings.dart
+++ b/lib/pages/settings.dart
@@ -31,6 +31,7 @@ class SettingsPage extends StatefulWidget {

 class _SettingsPageState extends State<SettingsPage> {
  int? androidSdkInt;
+  int _installerCheckSeq = 0;
  late final SourceProvider sourceProvider;

  @override
@@ -144,12 +145,17 @@ class _SettingsPageState extends State<SettingsPage> {
  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<SettingsPage> {
            }
          })
          .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<SettingsPage> {
                  onSelectionChanged: (selection) => handleInstallerModeChange(
                    settingsProvider,
                    selection.first,
+                    _installerCheckSeq,
                  ),
                ),
              ),
--- a/lib/providers/apps_provider.dart
+++ b/lib/providers/apps_provider.dart
@@ -378,7 +378,12 @@ Future<File?> _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<File> 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<FGBGType>? foregroundStream;
  StreamSubscription<FGBGType>? 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<AppInMemory> 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) {
--- 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<List<App>, bool>(importedApps, hasSettings);
  }

+  static const _importableKeys = {
+    'theme',
+    'colourSchemeMode',
+    'updateInterval',
+    'useFGService',
+    'parallelDownloads',
+    'includePrereleasesByDefault',
+    'tryInferAppID',
+    'tryInferAppIDFromLink',
+    'removeOnExternalUninstall',
+    'exportAppSettings',
+    'showAppDowngradeError',
+    'installerMode',
+    'interceptBundleDownloads',
+    'fallbackToStaticCache',
+    'shizukuPretendToBeGooglePlay',
+  };
+
  void _applyImportedSettings(Map<String, dynamic> settingsMap) {
    settingsMap.forEach((key, value) {
+      if (!_importableKeys.contains(key)) return;
      if (value is int) {
        settingsProvider.prefs?.setInt(key, value);
      } else if (value is double) {
--- a/lib/providers/apps_provider_install.dart
+++ b/lib/providers/apps_provider_install.dart
@@ -51,6 +51,7 @@ const List<String> _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<int> decompressed;

@@ -459,7 +464,7 @@ extension AppsProviderInstall on AppsProvider {
    DownloadedDir dir,
    BuildContext? firstTimeWithContext, {
    bool needsBGWorkaround = false,
-    bool shizukuPretendToBeGooglePlay = false,
+    Map<String, dynamic> 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<String, dynamic> installOptions = const {},
    List<DownloadedApk> 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.
--- 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);
          }
        }
--- 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,
--- 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;