From 0ec98521b98961cf04042c928fae5662e29475a1 Mon Sep 17 00:00:00 2001 From: Alfreedom <00tango.bromine@icloud.com> Date: Thu, 28 Sep 2023 17:51:19 +0200 Subject: [PATCH] add replay protection to core encode/decode --- example/dapp/lib/main.dart | 2 +- example/wallet/ios/Podfile.lock | 4 +- .../ios/Runner.xcodeproj/project.pbxproj | 9 ++- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- lib/apis/core/crypto/crypto.dart | 60 +++++++++++++------ lib/apis/core/crypto/i_crypto.dart | 2 +- lib/apis/core/pairing/pairing.dart | 15 ++--- lib/apis/utils/errors.dart | 5 ++ test/core_api/crypto_test.dart | 12 ++-- test/shared/shared_test_utils.mocks.dart | 38 +++++++----- 10 files changed, 92 insertions(+), 57 deletions(-) diff --git a/example/dapp/lib/main.dart b/example/dapp/lib/main.dart index b111debf..3a7c4612 100644 --- a/example/dapp/lib/main.dart +++ b/example/dapp/lib/main.dart @@ -60,7 +60,7 @@ class _MyHomePageState extends State { Future initialize() async { // try { - print('Project ID: ${DartDefines.projectId}'); + debugPrint('Project ID: ${DartDefines.projectId}'); _web3App = await Web3App.createInstance( projectId: DartDefines.projectId, metadata: const PairingMetadata( diff --git a/example/wallet/ios/Podfile.lock b/example/wallet/ios/Podfile.lock index d26a5d4a..fda1dc09 100644 --- a/example/wallet/ios/Podfile.lock +++ b/example/wallet/ios/Podfile.lock @@ -104,8 +104,8 @@ SPEC CHECKSUMS: mobile_scanner: 47056db0c04027ea5f41a716385542da28574662 nanopb: b552cce312b6c8484180ef47159bc0f65a1f0431 PromisesObjC: 09985d6d70fbe7878040aa746d78236e6946d2ef - shared_preferences_foundation: e2dae3258e06f44cc55f49d42024fd8dd03c590c + shared_preferences_foundation: 5b919d13b803cadd15ed2dc053125c68730e5126 PODFILE CHECKSUM: ef19549a9bc3046e7bb7d2fab4d021637c0c58a3 -COCOAPODS: 1.11.3 +COCOAPODS: 1.13.0 diff --git a/example/wallet/ios/Runner.xcodeproj/project.pbxproj b/example/wallet/ios/Runner.xcodeproj/project.pbxproj index d3ed00bc..d0be1ec2 100644 --- a/example/wallet/ios/Runner.xcodeproj/project.pbxproj +++ b/example/wallet/ios/Runner.xcodeproj/project.pbxproj @@ -121,7 +121,6 @@ E244D63034062C498A453D0D /* Pods-Runner.release.xcconfig */, 36DDA0CA204E24EC1CDA9FAA /* Pods-Runner.profile.xcconfig */, ); - name = Pods; path = Pods; sourceTree = ""; }; @@ -156,7 +155,7 @@ 97C146E61CF9000F007C117D /* Project object */ = { isa = PBXProject; attributes = { - LastUpgradeCheck = 1300; + LastUpgradeCheck = 1430; ORGANIZATIONNAME = ""; TargetAttributes = { 97C146ED1CF9000F007C117D = { @@ -359,7 +358,7 @@ ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; CLANG_ENABLE_MODULES = YES; CURRENT_PROJECT_VERSION = "$(FLUTTER_BUILD_NUMBER)"; - DEVELOPMENT_TEAM = 624CBRZYXF; + DEVELOPMENT_TEAM = W5R8AG9K22; ENABLE_BITCODE = NO; INFOPLIST_FILE = Runner/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( @@ -488,7 +487,7 @@ ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; CLANG_ENABLE_MODULES = YES; CURRENT_PROJECT_VERSION = "$(FLUTTER_BUILD_NUMBER)"; - DEVELOPMENT_TEAM = 624CBRZYXF; + DEVELOPMENT_TEAM = W5R8AG9K22; ENABLE_BITCODE = NO; INFOPLIST_FILE = Runner/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( @@ -511,7 +510,7 @@ ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; CLANG_ENABLE_MODULES = YES; CURRENT_PROJECT_VERSION = "$(FLUTTER_BUILD_NUMBER)"; - DEVELOPMENT_TEAM = 624CBRZYXF; + DEVELOPMENT_TEAM = W5R8AG9K22; ENABLE_BITCODE = NO; INFOPLIST_FILE = Runner/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( diff --git a/example/wallet/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme b/example/wallet/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme index c87d15a3..a6b826db 100644 --- a/example/wallet/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme +++ b/example/wallet/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme @@ -1,6 +1,6 @@ decode( + Future?> decode( String topic, String encoded, { DecodeOptions? options, }) async { _checkInitialized(); - final EncodingValidation params = utils.validateDecoding( + final params = utils.validateDecoding( encoded, receiverPublicKey: options?.receiverPublicKey, ); if (utils.isTypeOneEnvelope(params)) { - final String selfPublicKey = params.receiverPublicKey!; - final String peerPublicKey = params.senderPublicKey!; + final selfPublicKey = params.receiverPublicKey!; + final peerPublicKey = params.senderPublicKey!; topic = await generateSharedKey(selfPublicKey, peerPublicKey); } - final String? symKey = _getSymKey(topic); - if (symKey == null) { - return null; - } - - final String message = await utils.decrypt(symKey, encoded); - return message; + try { + final symKey = _getSymKey(topic); + if (symKey == null) { + return null; + } + + final message = await utils.decrypt(symKey, encoded); + + final payload = jsonDecode(message) as Map; + if (payload.containsKey('topic')) { + final payloadTopic = payload['topic'] as String; + if (payloadTopic != topic) { + throw Errors.getInternalError( + Errors.MISMATCHED_TOPIC, + context: 'decode() Mismatched topic decoded from message.', + ); + } + payload.remove('topic'); + } + + return payload; + } catch (e) { + throw Errors.getInternalError( + Errors.PARSING_FAILED, + context: 'Failed to decode message from topic: $topic, ' + 'clientId: ${await getClientId()} ' + 'Exception: $e', + ); + } } @override diff --git a/lib/apis/core/crypto/i_crypto.dart b/lib/apis/core/crypto/i_crypto.dart index de9663e4..74f28704 100644 --- a/lib/apis/core/crypto/i_crypto.dart +++ b/lib/apis/core/crypto/i_crypto.dart @@ -28,7 +28,7 @@ abstract class ICrypto { Map payload, { EncodeOptions? options, }); - Future decode( + Future?> decode( String topic, String encoded, { DecodeOptions? options, diff --git a/lib/apis/core/pairing/pairing.dart b/lib/apis/core/pairing/pairing.dart index c9743649..700456ee 100644 --- a/lib/apis/core/pairing/pairing.dart +++ b/lib/apis/core/pairing/pairing.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:convert'; import 'package:event/event.dart'; import 'package:walletconnect_flutter_v2/apis/core/pairing/i_json_rpc_history.dart'; @@ -630,7 +629,7 @@ class Pairing implements IPairing { } // Decode the message - String? payloadString = await core.crypto.decode( + Map? payload = await core.crypto.decode( event.topic, event.message, options: DecodeOptions( @@ -638,18 +637,16 @@ class Pairing implements IPairing { ), ); - if (payloadString == null) { + if (payload == null) { return; } - // print(payloadString); - Map data = jsonDecode(payloadString); - core.logger.i('Pairing _onMessageEvent, Received data: $data'); + core.logger.i('Pairing _onMessageEvent, Received data: $payload'); // If it's an rpc request, handle it // print('Pairing: Received data: $data'); - if (data.containsKey('method')) { - final request = JsonRpcRequest.fromJson(data); + if (payload.containsKey('method')) { + final request = JsonRpcRequest.fromJson(payload); if (routerMapRequest.containsKey(request.method)) { routerMapRequest[request.method]!.function(event.topic, request); } else { @@ -658,7 +655,7 @@ class Pairing implements IPairing { } // Otherwise handle it as a response else { - final response = JsonRpcResponse.fromJson(data); + final response = JsonRpcResponse.fromJson(payload); // Only handle the response if we have a record of the request // final JsonRpcRecord? record = history.get(response.id.toString()); diff --git a/lib/apis/utils/errors.dart b/lib/apis/utils/errors.dart index 8eafa574..d24465ab 100644 --- a/lib/apis/utils/errors.dart +++ b/lib/apis/utils/errors.dart @@ -187,6 +187,7 @@ class Errors { static const UNKNOWN_TYPE = 'UNKNOWN_TYPE'; static const MISMATCHED_TOPIC = 'MISMATCHED_TOPIC'; static const NON_CONFORMING_NAMESPACES = 'NON_CONFORMING_NAMESPACES'; + static const PARSING_FAILED = 'PARSING_FAILED'; static const INTERNAL_ERRORS = { NOT_INITIALIZED: { @@ -225,6 +226,10 @@ class Errors { 'message': 'Non conforming namespaces.', 'code': 9, }, + PARSING_FAILED: { + 'message': 'Failed to decode/encode.', + 'code': 10, + }, }; static WalletConnectError getInternalError( diff --git a/test/core_api/crypto_test.dart b/test/core_api/crypto_test.dart index b58a1b63..28ada850 100644 --- a/test/core_api/crypto_test.dart +++ b/test/core_api/crypto_test.dart @@ -234,11 +234,11 @@ void main() { final topic = await cryptoAPI.setSymKey(SYM_KEY); final String? encoded = await cryptoAPI.encode(topic, PAYLOAD); - final String? decoded = await cryptoAPI.decode(topic, encoded!); - expect(decoded, jsonEncode(PAYLOAD)); + final decoded = await cryptoAPI.decode(topic, encoded!); + expect(jsonEncode(decoded), jsonEncode(PAYLOAD)); - final String? decoded2 = await cryptoAPI.decode(topic, ENCODED); - expect(decoded2, jsonEncode(PAYLOAD)); + final decoded2 = await cryptoAPI.decode(topic, ENCODED); + expect(jsonEncode(decoded2), jsonEncode(PAYLOAD)); }, ); @@ -246,10 +246,10 @@ void main() { 'returns null if the passed topic is known', () async { final topic = CryptoUtils().hashKey(SYM_KEY); - final String? encoded = await cryptoAPI.encode(topic, PAYLOAD); + final encoded = await cryptoAPI.encode(topic, PAYLOAD); expect(encoded, isNull); - final String? decoded = await cryptoAPI.decode(topic, ENCODED); + final decoded = await cryptoAPI.decode(topic, ENCODED); expect(decoded, isNull); }, ); diff --git a/test/shared/shared_test_utils.mocks.dart b/test/shared/shared_test_utils.mocks.dart index 772acccc..a72d5ffe 100644 --- a/test/shared/shared_test_utils.mocks.dart +++ b/test/shared/shared_test_utils.mocks.dart @@ -584,7 +584,7 @@ class MockCrypto extends _i1.Mock implements _i19.Crypto { returnValue: _i18.Future.value(), ) as _i18.Future); @override - _i18.Future decode( + _i18.Future?> decode( String? topic, String? encoded, { _i2.DecodeOptions? options, @@ -598,8 +598,8 @@ class MockCrypto extends _i1.Mock implements _i19.Crypto { ], {#options: options}, ), - returnValue: _i18.Future.value(), - ) as _i18.Future); + returnValue: _i18.Future?>.value(), + ) as _i18.Future?>); @override _i18.Future signJWT(String? aud) => (super.noSuchMethod( Invocation.method( @@ -924,6 +924,19 @@ class MockCore extends _i1.Mock implements _i23.Core { _i1.throwOnMissingStub(this); } + @override + String get relayUrl => (super.noSuchMethod( + Invocation.getter(#relayUrl), + returnValue: '', + ) as String); + @override + set relayUrl(String? _relayUrl) => super.noSuchMethod( + Invocation.setter( + #relayUrl, + _relayUrl, + ), + returnValueForMissingStub: null, + ); @override String get projectId => (super.noSuchMethod( Invocation.getter(#projectId), @@ -1015,14 +1028,6 @@ class MockCore extends _i1.Mock implements _i23.Core { returnValueForMissingStub: null, ); @override - _i15.Logger get logger => (super.noSuchMethod( - Invocation.getter(#logger), - returnValue: _FakeLogger_15( - this, - Invocation.getter(#logger), - ), - ) as _i15.Logger); - @override _i7.IStore> get storage => (super.noSuchMethod( Invocation.getter(#storage), returnValue: _FakeIStore_7>( @@ -1049,10 +1054,13 @@ class MockCore extends _i1.Mock implements _i23.Core { returnValue: '', ) as String); @override - String get relayUrl => (super.noSuchMethod( - Invocation.getter(#relayUrl), - returnValue: '', - ) as String); + _i15.Logger get logger => (super.noSuchMethod( + Invocation.getter(#logger), + returnValue: _FakeLogger_15( + this, + Invocation.getter(#logger), + ), + ) as _i15.Logger); @override _i18.Future start() => (super.noSuchMethod( Invocation.method(