From 4c707474207ad4746b0def653d7d7fab94cd0790 Mon Sep 17 00:00:00 2001 From: Dmitry Bespalov Date: Fri, 31 May 2024 11:43:04 +0200 Subject: [PATCH 1/2] GH-3406 another attempt to fix crash Potential reason: crash in arithmetic operations --- .../KeystoneOwnerKey/KeystoneSignFlow.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift b/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift index acc9097b2..62d20853d 100644 --- a/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift +++ b/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift @@ -113,8 +113,8 @@ extension SECP256K1.UnmarshaledSignature { let vBytes = [UInt8](data.suffix(from: 64).prefix(8)) let vInt = UInt64(vBytes) // recover V by deducting (chainId * 2 + 35) according to EIP-155 - let vRecovered = vInt - (chainIdInt * 2 + 35) - v = try! UInt8(vRecovered % 256) + let vRecovered = vInt % 256 - (chainIdInt * 2 + 35) % 256 + v = try! UInt8(vRecovered) } else { vBytes = [UInt8]([data[64]]) let vInt = UInt8(vBytes) @@ -122,10 +122,10 @@ extension SECP256K1.UnmarshaledSignature { // Legacy ethereum (pre-eip-155) adds 27 to v v = vInt - 27 } else { - // v still can be chainId * 2 + 35 for non-legacy transactions (chaiId >=0) + // v still can be `{0, 1} + chainId * 2 + 35` for non-legacy transactions (chainId >=0) if vInt >= 35 { - let vRecovered = UInt64(vBytes) - (chainIdInt * 2 + 35) - v = try! UInt8(vRecovered % 256) + let vRecovered = UInt64(vBytes) - (chainIdInt * 2 + 35) % 256 + v = try! UInt8(vRecovered) } else { v = vInt } @@ -138,3 +138,4 @@ extension SECP256K1.UnmarshaledSignature { return (v, r, s) } } + From fb255a506d0142023b4ff09c007b70c763955acb Mon Sep 17 00:00:00 2001 From: Dmitry Bespalov Date: Fri, 31 May 2024 12:45:06 +0200 Subject: [PATCH 2/2] GH-3406 avoiding overflow on multiplication and addition in 64-bit --- .../KeystoneOwnerKey/KeystoneSignFlow.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift b/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift index 62d20853d..aa62aa4bb 100644 --- a/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift +++ b/Multisig/UI/Settings/OwnerKeyManagement/KeystoneOwnerKey/KeystoneSignFlow.swift @@ -10,6 +10,7 @@ import Foundation import SwiftUI import URRegistry import SafeWeb3 +import Solidity final class KeystoneSignFlow: UIFlow { var signCompletion: ((_ unmarshaledSignature: SECP256K1.UnmarshaledSignature) -> Void)? @@ -108,12 +109,13 @@ extension SECP256K1.UnmarshaledSignature { let vBytes: Bytes // if v was overflown (e.g. chain_id > 109 according to EIP-155) + let chainIdTerm = UInt64((Sol.UInt128(chainIdInt) * 2 + 35) % 256) if data.count > 65 { // max 8 bytes to fit into UInt64 let vBytes = [UInt8](data.suffix(from: 64).prefix(8)) let vInt = UInt64(vBytes) // recover V by deducting (chainId * 2 + 35) according to EIP-155 - let vRecovered = vInt % 256 - (chainIdInt * 2 + 35) % 256 + let vRecovered = vInt % 256 - chainIdTerm v = try! UInt8(vRecovered) } else { vBytes = [UInt8]([data[64]]) @@ -124,7 +126,7 @@ extension SECP256K1.UnmarshaledSignature { } else { // v still can be `{0, 1} + chainId * 2 + 35` for non-legacy transactions (chainId >=0) if vInt >= 35 { - let vRecovered = UInt64(vBytes) - (chainIdInt * 2 + 35) % 256 + let vRecovered = UInt64(vBytes) - chainIdTerm v = try! UInt8(vRecovered) } else { v = vInt