Skip to content

Commit 843b463

Browse files
fix(abstract-eth): verify inner batch calldata recipients
Decode the inner batch(address[],uint256[]) calldata embedded in txPrebuild.txHex and compare each (address, amount) pair to the user-supplied recipients. Without this check, the verifier validated only the outer batcher contract address and the total amount, so a compromised platform could swap inner recipients while preserving the outer wrapper checks and redirect batched payouts. Fails closed when txHex is missing, the outer selector is not sendMultiSig, or the inner selector is not batch(address[],uint256[]). Ticket: CGD-1319
1 parent 4863c85 commit 843b463

6 files changed

Lines changed: 420 additions & 8 deletions

File tree

modules/abstract-eth/src/abstractEthLikeNewCoins.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
MPCTx,
2424
MPCTxs,
2525
ParsedTransaction,
26+
ITransactionRecipient,
2627
ParseTransactionOptions,
2728
PrebuildTransactionResult,
2829
PresignTransactionOptions as BasePresignTransactionOptions,
@@ -71,8 +72,11 @@ import secp256k1 from 'secp256k1';
7172
import { AbstractEthLikeCoin } from './abstractEthLikeCoin';
7273
import { EthLikeToken } from './ethLikeToken';
7374
import {
75+
batchMethodId,
7476
calculateForwarderV1Address,
7577
coinFamiliesWithL1Fees,
78+
decodeBatchTransferData,
79+
decodeNativeTransferData,
7680
decodeTransferData,
7781
ERC1155TransferBuilder,
7882
ERC721TransferBuilder,
@@ -83,6 +87,7 @@ import {
8387
getRawDecoded,
8488
getToken,
8589
KeyPair as KeyPairLib,
90+
sendMultisigMethodId,
8691
TransactionBuilder,
8792
TransferBuilder,
8893
} from './lib';
@@ -1633,6 +1638,87 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
16331638
};
16341639
}
16351640

1641+
/**
1642+
* Verify that the inner batch(address[],uint256[]) calldata embedded in txPrebuild.txHex matches
1643+
* the user-supplied recipients. Throws via throwRecipientMismatch if any pair differs or if the
1644+
* calldata cannot be decoded. The verifier fails closed: missing txHex, an unexpected outer
1645+
* selector, or an unexpected inner selector all reject.
1646+
*/
1647+
private async verifyBatchInnerRecipients(
1648+
txPrebuild: TransactionPrebuild,
1649+
recipients: ITransactionRecipient[],
1650+
throwRecipientMismatch: (message: string, mismatchedRecipients: Recipient[]) => Promise<never>
1651+
): Promise<void> {
1652+
if (!txPrebuild.txHex) {
1653+
await throwRecipientMismatch('batch txPrebuild missing txHex required for inner calldata verification', []);
1654+
return;
1655+
}
1656+
1657+
let outerCalldata: string;
1658+
try {
1659+
const txBuffer = optionalDeps.ethUtil.toBuffer(txPrebuild.txHex);
1660+
const decodedTx = optionalDeps.EthTx.TransactionFactory.fromSerializedData(txBuffer);
1661+
outerCalldata = optionalDeps.ethUtil.bufferToHex(decodedTx.data);
1662+
} catch (e) {
1663+
await throwRecipientMismatch(`failed to parse batch txHex: ${e instanceof Error ? e.message : String(e)}`, []);
1664+
return;
1665+
}
1666+
1667+
if (!outerCalldata.toLowerCase().startsWith(sendMultisigMethodId)) {
1668+
await throwRecipientMismatch('batch txPrebuild outer call is not sendMultiSig', []);
1669+
return;
1670+
}
1671+
1672+
let innerBatchData: string;
1673+
try {
1674+
innerBatchData = decodeNativeTransferData(outerCalldata).data;
1675+
} catch (e) {
1676+
await throwRecipientMismatch(
1677+
`failed to decode outer sendMultiSig wrapper: ${e instanceof Error ? e.message : String(e)}`,
1678+
[]
1679+
);
1680+
return;
1681+
}
1682+
1683+
if (!innerBatchData || !innerBatchData.toLowerCase().startsWith(batchMethodId)) {
1684+
await throwRecipientMismatch('batch txPrebuild inner method selector is not batch(address[],uint256[])', []);
1685+
return;
1686+
}
1687+
1688+
let decoded;
1689+
try {
1690+
decoded = decodeBatchTransferData(innerBatchData);
1691+
} catch (e) {
1692+
await throwRecipientMismatch(
1693+
`failed to decode inner batch calldata: ${e instanceof Error ? e.message : String(e)}`,
1694+
[]
1695+
);
1696+
return;
1697+
}
1698+
1699+
if (decoded.recipients.length !== recipients.length) {
1700+
await throwRecipientMismatch(
1701+
`batch txPrebuild inner recipient count (${decoded.recipients.length}) does not match txParams (${recipients.length})`,
1702+
decoded.recipients
1703+
);
1704+
return;
1705+
}
1706+
1707+
for (let i = 0; i < recipients.length; i++) {
1708+
const expected = recipients[i];
1709+
const actual = decoded.recipients[i];
1710+
// Skip address comparison for non-hex inputs (e.g. unresolved ENS); mirrors normal-tx path.
1711+
if (this.isETHAddress(expected.address) && expected.address.toLowerCase() !== actual.address.toLowerCase()) {
1712+
await throwRecipientMismatch('batch txPrebuild inner recipient address does not match txParams', [actual]);
1713+
return;
1714+
}
1715+
if (!new BigNumber(expected.amount).isEqualTo(actual.amount)) {
1716+
await throwRecipientMismatch('batch txPrebuild inner recipient amount does not match txParams', [actual]);
1717+
return;
1718+
}
1719+
}
1720+
}
1721+
16361722
/**
16371723
* Extract recipients from transaction hex
16381724
* @param txHex - The transaction hex string
@@ -3325,6 +3411,13 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
33253411
{ address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() },
33263412
]);
33273413
}
3414+
3415+
// Decode the inner batch(address[],uint256[]) calldata and verify each (address, amount) pair
3416+
// matches user intent. Without this, a compromised platform could swap inner recipients while
3417+
// preserving the outer total amount and batcher-address checks.
3418+
if (!txParams.tokenName) {
3419+
await this.verifyBatchInnerRecipients(txPrebuild, recipients, throwRecipientMismatch);
3420+
}
33283421
} else {
33293422
// Check recipient address and amount for normal transaction
33303423
if (recipients.length !== 1) {

modules/abstract-eth/src/lib/iface.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,15 @@ export interface NativeTransferData extends TransferData {
144144
data: string;
145145
}
146146

147+
export interface BatchTransferRecipient {
148+
address: string;
149+
amount: string;
150+
}
151+
152+
export interface BatchTransferData {
153+
recipients: BatchTransferRecipient[];
154+
}
155+
147156
export interface WalletInitializationData {
148157
salt?: string;
149158
owners: string[];

modules/abstract-eth/src/lib/utils.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
} from '@bitgo/sdk-core';
3232

3333
import {
34+
BatchTransferData,
3435
ERC1155TransferData,
3536
ERC721TransferData,
3637
FlushTokensData,
@@ -65,6 +66,8 @@ import {
6566
flushERC1155ForwarderTokensMethodIdV4,
6667
flushERC1155TokensTypes,
6768
flushERC1155TokensTypesv4,
69+
batchMethodId,
70+
batchMethodTypes,
6871
sendMultisigMethodId,
6972
sendMultisigTokenMethodId,
7073
sendMultiSigTokenTypes,
@@ -459,6 +462,34 @@ export function decodeTransferData(data: string, isFirstSigner?: boolean): Trans
459462
}
460463
}
461464

465+
/**
466+
* Decode the inner batch(address[],uint256[]) calldata produced for batcher contract sends.
467+
* The data is the inner payload nested inside a sendMultiSig wrapper, not a full transaction.
468+
*
469+
* @param data Hex string starting with the batch method selector
470+
* @returns Decoded recipients and amounts in the order they appear in the calldata
471+
*/
472+
export function decodeBatchTransferData(data: string): BatchTransferData {
473+
if (!data.toLowerCase().startsWith(batchMethodId)) {
474+
throw new BuildTransactionError(`Invalid batch transfer bytecode: ${data}`);
475+
}
476+
const [addresses, amounts] = getRawDecoded(batchMethodTypes, getBufferedByteCode(batchMethodId, data));
477+
if (!Array.isArray(addresses) || !Array.isArray(amounts)) {
478+
throw new BuildTransactionError(`Invalid batch transfer bytecode: ${data}`);
479+
}
480+
if (addresses.length !== amounts.length) {
481+
throw new BuildTransactionError(
482+
`Mismatched batch address/amount array lengths: ${addresses.length} vs ${amounts.length}`
483+
);
484+
}
485+
return {
486+
recipients: addresses.map((addr, i) => ({
487+
address: addHexPrefix(addr as string),
488+
amount: new BigNumber(bufferToHex(amounts[i] as Buffer)).toFixed(),
489+
})),
490+
};
491+
}
492+
462493
/**
463494
* Decode the given ABI-encoded transfer data for the sendMultisigToken function and return parsed fields
464495
*

modules/abstract-eth/src/lib/walletUtil.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
export const sendMultisigMethodId = '0x39125215';
22
export const sendMultisigTokenMethodId = '0x0dcd7a6c';
3+
// Selector for batch(address[],uint256[]) used by batcher contract sends.
4+
export const batchMethodId = '0xc00c4e9e';
35
export const v1CreateForwarderMethodId = '0xfb90b320';
46
export const v4CreateForwarderMethodId = '0x13b2f75c';
57
export const v1WalletInitializationFirstBytes = '0x60806040';
@@ -38,6 +40,9 @@ export const sendMultiSigTypesFirstSigner = ['string', 'address', 'uint', 'bytes
3840
export const sendMultiSigTokenTypes = ['address', 'uint', 'address', 'uint', 'uint', 'bytes'];
3941
export const sendMultiSigTokenTypesFirstSigner = ['string', 'address', 'uint', 'address', 'uint', 'uint'];
4042

43+
export const batchMethodName = 'batch';
44+
export const batchMethodTypes = ['address[]', 'uint256[]'];
45+
4146
export const ERC721SafeTransferTypes = ['address', 'address', 'uint256', 'bytes'];
4247
export const ERC721TransferFromTypes = ['address', 'address', 'uint256'];
4348

modules/abstract-eth/test/unit/utils.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
import should from 'should';
2+
import EthereumAbi from 'ethereumjs-abi';
23
import {
34
flushERC721TokensData,
45
flushERC1155TokensData,
56
decodeFlushERC721TokensData,
67
decodeFlushERC1155TokensData,
8+
decodeBatchTransferData,
79
} from '../../src/lib/utils';
810
import { ERC721TransferBuilder } from '../../src/lib/transferBuilders/transferBuilderERC721';
9-
import { ERC721TransferFromMethodId, ERC721SafeTransferTypeMethodId } from '../../src/lib/walletUtil';
11+
import {
12+
ERC721TransferFromMethodId,
13+
ERC721SafeTransferTypeMethodId,
14+
batchMethodId,
15+
batchMethodName,
16+
batchMethodTypes,
17+
} from '../../src/lib/walletUtil';
1018

1119
describe('Abstract ETH Utils', () => {
1220
describe('ERC721 Flush Functions', () => {
@@ -268,4 +276,42 @@ describe('Abstract ETH Utils', () => {
268276
decoded1155.tokenAddress.toLowerCase().should.equal(tokenAddressChecksum.toLowerCase());
269277
});
270278
});
279+
280+
describe('decodeBatchTransferData', () => {
281+
const address1 = '0x1111111111111111111111111111111111111111';
282+
const address2 = '0x2222222222222222222222222222222222222222';
283+
const encodeBatch = (addresses: string[], amounts: string[]): string => {
284+
const selector = EthereumAbi.methodID(batchMethodName, batchMethodTypes);
285+
const args = EthereumAbi.rawEncode(batchMethodTypes, [addresses, amounts]);
286+
return '0x' + Buffer.concat([selector, args]).toString('hex');
287+
};
288+
289+
it('hardcoded batchMethodId matches the runtime-computed selector', () => {
290+
const computed = '0x' + EthereumAbi.methodID(batchMethodName, batchMethodTypes).toString('hex');
291+
computed.should.equal(batchMethodId);
292+
});
293+
294+
it('round-trips encode/decode for multiple recipients', () => {
295+
const data = encodeBatch([address1, address2], ['1000', '2500']);
296+
const decoded = decodeBatchTransferData(data);
297+
298+
decoded.recipients.length.should.equal(2);
299+
decoded.recipients[0].address.toLowerCase().should.equal(address1);
300+
decoded.recipients[0].amount.should.equal('1000');
301+
decoded.recipients[1].address.toLowerCase().should.equal(address2);
302+
decoded.recipients[1].amount.should.equal('2500');
303+
});
304+
305+
it('throws on wrong method selector', () => {
306+
should.throws(() => decodeBatchTransferData('0xdeadbeef00000000'), /Invalid batch transfer bytecode/);
307+
});
308+
309+
it('throws when the encoded address[] and uint256[] arrays have different lengths', () => {
310+
// Encode the batch payload directly with mismatched array lengths.
311+
const payload = EthereumAbi.rawEncode(batchMethodTypes, [[address1, address2], ['1000']]);
312+
const tampered =
313+
'0x' + Buffer.concat([EthereumAbi.methodID(batchMethodName, batchMethodTypes), payload]).toString('hex');
314+
should.throws(() => decodeBatchTransferData(tampered), /Mismatched batch address\/amount array lengths/);
315+
});
316+
});
271317
});

0 commit comments

Comments
 (0)