Skip to content

Commit a4b9db6

Browse files
mullapudipruthvikmullapudipruthvik
authored
fix(sdk-core): add validation for unsigned txHex
2 parents f459d74 + eff30e3 commit a4b9db6

File tree

8 files changed

+135
-11
lines changed

8 files changed

+135
-11
lines changed

‎modules/abstract-cosmos/src/cosmosCoin.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,6 @@ export class CosmosCoin<CustomMessage = never> extends BaseCoin {
360360
}
361361
const transaction = await this.getBuilder().from(rawTx).build();
362362
const explainedTx = transaction.explainTransaction();
363-
364363
if (txParams.recipients && txParams.recipients.length > 0) {
365364
const filteredRecipients = txParams.recipients?.map((recipient) => _.pick(recipient, ['address', 'amount']));
366365
const filteredOutputs = explainedTx.outputs.map((output) => _.pick(output, ['address', 'amount']));

‎modules/abstract-eth/src/abstractEthLikeNewCoins.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,10 @@ import {
6666
calculateForwarderV1Address,
6767
ERC1155TransferBuilder,
6868
ERC721TransferBuilder,
69+
getBufferedByteCode,
6970
getCommon,
7071
getProxyInitcode,
72+
getRawDecoded,
7173
getToken,
7274
KeyPair as KeyPairLib,
7375
TransactionBuilder,
@@ -2523,7 +2525,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
25232525
* @param {Wallet} params.wallet - Wallet object to obtain keys to verify against
25242526
* @returns {boolean}
25252527
*/
2526-
verifyTssTransaction(params: VerifyEthTransactionOptions): boolean {
2528+
async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
25272529
const { txParams, txPrebuild, wallet } = params;
25282530
if (
25292531
!txParams?.recipients &&
@@ -2540,6 +2542,39 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
25402542
if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) {
25412543
throw new Error(`tx cannot be both a batch and hop transaction`);
25422544
}
2545+
2546+
if (txParams.type && ['transfer'].includes(txParams.type)) {
2547+
if (txParams.recipients && txParams.recipients.length === 1) {
2548+
const recipients = txParams.recipients;
2549+
const expectedAmount = recipients[0].amount.toString();
2550+
const expectedDestination = recipients[0].address;
2551+
2552+
const txBuilder = this.getTransactionBuilder();
2553+
txBuilder.from(txPrebuild.txHex);
2554+
const tx = await txBuilder.build();
2555+
const txJson = tx.toJson();
2556+
if (txJson.data === '0x') {
2557+
if (expectedAmount !== txJson.value) {
2558+
throw new Error('the transaction amount in txPrebuild does not match the value given by client');
2559+
}
2560+
if (expectedDestination !== txJson.to) {
2561+
throw new Error('destination address does not match with the recipient address');
2562+
}
2563+
} else if (txJson.data.startsWith('0xa9059cbb')) {
2564+
const [recipientAddress, amount] = getRawDecoded(
2565+
['address', 'uint256'],
2566+
getBufferedByteCode('0xa9059cbb', txJson.data)
2567+
);
2568+
if (expectedAmount !== amount.toString()) {
2569+
throw new Error('the transaction amount in txPrebuild does not match the value given by client');
2570+
}
2571+
if (expectedDestination !== addHexPrefix(recipientAddress.toString())) {
2572+
throw new Error('destination address does not match with the recipient address');
2573+
}
2574+
}
2575+
}
2576+
}
2577+
25432578
return true;
25442579
}
25452580

‎modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts‎

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -490,8 +490,11 @@ describe('TSS Ecdsa Utils:', async function () {
490490
transactions: [
491491
{
492492
unsignedTx: {
493-
serializedTxHex: 'TOO MANY SECRETS',
494-
signableHex: 'TOO MANY SECRETS',
493+
// hteth txid: 0xc5a7bfe6b13ceae563da0f9feaa9c4ad1c101a15366a2a488828a5dd27cb9da3
494+
serializedTxHex:
495+
'02f38242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0808080',
496+
signableHex:
497+
'02f08242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0',
495498
derivationPath: '', // Needs this when key derivation is supported
496499
},
497500
state: 'pendingSignature',
@@ -500,8 +503,11 @@ describe('TSS Ecdsa Utils:', async function () {
500503
],
501504
unsignedTxs: [
502505
{
503-
serializedTxHex: 'TOO MANY SECRETS',
504-
signableHex: 'TOO MANY SECRETS',
506+
// hteth txid: 0xc5a7bfe6b13ceae563da0f9feaa9c4ad1c101a15366a2a488828a5dd27cb9da3
507+
serializedTxHex:
508+
'02f38242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0808080',
509+
signableHex:
510+
'02f38242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0808080',
505511
derivationPath: '', // Needs this when key derivation is supported
506512
},
507513
],
@@ -764,6 +770,68 @@ describe('TSS Ecdsa Utils:', async function () {
764770
userGpgActual.should.startWith('-----BEGIN PGP PUBLIC KEY BLOCK-----');
765771
});
766772

773+
it('signTxRequest should fail with wrong recipient', async function () {
774+
await setupSignTxRequestNocks(true, userSignShare, aShare, dShare, enterpriseData);
775+
await tssUtils
776+
.signTxRequest({
777+
txRequest: txRequestId,
778+
prv: JSON.stringify({
779+
pShare: userKeyShare.pShare,
780+
bitgoNShare: bitgoKeyShare.nShares[1],
781+
backupNShare: backupKeyShare.nShares[1],
782+
}),
783+
reqId,
784+
txParams: { recipients: [{ address: '0x1234', amount: '100000000000000' }], type: 'transfer' },
785+
})
786+
.should.be.rejectedWith('destination address does not match with the recipient address');
787+
});
788+
789+
it('signTxRequest should fail with incorrect value', async function () {
790+
await setupSignTxRequestNocks(true, userSignShare, aShare, dShare, enterpriseData);
791+
await tssUtils
792+
.signTxRequest({
793+
txRequest: txRequestId,
794+
prv: JSON.stringify({
795+
pShare: userKeyShare.pShare,
796+
bitgoNShare: bitgoKeyShare.nShares[1],
797+
backupNShare: backupKeyShare.nShares[1],
798+
}),
799+
reqId,
800+
txParams: {
801+
recipients: [{ address: '0xa1cfb9d51c0af191ff21c5f0f01723e056f7dc12', amount: '1' }],
802+
type: 'transfer',
803+
},
804+
})
805+
.should.be.rejectedWith('the transaction amount in txPrebuild does not match the value given by client');
806+
});
807+
808+
it('signTxRequest should fail with incorrect value for token txn', async function () {
809+
const signableHex =
810+
'02f86d8242681083122c9e83122cae8301e04994ebe8b46a42f05072b723b00013ff822b2af1b5cb80b844a9059cbb0000000000000000000000002b0d6cb2f8c388757f4d7ad857fccab18290dbc900000000000000000000000000000000000000000000000000000000000186a0c0';
811+
const serializedTxHex =
812+
'02f8708242681083122c9e83122cae8301e04994ebe8b46a42f05072b723b00013ff822b2af1b5cb80b844a9059cbb0000000000000000000000002b0d6cb2f8c388757f4d7ad857fccab18290dbc900000000000000000000000000000000000000000000000000000000000186a0c0808080';
813+
await setupSignTxRequestNocks(true, userSignShare, aShare, dShare, enterpriseData, {
814+
signableHex,
815+
serializedTxHex,
816+
apiVersion: 'full',
817+
});
818+
await tssUtils
819+
.signTxRequest({
820+
txRequest: txRequestId,
821+
prv: JSON.stringify({
822+
pShare: userKeyShare.pShare,
823+
bitgoNShare: bitgoKeyShare.nShares[1],
824+
backupNShare: backupKeyShare.nShares[1],
825+
}),
826+
reqId,
827+
txParams: {
828+
recipients: [{ address: '0x2b0d6cb2f8c388757f4d7ad857fccab18290dbc9', amount: '707' }],
829+
type: 'transfer',
830+
},
831+
})
832+
.should.be.rejectedWith('the transaction amount in txPrebuild does not match the value given by client');
833+
});
834+
767835
it('getOfflineSignerPaillierModulus should succeed', async function () {
768836
const paillierModulus = tssUtils.getOfflineSignerPaillierModulus({
769837
prv: JSON.stringify({
@@ -932,7 +1000,12 @@ describe('TSS Ecdsa Utils:', async function () {
9321000
userSignShare: ECDSA.SignShareRT,
9331001
aShare: ECDSA.AShare,
9341002
dShare: ECDSA.DShare,
935-
enterpriseData?: EnterpriseData
1003+
enterpriseData: EnterpriseData,
1004+
{
1005+
signableHex,
1006+
serializedTxHex,
1007+
apiVersion,
1008+
}: { signableHex?: string; serializedTxHex?: string; apiVersion?: 'full' | 'lite' } = {}
9361009
) {
9371010
if (enterpriseData) {
9381011
await nockGetEnterprise({ enterpriseId: enterpriseData.id, response: enterpriseData, times: 1 });
@@ -947,12 +1020,13 @@ describe('TSS Ecdsa Utils:', async function () {
9471020
{
9481021
...txRequest,
9491022
unsignedTx: {
950-
signableHex: txRequest.unsignedTxs[0].signableHex,
951-
serializedTxHex: txRequest.unsignedTxs[0].serializedTxHex,
1023+
signableHex: signableHex ?? txRequest.unsignedTxs[0].signableHex,
1024+
serializedTxHex: serializedTxHex ?? txRequest.unsignedTxs[0].serializedTxHex,
9521025
derivationPath,
9531026
},
9541027
},
9551028
],
1029+
apiVersion: apiVersion,
9561030
},
9571031
],
9581032
};
@@ -976,6 +1050,7 @@ describe('TSS Ecdsa Utils:', async function () {
9761050
},
9771051
},
9781052
],
1053+
apiVersion: apiVersion,
9791054
},
9801055
],
9811056
};
@@ -996,6 +1071,7 @@ describe('TSS Ecdsa Utils:', async function () {
9961071
},
9971072
},
9981073
],
1074+
apiVersion: apiVersion,
9991075
},
10001076
],
10011077
};

‎modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ export interface VerificationOptions {
160160
export interface VerifyTransactionOptions {
161161
txPrebuild: TransactionPrebuild;
162162
txParams: TransactionParams;
163-
wallet: Wallet;
163+
wallet: IWallet;
164164
verification?: VerificationOptions;
165165
reqId?: IRequestTracer;
166166
walletType?: 'onchain' | 'tss';

‎modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Key, SerializedKeyPair } from 'openpgp';
22
import { IRequestTracer } from '../../../api';
3-
import { KeychainsTriplet, ParsedTransaction } from '../../baseCoin';
3+
import { KeychainsTriplet, ParsedTransaction, TransactionParams } from '../../baseCoin';
44
import { ApiKeyShare, Keychain } from '../../keychain';
55
import { ApiVersion, Memo, WalletType } from '../../wallet';
66
import { EDDSA, GShare, Signature, SignShare } from '../../../account-lib/mpc/tss';
@@ -431,6 +431,7 @@ export type TSSParams = {
431431
txRequest: string | TxRequest; // can be either a string or TxRequest
432432
reqId: IRequestTracer;
433433
apiVersion?: ApiVersion;
434+
txParams?: TransactionParams;
434435
};
435436

436437
export type TSSParamsForMessage = TSSParams & {

‎modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,12 @@ export class EcdsaUtils extends BaseEcdsaUtils {
744744
assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
745745
const unsignedTx =
746746
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
747+
await this.baseCoin.verifyTransaction({
748+
txPrebuild: { txHex: unsignedTx.signableHex },
749+
txParams: params.txParams || { recipients: [] },
750+
wallet: this.wallet,
751+
walletType: this.wallet.multisigType(),
752+
});
747753
signablePayload = Buffer.from(unsignedTx.signableHex, 'hex');
748754
derivationPath = unsignedTx.derivationPath;
749755
} else if (requestType === RequestType.message) {

‎modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,12 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
731731
assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
732732
const unsignedTx =
733733
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
734+
await this.baseCoin.verifyTransaction({
735+
txPrebuild: { txHex: unsignedTx.signableHex },
736+
txParams: params.txParams || { recipients: [] },
737+
wallet: this.wallet,
738+
walletType: this.wallet.multisigType(),
739+
});
734740
txOrMessageToSign = unsignedTx.signableHex;
735741
derivationPath = unsignedTx.derivationPath;
736742
bufferContent = Buffer.from(txOrMessageToSign, 'hex');

‎modules/sdk-core/src/bitgo/wallet/wallet.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3468,6 +3468,7 @@ export class Wallet implements IWallet {
34683468
try {
34693469
return await this.tssUtils!.signTxRequest({
34703470
txRequest: params.txPrebuild.txRequestId,
3471+
txParams: params.txPrebuild.buildParams,
34713472
prv: params.prv,
34723473
reqId: params.reqId || new RequestTracer(),
34733474
apiVersion: params.apiVersion,

0 commit comments

Comments
 (0)