Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gh 2886/warning in transactions #3364

Merged
merged 6 commits into from
Nov 21, 2023
Merged

Gh 2886/warning in transactions #3364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1d8f2e6
GH-2886 add check for transaction signatures
DmitryBespalov Nov 20, 2023
1e1490d
GH-2886 disable warning while not ready
DmitryBespalov Nov 20, 2023
2b1b52c
GH-2886 test happy case
DmitryBespalov Nov 21, 2023
036b4f5
GH-2886 test cases for all branches of validation logic
DmitryBespalov Nov 21, 2023
55513ea
GH-2886 add descriptions
DmitryBespalov Nov 21, 2023
852398e
GH-2886 change names
DmitryBespalov Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions Multisig.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@
04FEFE8128C6304B0028A349 /* TokenDistributionViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 04FEFE7F28C6304B0028A349 /* TokenDistributionViewController.xib */; };
0A007124254B255C00A57CAF /* UIFont+Styles.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A007123254B255C00A57CAF /* UIFont+Styles.swift */; };
0A00712A254B379000A57CAF /* UIColor+Styles.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A007129254B379000A57CAF /* UIColor+Styles.swift */; };
0A01DCE42770B0A100A87D7A /* TransactionIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A01DCE32770B0A100A87D7A /* TransactionIntegrationTests.swift */; };
0A0391CB27C565D4003C871A /* IconButtonTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 0A0391C727C565D3003C871A /* IconButtonTableViewCell.xib */; };
0A0391CC27C565D4003C871A /* HelpTextTableViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A0391C827C565D3003C871A /* HelpTextTableViewCell.swift */; };
0A0391CD27C565D4003C871A /* HelpTextTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 0A0391C927C565D3003C871A /* HelpTextTableViewCell.xib */; };
Expand Down Expand Up @@ -1026,8 +1025,17 @@
B32620492A961E690003A2F0 /* AddSafeFlow.swift in Sources */ = {isa = PBXBuildFile; fileRef = B32620482A961E690003A2F0 /* AddSafeFlow.swift */; };
B343BEB32A77DDA1006BF46B /* DMSans-Bold.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 0491067C28F99246005A4A99 /* DMSans-Bold.ttf */; };
B35BFD0F2A937DC000A9FB15 /* NavigationRouterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B35BFD0E2A937DC000A9FB15 /* NavigationRouterTests.swift */; };
B367AED02B0CB69200F06B86 /* TransactionValidationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B367AECE2B0CB59700F06B86 /* TransactionValidationTests.swift */; };
B367AED22B0CB89600F06B86 /* TransactionValidationTestCase1.json in Resources */ = {isa = PBXBuildFile; fileRef = B367AED12B0CB89600F06B86 /* TransactionValidationTestCase1.json */; };
B3710B3D2AD584BE002E503B /* SecureConfig in Frameworks */ = {isa = PBXBuildFile; productRef = B3710B3C2AD584BE002E503B /* SecureConfig */; };
B3710B442AD5AAD7002E503B /* config.bundle in Resources */ = {isa = PBXBuildFile; fileRef = B3710B432AD5AAD7002E503B /* config.bundle */; };
B380756B2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json in Resources */ = {isa = PBXBuildFile; fileRef = B380756A2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json */; };
B380756D2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json in Resources */ = {isa = PBXBuildFile; fileRef = B380756C2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json */; };
B380756F2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json in Resources */ = {isa = PBXBuildFile; fileRef = B380756E2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json */; };
B38075712B0CF75200C62BCF /* TransactionValidationTestCase_SignerNotMatchingSignature.json in Resources */ = {isa = PBXBuildFile; fileRef = B38075702B0CF75200C62BCF /* TransactionValidationTestCase_SignerNotMatchingSignature.json */; };
B38075732B0CF7C600C62BCF /* TransactionValidationTestCase_MultipleConfirmations.json in Resources */ = {isa = PBXBuildFile; fileRef = B38075722B0CF7C600C62BCF /* TransactionValidationTestCase_MultipleConfirmations.json */; };
B38075752B0CF85900C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_1_0.json in Resources */ = {isa = PBXBuildFile; fileRef = B38075742B0CF85900C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_1_0.json */; };
B38075772B0CFD3A00C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_0_0.json in Resources */ = {isa = PBXBuildFile; fileRef = B38075762B0CFD3A00C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_0_0.json */; };
B3B6044F2A850F5E007BDAC0 /* UIAlertControllerStyle+Multiplatform.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B6044E2A850F5E007BDAC0 /* UIAlertControllerStyle+Multiplatform.swift */; };
B3C4D2D92AAF596D0026A8BC /* UIViewController+Navigation.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3C4D2D82AAF596D0026A8BC /* UIViewController+Navigation.swift */; };
D80B5A032769CEAD00D6E024 /* Tooltip.swift in Sources */ = {isa = PBXBuildFile; fileRef = D80B5A022769CEAD00D6E024 /* Tooltip.swift */; };
Expand Down Expand Up @@ -2082,10 +2090,19 @@
B300E8CA2AF3A7A90073A908 /* SafeNoncesRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SafeNoncesRequest.swift; sourceTree = "<group>"; };
B32620482A961E690003A2F0 /* AddSafeFlow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddSafeFlow.swift; sourceTree = "<group>"; };
B35BFD0E2A937DC000A9FB15 /* NavigationRouterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationRouterTests.swift; sourceTree = "<group>"; };
B367AECE2B0CB59700F06B86 /* TransactionValidationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TransactionValidationTests.swift; sourceTree = "<group>"; };
B367AED12B0CB89600F06B86 /* TransactionValidationTestCase1.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = TransactionValidationTestCase1.json; sourceTree = "<group>"; };
B3710B3B2AD58492002E503B /* SecureConfig */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = SecureConfig; path = Packages/SecureConfig; sourceTree = "<group>"; };
B3710B432AD5AAD7002E503B /* config.bundle */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.plug-in"; path = config.bundle; sourceTree = "<group>"; };
B3710B452AD5AE9D002E503B /* apis-prod.example.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "apis-prod.example.json"; sourceTree = "<group>"; };
B3710B462AD5AE9D002E503B /* apis-staging.example.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "apis-staging.example.json"; sourceTree = "<group>"; };
B380756A2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = TransactionValidationTestCase_InvalidSafeTxHash.json; sourceTree = "<group>"; };
B380756C2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_NoConfirmations.json; sourceTree = "<group>"; };
B380756E2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_ConfirmationsNotFromOwners.json; sourceTree = "<group>"; };
B38075702B0CF75200C62BCF /* TransactionValidationTestCase_SignerNotMatchingSignature.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_SignerNotMatchingSignature.json; sourceTree = "<group>"; };
B38075722B0CF7C600C62BCF /* TransactionValidationTestCase_MultipleConfirmations.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_MultipleConfirmations.json; sourceTree = "<group>"; };
B38075742B0CF85900C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_1_0.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_AllSignatureTypes_v1_1_0.json; sourceTree = "<group>"; };
B38075762B0CFD3A00C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_0_0.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = TransactionValidationTestCase_AllSignatureTypes_v1_0_0.json; sourceTree = "<group>"; };
B3B6044E2A850F5E007BDAC0 /* UIAlertControllerStyle+Multiplatform.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIAlertControllerStyle+Multiplatform.swift"; sourceTree = "<group>"; };
B3C4D2D82AAF596D0026A8BC /* UIViewController+Navigation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIViewController+Navigation.swift"; sourceTree = "<group>"; };
D80B5A022769CEAD00D6E024 /* Tooltip.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Tooltip.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3477,6 +3494,15 @@
6A91EFD827DA2C45009E63E9 /* Data */,
0A3DFF002667D7DA00B45770 /* CoreData */,
0A64313F247ED1AA006FD30A /* MultisigIntegrationTests.xctestplan */,
B367AECE2B0CB59700F06B86 /* TransactionValidationTests.swift */,
B367AED12B0CB89600F06B86 /* TransactionValidationTestCase1.json */,
B38075762B0CFD3A00C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_0_0.json */,
B38075742B0CF85900C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_1_0.json */,
B38075722B0CF7C600C62BCF /* TransactionValidationTestCase_MultipleConfirmations.json */,
B380756C2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json */,
B38075702B0CF75200C62BCF /* TransactionValidationTestCase_SignerNotMatchingSignature.json */,
B380756E2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json */,
B380756A2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json */,
0A802B8F24E581A50001790F /* SafeClientGatewayServiceIntegrationTests.swift */,
0A513A9C2768EBC900F07D5A /* DelegateKeyTests.swift */,
5532D4D02449A1E40067505A /* MockLogger.swift */,
Expand Down Expand Up @@ -5215,10 +5241,18 @@
isa = PBXResourcesBuildPhase;
buildActionMask = 2147483647;
files = (
B38075732B0CF7C600C62BCF /* TransactionValidationTestCase_MultipleConfirmations.json in Resources */,
B38075772B0CFD3A00C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_0_0.json in Resources */,
B38075752B0CF85900C62BCF /* TransactionValidationTestCase_AllSignatureTypes_v1_1_0.json in Resources */,
0AA5F55F28BF6F2200D6D220 /* claiming_test_fixtures.yml in Resources */,
B380756B2B0CF45200C62BCF /* TransactionValidationTestCase_InvalidSafeTxHash.json in Resources */,
B380756D2B0CF60A00C62BCF /* TransactionValidationTestCase_NoConfirmations.json in Resources */,
0A61E8452670E632009D68A4 /* __Snapshots__ in Resources */,
6A91EFDA27DA2C8D009E63E9 /* wc_registry_wallets.json in Resources */,
0AA5F55D28BF667000D6D220 /* claiming_test_fixtures.json in Resources */,
B380756F2B0CF6AC00C62BCF /* TransactionValidationTestCase_ConfirmationsNotFromOwners.json in Resources */,
B367AED22B0CB89600F06B86 /* TransactionValidationTestCase1.json in Resources */,
B38075712B0CF75200C62BCF /* TransactionValidationTestCase_SignerNotMatchingSignature.json in Resources */,
0A74719A269F152D008E9F2D /* chains_4_safes_0x1230B3d59858296A31053C1b8562Ecf89A2f888b_balances_usd.json in Resources */,
0A3DFEFE2667D3A700B45770 /* README.md in Resources */,
);
Expand Down Expand Up @@ -5996,13 +6030,13 @@
0A9BC35F246058F800EB9C5D /* MockLogger.swift in Sources */,
550A6182268A23A9002C02E1 /* SafeNetworkMigrationTests.swift in Sources */,
0A7B3518288071EB00EC04EC /* TestCoreDataStack.swift in Sources */,
0A01DCE42770B0A100A87D7A /* TransactionIntegrationTests.swift in Sources */,
0A65522B28816B9900701238 /* TestingAppDelegate.swift in Sources */,
0A61E83E2670DED5009D68A4 /* BalanceTableViewCellTests.swift in Sources */,
0A3DFEF72667D1C000B45770 /* CoreDataTestCase.swift in Sources */,
0A06532F27B68AF40008C5F1 /* WebConnectionRepositoryTests.swift in Sources */,
0A6552282881642300701238 /* UIIntegrationTestCase.swift in Sources */,
0A802B9024E581A50001790F /* SafeClientGatewayServiceIntegrationTests.swift in Sources */,
B367AED02B0CB69200F06B86 /* TransactionValidationTests.swift in Sources */,
0AC62ED628783700007945A1 /* CreatePasscodeFlowTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
4 changes: 2 additions & 2 deletions Multisig.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,8 @@
"repositoryURL": "https://github.com/WalletConnect/WalletConnectSwiftV2",
"state": {
"branch": null,
"revision": "7cf45ea8f98097b5813cd3c99d08b87c1be28ebe",
"version": "1.9.5"
"revision": "4aa4c8229077c133730e361d0d7a17f9e56bdffd",
"version": "1.9.6"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion Multisig/Logic/Models/Transaction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ extension Transaction {
.reduce(Data()) { $0 + $1 }
}

private func safeTransactionHash() -> HashString? {
func safeTransactionHash() -> HashString? {
let data = encodeTransactionData()
return try? HashString(hex: EthHasher.hash(data).toHexString())
}
Expand Down
141 changes: 141 additions & 0 deletions Multisig/UI/Transaction/TransactionDetailsViewController/TransactionDetailCellBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import Foundation
import UIKit
import SwiftCryptoTokenFormatter
import SwiftUI
import Version
import Solidity
import SafeWeb3

class TransactionDetailCellBuilder {

Expand Down Expand Up @@ -43,6 +46,7 @@ class TransactionDetailCellBuilder {
tableView.registerCell(DetailTransferInfoCell.self)
tableView.registerCell(DetailRejectionInfoCell.self)
tableView.registerCell(DetailStatusCell.self)
tableView.registerCell(WarningTableViewCell.self)
}

func build(_ tx: SCGModels.TransactionDetails) -> [UITableViewCell] {
Expand All @@ -54,6 +58,7 @@ class TransactionDetailCellBuilder {
func buildTransaction(_ tx: SCGModels.TransactionDetails) {
let isCreationTx = buildCreationTx(tx)
if !isCreationTx {
buildWarning(tx)
buildHeader(tx)
buildAssetContract(tx)
buildStatus(tx)
Expand Down Expand Up @@ -427,6 +432,142 @@ class TransactionDetailCellBuilder {
addressLogoUri: addressLogoUri,
isOutgoing: isOutgoing)
}

func validate(tx: SCGModels.TransactionDetails, safe: Safe) throws {
guard tx.txStatus.isAwatingConfiramtions || tx.txStatus == .awaitingExecution else {
// don't warn outside of signature or execution requests
return
}

// safe has owners
guard let ownersInfo = safe.ownersInfo, !ownersInfo.isEmpty else {
// not enough data for further checks
return
}
let ownerAddresses = ownersInfo.map(\.address)

// transaction has confirmations
guard let txMultisigInfo = tx.multisigInfo else {
// not a kind of transaction we can take a look into
return
}

guard !txMultisigInfo.confirmations.isEmpty else {
throw "Transaction has no confirmations."
}

// all confirming addresses are from safe owners
guard txMultisigInfo.confirmations.allSatisfy({ confirmation in
ownerAddresses.contains(confirmation.signer.value.address)
}) else {
throw "Not all confirmations are from safe owners."
}

// transaction hash is valid
guard var transaction = Transaction(tx: tx),
let contractVersion = safe.version.flatMap(Version.init),
let chainId = safe.chain?.id
else {
// not enough information for further checks
return
}
transaction.safe = AddressString(stringLiteral: safe.displayAddress)
transaction.safeVersion = contractVersion
transaction.chainId = chainId

guard
let computedSafeTxHash = transaction.safeTransactionHash(),
transaction.safeTxHash == computedSafeTxHash
else {
throw "Invalid safeTxHash. This may be a dangerous transaction."
}

// all confirming signatures are from a confirming addresses
func signer(of signature: Data) -> Address? {
guard signature.count >= 65 else {
// cannot decode signature
return nil
}

let r: Data /* 32 bytes */ = signature[0..<32]
let s: Data /* 32 bytes */ = signature[32..<64]
let v: UInt8 = signature[64]

let contractSignature: UInt8 = 0
let approvedHashSignature: UInt8 = 1
let ethSignSignature: ClosedRange<UInt8> = (31...UInt8.max)

if contractVersion >= Version(1, 1, 0) {
switch v {
case contractSignature, approvedHashSignature:
let owner = (try? Sol.Address(data: r)).map { Address($0) }
return owner

case ethSignSignature:
let hash = computedSafeTxHash.hash
let message = "\u{19}Ethereum Signed Message:\n\(hash.count)".data(using: .utf8)! + hash

let pubKey = try? EthereumPublicKey(
message: message.makeBytes(),
v: EthereumQuantity(quantity: (BigUInt(v) - 4) - 27),
r: EthereumQuantity(r.makeBytes()),
s: EthereumQuantity(s.makeBytes())
)

let owner = pubKey.map(\.address).map(Address.init)
return owner

default:
let message = transaction.encodeTransactionData()

let pubKey = try? EthereumPublicKey(
message: message.makeBytes(),
v: EthereumQuantity(quantity: v >= 27 ? BigUInt(v) - 27 : BigUInt(v)),
r: EthereumQuantity(r.makeBytes()),
s: EthereumQuantity(s.makeBytes())
)
let owner = pubKey.map(\.address).map(Address.init)
return owner
}
} else {
switch v {
case contractSignature, approvedHashSignature:
let owner = (try? Sol.Address(data: r)).map { Address($0) }
return owner
default:
let message = transaction.encodeTransactionData()

let pubKey = try? EthereumPublicKey(
message: message.makeBytes(),
v: EthereumQuantity(quantity: v >= 27 ? BigUInt(v) - 27 : BigUInt(v)),
r: EthereumQuantity(r.makeBytes()),
s: EthereumQuantity(s.makeBytes())
)
let owner = pubKey.map(\.address).map(Address.init)
return owner
}
}
}

guard txMultisigInfo.confirmations.allSatisfy({ confirmation in
signer(of: confirmation.signature.data) == confirmation.signer.value.address
}) else {
throw "Not all signatures are from safe's owners. This may be a dangerous transaction."
}
}

func buildWarning(_ tx: SCGModels.TransactionDetails) {
do {
guard let aSafe = Safe.by(address: tx.safeAddress.description, chainId: chain.id!) else {
return
}
try validate(tx: tx, safe: aSafe)
} catch {
let cell = newCell(WarningTableViewCell.self)
cell.set(title: "Warning!", description: error.localizedDescription)
result.append(cell)
}
}


func buildActions(_ tx: SCGModels.TransactionDetails) {
Expand Down
Loading
Loading