Merge pull request #2557 from SatoshiPortal/audit-security-deps

refactor: Security audit fixes (certs, webview, RNG) + drop 5 dependencies

Author: ethicnology

assets/icons/github.svg

@@ -1,4 +1,5 @@
 import 'package:bb_mobile/core/electrum/domain/repositories/electrum_transaction_repository.dart';
+import 'package:bb_mobile/core/electrum/domain/value_objects/electrum_connection.dart';
 import 'package:bb_mobile/core/electrum/frameworks/drift/datasources/electrum_remote_datasource.dart';
 import 'package:bb_mobile/core/storage/tables/transactions_table.dart';
 import 'package:bb_mobile/core/utils/bitcoin_tx.dart';
@@ -11,10 +12,10 @@ class DriftElectrumTransactionRepository
 
   @override
   Future<BitcoinTx> fetch({
-    required String serverUrl,
+    required ElectrumConnection connection,
     required String txid,
   }) async {
-    final model = await _datasource.fetch(serverUrl: serverUrl, txid: txid);
+    final model = await _datasource.fetch(connection: connection, txid: txid);
     return TransactionModelExtension.toEntity(model);
   }
 }

lib/core/electrum/adapters/drift_electrum_transaction_repository.dart

@@ -37,9 +37,9 @@ class ElectrumTransactionPortAdapter implements TransactionPort {
     try {
       return await _serversPort.runWithFallback(
         network: network,
-        operation: (server) async {
+        operation: (connection) async {
           final bitcoinTx = await _repository.fetch(
-            serverUrl: server.url,
+            connection: connection,
             txid: txid,
           );
           return TransactionMapper.fromBitcoinTx(

lib/core/electrum/adapters/electrum_transaction_port_adapter.dart

@@ -58,6 +58,7 @@ class ServerStatusAdapter implements ServerStatusPort {
   Future<ElectrumServerStatus> checkElectrum({
     required String url,
     required ElectrumServerNetwork network,
+    required bool validateDomain,
     int? timeout,
   }) async {
     try {
@@ -79,6 +80,7 @@ class ServerStatusAdapter implements ServerStatusPort {
         uri: uri,
         request: request,
         timeoutSeconds: effectiveTimeout,
+        validateDomain: validateDomain,
       );
 
       if (response.isEmpty) return ElectrumServerStatus.offline;
@@ -135,13 +137,17 @@ class ServerStatusAdapter implements ServerStatusPort {
     required Uri uri,
     required String request,
     required int timeoutSeconds,
+    required bool validateDomain,
   }) async {
+    // A null onBadCertificate callback enforces strict CA validation. The
+    // flag comes from the user's electrum settings, so the probe accepts
+    // exactly the certificates the BDK/LWK sync would accept.
     final Socket socket = uri.scheme == 'ssl'
         ? await SecureSocket.connect(
             uri.host,
             uri.port,
             timeout: Duration(seconds: timeoutSeconds),
-            onBadCertificate: (_) => true, // accept self-signed certs
+            onBadCertificate: validateDomain ? null : (_) => true,
           )
         : await Socket.connect(
             uri.host,

lib/core/electrum/adapters/server_status_adapter.dart

@@ -1,8 +1,10 @@
 import 'package:bb_mobile/core/electrum/application/dtos/requests/add_custom_server_request.dart';
 import 'package:bb_mobile/core/electrum/domain/entities/electrum_server.dart';
+import 'package:bb_mobile/core/electrum/domain/entities/electrum_settings.dart';
 import 'package:bb_mobile/core/electrum/domain/errors/electrum_failure.dart';
 import 'package:bb_mobile/core/electrum/domain/ports/server_status_port.dart';
 import 'package:bb_mobile/core/electrum/domain/repositories/electrum_server_repository.dart';
+import 'package:bb_mobile/core/electrum/domain/repositories/electrum_settings_repository.dart';
 import 'package:bb_mobile/core/electrum/domain/value_objects/electrum_server_network.dart';
 import 'package:bb_mobile/core/electrum/domain/value_objects/electrum_server_status.dart';
 import 'package:bb_mobile/core/settings/domain/repositories/settings_repository.dart';
@@ -12,11 +14,13 @@ import 'package:meta/meta.dart';
 
 class AddCustomServerUsecase {
   final ElectrumServerRepository _electrumServerRepository;
+  final ElectrumSettingsRepository _electrumSettingsRepository;
   final ServerStatusPort _serverStatusPort;
   final SettingsRepository _settingsRepository;
 
   AddCustomServerUsecase({
     required this._electrumServerRepository,
+    required this._electrumSettingsRepository,
     required this._serverStatusPort,
     required this._settingsRepository,
   });
@@ -61,9 +65,23 @@ class AddCustomServerUsecase {
 
       // Step 2: verify the server actually serves chain data by fetching a
       // known historical tx (falls back to server.version on testnets).
+      // Probe with the user's own validateDomain setting: accepting a
+      // certificate the sync would refuse saves a server that can never be
+      // used, and the failure only surfaces later as a broken sync.
+      final ElectrumSettings electrumSettings;
+      switch (await _electrumSettingsRepository.fetchByNetwork(
+        server.network,
+      )) {
+        case Ok(:final value):
+          electrumSettings = value;
+        case Err(:final failure):
+          return Err(failure);
+      }
+
       final protocolStatus = await _serverStatusPort.checkElectrum(
         url: server.url,
         network: server.network,
+        validateDomain: electrumSettings.validateDomain,
       );
       if (protocolStatus == ElectrumServerStatus.offline) {
         return const Err(ElectrumServerUnreachableFailure());

lib/core/electrum/application/usecases/add_custom_server_usecase.dart

@@ -14,9 +14,20 @@ abstract class ServerStatusPort {
   /// only responds to `server.version` can still be desynced, pruned, or
   /// otherwise broken — fetching a real tx proves it can answer wallet
   /// queries. Falls back to `server.version` on testnets (no stable txid).
+  ///
+  /// [validateDomain] must mirror the user's electrum setting of the same
+  /// name — the flag the BDK/LWK sync obeys. When true, the certificate
+  /// chain, expiry and hostname must all check out; when false, every check
+  /// is skipped, so an active MITM becomes indistinguishable from the user's
+  /// own node and the user vouches for the endpoint.
+  ///
+  /// Probing with anything else makes this check lie: a laxer probe reports
+  /// a server online that the sync will then refuse, and a stricter one hides
+  /// a server that would have worked.
   Future<ElectrumServerStatus> checkElectrum({
     required String url,
     required ElectrumServerNetwork network,
+    required bool validateDomain,
     int? timeout,
   });
 }

lib/core/electrum/domain/ports/server_status_port.dart

@@ -1,12 +1,18 @@
+import 'package:bb_mobile/core/electrum/domain/value_objects/electrum_connection.dart';
 import 'package:bb_mobile/core/utils/bitcoin_tx.dart';
 
 /// Fetches a parsed Bitcoin transaction from a given Electrum server.
 ///
 /// Server selection and fallback are the caller's responsibility — this repo
-/// is a thin abstraction over a single server fetch (with local caching).
+/// is a thin abstraction over a single server fetch (with local caching), and
+/// takes the already-resolved [ElectrumConnection] so the transport (scheme,
+/// certificate validation, timeout) matches what the sync path uses.
 /// Returns the parsed [BitcoinTx]; cross-module domain mapping is left to
 /// callers so the electrum module's domain does not depend on another
 /// module's domain entities.
 abstract class ElectrumTransactionRepository {
-  Future<BitcoinTx> fetch({required String serverUrl, required String txid});
+  Future<BitcoinTx> fetch({
+    required ElectrumConnection connection,
+    required String txid,
+  });
 }

lib/core/electrum/domain/repositories/electrum_transaction_repository.dart

@@ -88,6 +88,7 @@ class ElectrumLocator {
     locator.registerFactory<AddCustomServerUsecase>(
       () => AddCustomServerUsecase(
         electrumServerRepository: locator<ElectrumServerRepository>(),
+        electrumSettingsRepository: locator<ElectrumSettingsRepository>(),
         serverStatusPort: locator<ServerStatusPort>(),
         settingsRepository: locator<SettingsRepository>(),
       ),

lib/core/electrum/frameworks/di/electrum_locator.dart

@@ -1,6 +1,7 @@
 import 'dart:convert';
-import 'dart:io' show SecureSocket;
+import 'dart:io' show SecureSocket, Socket;
 
+import 'package:bb_mobile/core/electrum/domain/value_objects/electrum_connection.dart';
 import 'package:bb_mobile/core/storage/sqlite_database.dart';
 import 'package:bb_mobile/core/utils/bitcoin_tx.dart';
 import 'package:convert/convert.dart';
@@ -11,7 +12,7 @@ class ElectrumRemoteDatasource {
   ElectrumRemoteDatasource({required this._sqlite});
 
   Future<TransactionModel> fetch({
-    required String serverUrl,
+    required ElectrumConnection connection,
     required String txid,
   }) async {
     final cachedTransaction = await _sqlite.managers.transactions
@@ -20,7 +21,7 @@ class ElectrumRemoteDatasource {
 
     if (cachedTransaction != null) return cachedTransaction;
 
-    final txBytes = await _getTransaction(Uri.parse(serverUrl), txid);
+    final txBytes = await _getTransaction(connection, txid);
     final tx = await BitcoinTx.fromBytes(txBytes);
 
     final txModel = TransactionModel(
@@ -37,9 +38,12 @@ class ElectrumRemoteDatasource {
     return txModel;
   }
 
-  Future<List<int>> _getTransaction(Uri serverUri, String txid) async {
+  Future<List<int>> _getTransaction(
+    ElectrumConnection connection,
+    String txid,
+  ) async {
     try {
-      final socket = await SecureSocket.connect(serverUri.host, serverUri.port);
+      final socket = await _connect(connection);
 
       final request = {
         'id': 1,
@@ -58,4 +62,36 @@ class ElectrumRemoteDatasource {
       throw Exception('Electrum RPC error: $e');
     }
   }
+
+  /// Opens the socket described by the resolved [connection] rather than
+  /// assuming a CA-validated TLS endpoint.
+  ///
+  /// A `tcp://` server is reached in the clear, and certificates are validated
+  /// according to the user's `validateDomain` setting — the very flag the
+  /// BDK/LWK sync obeys — so a personal node with a self-signed certificate
+  /// behaves the same on both paths instead of syncing fine but failing here.
+  Future<Socket> _connect(ElectrumConnection connection) {
+    final uri = _parseUrl(connection.url);
+    final timeout = Duration(seconds: connection.timeout);
+
+    if (uri.scheme == 'tcp') {
+      return Socket.connect(uri.host, uri.port, timeout: timeout);
+    }
+
+    return SecureSocket.connect(
+      uri.host,
+      uri.port,
+      timeout: timeout,
+      onBadCertificate: connection.validateDomain ? null : (_) => true,
+    );
+  }
+
+  /// Servers are stored either with an explicit `ssl://` / `tcp://` scheme or
+  /// as a bare `host:port` (how Liquid urls are persisted), which `Uri.parse`
+  /// would otherwise read as the scheme. Bare urls default to TLS, matching
+  /// the rest of the electrum module.
+  Uri _parseUrl(String url) =>
+      url.startsWith('ssl://') || url.startsWith('tcp://')
+      ? Uri.parse(url)
+      : Uri.parse('ssl://$url');
 }

lib/core/electrum/frameworks/drift/datasources/electrum_remote_datasource.dart

@@ -30,7 +30,7 @@ class CreateLogAttachmentUsecase {
       final logContent = logs.join('\n');
       final bytes = Uint8List.fromList(utf8.encode(logContent));
 
-      final random = Random();
+      final random = Random.secure();
       final timestamp = DateFormat('yyyyMMdd_HHmmss').format(DateTime.now());
 
       const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789';