From 7ad3fe125c993b23eb24f538cbcabecacc7f842a Mon Sep 17 00:00:00 2001 From: Jason McPheron Date: Fri, 25 Sep 2026 00:29:08 -0700 Subject: [PATCH] fix: keep EIP-1271 contract signatures when rebuilding a queued tx (GS021) createExistingTx (web) and addSignaturesToTx (mobile) added every confirmation from the Transaction Service as a plain signature: staticPart = the whole stored signature, dynamicPart = ''. A contract owner's confirmation (a nested Safe, a passkey signer) is stored standalone as {owner}{offset = 65}{00}{length}{data}, so it was concatenated whole with its offset still at 65. When the Safe's threshold is 2 or more and that confirmation is among the ones checked, the offset points inside the static region and the Safe reverts with GS021. toSafeSignature (packages/utils) turns a stored confirmation back into an EthSafeSignature. For v = 0 it reads the encoded offset, requires it past the 65-byte static part with a length word whose data runs exactly to the end, and returns a contract signature with just that data, so buildSignatureBytes lays it out. Anything else stays a plain signature, as before. Tests use confirmations from Safes 1.5.0 owned by throwaway keys: a 2-of-2 (EOA, contract) and a 3-of-3 (contract, EOA, contract). The rebuilt bytes are checked against what those Safes executed with on a local fork. Also covered: eth_sign and pre-approved (v = 1) signatures unchanged, input order, and malformed v = 0 signatures left plain. Refs #3673 --- .../src/services/tx/tx-sender/create.ts | 9 +- apps/web/src/services/tx/tx-sender/create.ts | 9 +- .../utils/__tests__/safeTransaction.test.ts | 137 ++++++++++++++++++ packages/utils/src/utils/safeTransaction.ts | 42 +++++- 4 files changed, 181 insertions(+), 16 deletions(-) create mode 100644 packages/utils/src/utils/__tests__/safeTransaction.test.ts diff --git a/apps/mobile/src/services/tx/tx-sender/create.ts b/apps/mobile/src/services/tx/tx-sender/create.ts index 8ad21f3..9772a2a 100644 --- a/apps/mobile/src/services/tx/tx-sender/create.ts +++ b/apps/mobile/src/services/tx/tx-sender/create.ts @@ -4,6 +4,7 @@ import extractTxInfo from '@/src/services/tx/extractTx' import { createConnectedWallet } from '../../web3' import { SafeInfo } from '@/src/types/address' import type { SafeTransaction, SafeTransactionDataPartial } from '@safe-global/types-kit' +import { toSafeSignature } from '@safe-global/utils/utils/safeTransaction' import { getSafeSDK } from '@/src/hooks/coreSDK/safeCoreSDK' import { TransactionDetails } from '@safe-global/store/gateway/AUTO_GENERATED/transactions' @@ -39,13 +40,7 @@ export const createTx = async (txParams: SafeTransactionDataPartial, nonce?: num */ export const addSignaturesToTx = (safeTx: SafeTransaction, signatures: Record): void => { Object.entries(signatures).forEach(([signer, data]) => { - safeTx.addSignature({ - signer, - data, - staticPart: () => data, - dynamicPart: () => '', - isContractSignature: false, - }) + safeTx.addSignature(toSafeSignature(signer, data)) }) } diff --git a/apps/web/src/services/tx/tx-sender/create.ts b/apps/web/src/services/tx/tx-sender/create.ts index 95dd682..e66262c 100644 --- a/apps/web/src/services/tx/tx-sender/create.ts +++ b/apps/web/src/services/tx/tx-sender/create.ts @@ -1,6 +1,7 @@ import type { TransactionDetails } from '@safe-global/store/gateway/AUTO_GENERATED/transactions' import { getReadOnlyGnosisSafeContract } from '@/services/contracts/safeContracts' import { SENTINEL_ADDRESS } from '@safe-global/utils/utils/constants' +import { toSafeSignature } from '@safe-global/utils/utils/safeTransaction' import type { Chain } from '@safe-global/store/gateway/AUTO_GENERATED/chains' import { getTransactionDetails } from '@/utils/transactions' import type { AddOwnerTxParams, RemoveOwnerTxParams, SwapOwnerTxParams } from '@safe-global/protocol-kit' @@ -131,13 +132,7 @@ export const createExistingTx = async ( // Create a tx and add pre-approved signatures const safeTx = await createTx(txParams, txParams.nonce, scope) Object.entries(signatures).forEach(([signer, data]) => { - safeTx.addSignature({ - signer, - data, - staticPart: () => data, - dynamicPart: () => '', - isContractSignature: false, - }) + safeTx.addSignature(toSafeSignature(signer, data)) }) return safeTx diff --git a/packages/utils/src/utils/__tests__/safeTransaction.test.ts b/packages/utils/src/utils/__tests__/safeTransaction.test.ts new file mode 100644 index 0000000..1a38beb --- /dev/null +++ b/packages/utils/src/utils/__tests__/safeTransaction.test.ts @@ -0,0 +1,137 @@ +import { buildSignatureBytes } from '@safe-global/protocol-kit' +import { toSafeSignature } from '../safeTransaction' + +// Confirmations in the form the Transaction Service returns them, for Safes 1.5.0 owned by +// throwaway keys (EOAs and SafeWebAuthnSignerProxy passkey signers) on a local fork of Base +// Sepolia. `executed` is what the Safe's SafeMultiSigTransaction event logged when execTransaction +// ran with the fixed layout; each execution also emitted ExecutionSuccess and paid the recipient. +// The same confirmations laid out as plain signatures (the old createExistingTx) reverted with +// GS021. Generated with a fork script (see the PR description), sorted by owner. +const TWO_OF_TWO = { + confirmations: [ + { + owner: '0xaF9503e481058ACB546d033B0Bb5B9cDf5c709ab', + signature: + '0xe5bb15e1131ebbf9a5cf3778e69658734f78ae4c3b7208d068c923eb737c5a142d0df4205133c98f917390e1c4d15872d015cfc79e1c9d85841236d06ae075ea1c', + }, // EOA + { + owner: '0xE9F5f604cbc833A32bBFaf7dDf896fB66f5Af073', + signature: + '0x000000000000000000000000E9F5f604cbc833A32bBFaf7dDf896fB66f5Af0730000000000000000000000000000000000000000000000000000000000000041000000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0c2ad6261a5e92c3ca6e11f3ba49566f6a627a0671f20c175516c0d8ce09937ea5d15f4326d9cc2842f132abf73c4b50f1d7358031fe79852b4593e0f3c5bfde200000000000000000000000000000000000000000000000000000000000000256589e4cecfa8f8e42009c6cff7b161cf09e40ae6af81ff73aa96e47f7f7ce28405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020226f726967696e223a2268747470733a2f2f67733032312e6578616d706c6522', + }, // CONTRACT + ], + executed: + '0xe5bb15e1131ebbf9a5cf3778e69658734f78ae4c3b7208d068c923eb737c5a142d0df4205133c98f917390e1c4d15872d015cfc79e1c9d85841236d06ae075ea1c000000000000000000000000e9f5f604cbc833a32bbfaf7ddf896fb66f5af0730000000000000000000000000000000000000000000000000000000000000082000000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0c2ad6261a5e92c3ca6e11f3ba49566f6a627a0671f20c175516c0d8ce09937ea5d15f4326d9cc2842f132abf73c4b50f1d7358031fe79852b4593e0f3c5bfde200000000000000000000000000000000000000000000000000000000000000256589e4cecfa8f8e42009c6cff7b161cf09e40ae6af81ff73aa96e47f7f7ce28405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020226f726967696e223a2268747470733a2f2f67733032312e6578616d706c6522', +} + +const THREE_OF_THREE = { + confirmations: [ + { + owner: '0x9ef150E9bDdd63c157E3162B83a565423b2c5a20', + signature: + '0x0000000000000000000000009ef150E9bDdd63c157E3162B83a565423b2c5a200000000000000000000000000000000000000000000000000000000000000041000000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0edbb4bbe5fc2c7283a1093676b395bd241dee77019cec81423615e1d9bd552ff4bfd5621ab5dd4f0e92cba541f4ab8e250c6393fddfb8690c514f5b3d26d15f400000000000000000000000000000000000000000000000000000000000000256589e4cecfa8f8e42009c6cff7b161cf09e40ae6af81ff73aa96e47f7f7ce28405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020226f726967696e223a2268747470733a2f2f67733032312e6578616d706c6522', + }, // CONTRACT + { + owner: '0xaF9503e481058ACB546d033B0Bb5B9cDf5c709ab', + signature: + '0xa31c68b6b3a48786a4882079dacb37e27134d3f117f00af247353899562b1b1c0238502d31a96b0881b9d2852c12ed8ad2f69e2b1aebe0d4919b4b648f403a571b', + }, // EOA + { + owner: '0xE9F5f604cbc833A32bBFaf7dDf896fB66f5Af073', + signature: + '0x000000000000000000000000E9F5f604cbc833A32bBFaf7dDf896fB66f5Af0730000000000000000000000000000000000000000000000000000000000000041000000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0de03c6c8810e9a4e0cc9b6b4b657f63557005c827b7b44e7458be9ebd692972704368085ec366bd78aa760ee3b01d3ea87d269d668d988f871ae0d94fe3a657b00000000000000000000000000000000000000000000000000000000000000256589e4cecfa8f8e42009c6cff7b161cf09e40ae6af81ff73aa96e47f7f7ce28405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020226f726967696e223a2268747470733a2f2f67733032312e6578616d706c6522', + }, // CONTRACT + ], + executed: + '0x0000000000000000000000009ef150e9bddd63c157e3162b83a565423b2c5a2000000000000000000000000000000000000000000000000000000000000000c300a31c68b6b3a48786a4882079dacb37e27134d3f117f00af247353899562b1b1c0238502d31a96b0881b9d2852c12ed8ad2f69e2b1aebe0d4919b4b648f403a571b000000000000000000000000e9f5f604cbc833a32bbfaf7ddf896fb66f5af0730000000000000000000000000000000000000000000000000000000000000203000000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0edbb4bbe5fc2c7283a1093676b395bd241dee77019cec81423615e1d9bd552ff4bfd5621ab5dd4f0e92cba541f4ab8e250c6393fddfb8690c514f5b3d26d15f400000000000000000000000000000000000000000000000000000000000000256589e4cecfa8f8e42009c6cff7b161cf09e40ae6af81ff73aa96e47f7f7ce28405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020226f726967696e223a2268747470733a2f2f67733032312e6578616d706c65220000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0de03c6c8810e9a4e0cc9b6b4b657f63557005c827b7b44e7458be9ebd692972704368085ec366bd78aa760ee3b01d3ea87d269d668d988f871ae0d94fe3a657b00000000000000000000000000000000000000000000000000000000000000256589e4cecfa8f8e42009c6cff7b161cf09e40ae6af81ff73aa96e47f7f7ce28405000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020226f726967696e223a2268747470733a2f2f67733032312e6578616d706c6522', +} + +const EOA = TWO_OF_TWO.confirmations[0] +const CONTRACT = TWO_OF_TWO.confirmations[1] + +const rebuild = (confirmations: { owner: string; signature: string }[]) => + buildSignatureBytes(confirmations.map(({ owner, signature }) => toSafeSignature(owner, signature))) + +// The offset word of the static part at `slot` (65 bytes each), in bytes. +const offsetAt = (bytes: string, slot: number) => { + const start = 2 + 2 * (65 * slot + 32) + return parseInt(bytes.slice(start, start + 64), 16) +} + +describe('toSafeSignature', () => { + describe('plain signatures stay as they are', () => { + it('an EOA signature', () => { + const signature = toSafeSignature(EOA.owner, EOA.signature) + + expect(signature.isContractSignature).toBe(false) + expect(signature.staticPart()).toBe(EOA.signature) + expect(signature.dynamicPart()).toBe('') + }) + + it('an eth_sign signature (v = 31/32)', () => { + const v = parseInt(EOA.signature.slice(-2), 16) + 4 + const ethSign = `${EOA.signature.slice(0, -2)}${v.toString(16)}` + const signature = toSafeSignature(EOA.owner, ethSign) + + expect(signature.isContractSignature).toBe(false) + expect(signature.staticPart()).toBe(ethSign) + }) + + it('a pre-approved hash (v = 1)', () => { + const approved = `0x${EOA.owner.slice(2).toLowerCase().padStart(64, '0')}${'0'.repeat(64)}01` + const signature = toSafeSignature(EOA.owner, approved) + + expect(signature.isContractSignature).toBe(false) + expect(signature.staticPart()).toBe(approved) + }) + }) + + describe('contract signatures', () => { + it('keep only their data', () => { + const signature = toSafeSignature(CONTRACT.owner, CONTRACT.signature) + + expect(signature.isContractSignature).toBe(true) + // the 65-byte static part and the 32-byte length word are dropped + expect(signature.data).toBe(`0x${CONTRACT.signature.slice(2 + 2 * (65 + 32))}`) + }) + + it('lay out an EOA then a contract signature as the Safe executed them', () => { + const bytes = rebuild(TWO_OF_TWO.confirmations) + + expect(bytes.toLowerCase()).toBe(TWO_OF_TWO.executed.toLowerCase()) + expect(offsetAt(bytes, 1)).toBe(2 * 65) + }) + + it('lay out contract, EOA, contract as the Safe executed them, each pointing past every static part', () => { + const bytes = rebuild(THREE_OF_THREE.confirmations) + const firstDataLength = (THREE_OF_THREE.confirmations[0].signature.length - 2) / 2 - 65 - 32 + + expect(bytes.toLowerCase()).toBe(THREE_OF_THREE.executed.toLowerCase()) + expect(offsetAt(bytes, 0)).toBe(3 * 65) + expect(offsetAt(bytes, 2)).toBe(3 * 65 + 32 + firstDataLength) + }) + + it('do not depend on the order the confirmations come in', () => { + const reversed = [...THREE_OF_THREE.confirmations].reverse() + + expect(rebuild(reversed).toLowerCase()).toBe(THREE_OF_THREE.executed.toLowerCase()) + }) + }) + + describe('a v = 0 signature that does not parse stays plain', () => { + const withOffset = (offset: number) => + `${CONTRACT.signature.slice(0, 2 + 64)}${offset.toString(16).padStart(64, '0')}${CONTRACT.signature.slice(2 + 128)}` + + it.each([ + ['an offset inside the static part', withOffset(32)], + ['an offset past the end', withOffset(10_000)], + ['data shorter than its length', CONTRACT.signature.slice(0, -64)], + ['trailing bytes after the data', `${CONTRACT.signature}${'00'.repeat(32)}`], + ])('%s', (_, malformed) => { + const signature = toSafeSignature(CONTRACT.owner, malformed) + + expect(signature.isContractSignature).toBe(false) + expect(signature.staticPart()).toBe(malformed) + }) + }) +}) diff --git a/packages/utils/src/utils/safeTransaction.ts b/packages/utils/src/utils/safeTransaction.ts index 1b1d8d1..4ad2b2f 100644 --- a/packages/utils/src/utils/safeTransaction.ts +++ b/packages/utils/src/utils/safeTransaction.ts @@ -1,5 +1,5 @@ -import type { SafeTransaction, SafeTransactionData, SafeVersion } from '@safe-global/types-kit' -import { calculateSafeTransactionHash } from '@safe-global/protocol-kit' +import type { SafeSignature, SafeTransaction, SafeTransactionData, SafeVersion } from '@safe-global/types-kit' +import { calculateSafeTransactionHash, EthSafeSignature } from '@safe-global/protocol-kit' import type { Chain } from '@safe-global/store/gateway/AUTO_GENERATED/chains' /** @@ -87,3 +87,41 @@ export const getNestedExecTransactionHashFromInfo = ({ txData, }) } + +const STATIC_PART_HEX_LENGTH = 65 * 2 + +/** + * Turns a confirmation as the Transaction Service returns it back into a Core SDK signature. + * + * An owner that is a contract (a nested Safe, or a passkey signer such as SafeWebAuthnSignerProxy) + * confirms with an EIP-1271 contract signature (v = 0): a 65-byte static part `{owner}{offset}{00}` + * followed by `{length}{signature data}`, stored as a standalone signature, so its offset is 65. + * Added to a SafeTransaction as a plain signature, it is concatenated whole and keeps that offset, + * which points inside the static region once the Safe's threshold is 2 or more, and the Safe + * reverts with GS021. Added as a contract signature, `buildSignatureBytes` lays it out after all + * the static parts with the right offset. + * + * The encoded offset is read, not assumed: it must lie past the 65-byte static part and point at a + * length word whose data runs exactly to the end. Anything else (EOA, eth_sign, pre-approved v = 1, + * or a v = 0 signature that doesn't parse) is returned as a plain signature, as before. + */ +export const toSafeSignature = (signer: string, signature: string): SafeSignature => { + const hex = signature.startsWith('0x') ? signature.slice(2) : signature + const isContractSignature = hex.length > STATIC_PART_HEX_LENGTH && hex.slice(128, 130) === '00' + + if (isContractSignature) { + const offset = Number.parseInt(hex.slice(64, 128), 16) * 2 + const length = Number.parseInt(hex.slice(offset, offset + 64), 16) * 2 + + if ( + Number.isSafeInteger(offset) && + offset >= STATIC_PART_HEX_LENGTH && + Number.isSafeInteger(length) && + offset + 64 + length === hex.length + ) { + return new EthSafeSignature(signer, `0x${hex.slice(offset + 64)}`, true) + } + } + + return new EthSafeSignature(signer, signature) +} -- 2.50.1 (Apple Git-155)