Skip to content

Commit

Permalink
fix: use whitelisted send params for tx initiate
Browse files Browse the repository at this point in the history
- limit params sent to the /tx/initiate API

Ticket: CE-3211
  • Loading branch information
bitgoAaron committed Nov 14, 2023
1 parent 73be786 commit 0cf9f4c
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 13 deletions.
4 changes: 2 additions & 2 deletions modules/bitgo/test/v2/unit/accountConsolidations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ describe('Account Consolidations:', function () {
.reply(200);

const params = { prebuildTx: fixtures.buildAccountConsolidation[0] };
const paramsWithJunk = { ...params, junk: 'junk' };
const paramsAfterCodec = { ...params, type: 'consolidate' };
const paramsWithJunk = { ...params, junk: 'junk', otp: '000000' };
const paramsAfterCodec = { type: 'consolidate', otp: '000000' };

sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(fixtures.signedAccountConsolidationBuilds[0]);
await custodialWallet.sendAccountConsolidation(paramsWithJunk);
Expand Down
20 changes: 19 additions & 1 deletion modules/bitgo/test/v2/unit/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3574,8 +3574,12 @@ describe('V2 Wallet:', function () {
};

const initiateTxPath = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/initiate`;
let initiateTxBody;
const response = nock(bgUrl)
.post(initiateTxPath, _.matches(initiateTxParams)) // use _.matches to do a partial match on request body object instead of strict matching
.post(initiateTxPath, (body) => {
initiateTxBody = body;
return true;
})
.reply(200);

const feeEstimationPath = `/api/v2/${wallet.coin()}/tx/fee?hop=true&recipient=${address}&amount=10000000000000000&type=Export`;
Expand All @@ -3590,6 +3594,20 @@ describe('V2 Wallet:', function () {
console.log(e);
// test is successful if nock is consumed, HMAC errors expected
}
_.isMatch(initiateTxBody, {
hopParams: {
gasPriceMax: 7187500000,
},
type: 'Export',
gasLimit: 500000,
recipients: [
{
amount: '10000000000000000',
address,
},
],
}).should.be.true();

response.isDone().should.be.true();
});
});
Expand Down
2 changes: 1 addition & 1 deletion modules/sdk-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
},
"dependencies": {
"@bitgo/bls-dkg": "^1.3.1",
"@bitgo/public-types": "1.1.2",
"@bitgo/public-types": "1.2.1",
"@bitgo/sdk-lib-mpc": "^8.18.0",
"@bitgo/statics": "^32.0.0",
"@bitgo/utxo-lib": "^9.19.0",
Expand Down
24 changes: 19 additions & 5 deletions modules/sdk-core/src/bitgo/wallet/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ const debug = require('debug')('bitgo:v2:wallet');

type ManageUnspents = 'consolidate' | 'fanout';

const whitelistedSendParams = TxSendBody.type.types.flatMap((t) => Object.keys(t.props));

export enum ManageUnspentsOptions {
BUILD_ONLY,
BUILD_SIGN_SEND,
Expand Down Expand Up @@ -2108,7 +2110,7 @@ export class Wallet implements IWallet {
if (this._wallet.type === 'custodial') {
const extraParams = await this.baseCoin.getExtraPrebuildParams(Object.assign(params, { wallet: this }));
Object.assign(selectParams, extraParams);
return await this.bitgo.post(this.url('/tx/initiate')).send(selectParams).result();
return this.initiateTransaction(selectParams);
}

const halfSignedTransaction = await this.prebuildAndSignTransaction(params);
Expand Down Expand Up @@ -2452,7 +2454,7 @@ export class Wallet implements IWallet {

if (this._wallet.type === 'custodial' && this._wallet.multisigType !== 'tss') {
params.type = 'consolidate';
return await this.bitgo.post(this.url('/tx/initiate')).send(BuildParams.encode(params)).result();
return this.initiateTransaction(params as TxSendBody);
}

// one of a set of consolidation transactions
Expand Down Expand Up @@ -2608,8 +2610,7 @@ export class Wallet implements IWallet {
const signedPrebuild = await this.prebuildAndSignTransaction(params);
return await this.submitTransaction(signedPrebuild);
case 'custodial':
const url = this.baseCoin.url('/wallet/' + this.id() + '/tx/initiate');
return await this.bitgo.post(url).send(params.prebuildTx.buildParams).result();
return this.initiateTransaction(params.prebuildTx.buildParams);
}
}
}
Expand Down Expand Up @@ -3159,12 +3160,25 @@ export class Wallet implements IWallet {
// extract the whitelisted params from the top level, in case
// other invalid params are present that would fail encoding
// and fall back to the body params
const whitelistedParams = _.pick(params, Object.keys(TxSendBody.type.props));
const whitelistedParams = _.pick(params, whitelistedSendParams);
return postWithCodec(
this.bitgo,
this.baseCoin.url('/wallet/' + this.id() + '/tx/send'),
TxSendBody,
whitelistedParams
).result();
}

private initiateTransaction(params: TxSendBody) {
// extract the whitelisted params from the top level, in case
// other invalid params are present that would fail encoding
// and fall back to the body params
const whitelistedParams = _.pick(params, whitelistedSendParams);
return postWithCodec(
this.bitgo,
this.baseCoin.url('/wallet/' + this.id() + '/tx/initiate'),
TxSendBody,
whitelistedParams
).result();
}
}
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1030,10 +1030,10 @@
"@babel/helper-validator-identifier" "^7.22.20"
to-fast-properties "^2.0.0"

"@bitgo/public-types@1.1.2":
version "1.1.2"
resolved "https://registry.yarnpkg.com/@bitgo/public-types/-/public-types-1.1.2.tgz#125ff179cb1b6df3a2d7bf0f7cc430cab6e3990b"
integrity sha512-Z/mg63Pj8VFXEV9/J6F8w0L2XAPzzul7mT7mF7K2J0FtXNyHyJT5JBS1cRLc6lQ7bv/egG0YLTVZLyf3QUZgEg==
"@bitgo/public-types@1.2.1":
version "1.2.1"
resolved "https://registry.yarnpkg.com/@bitgo/public-types/-/public-types-1.2.1.tgz#45028dd7ba89103d3fabde295cf33e1b733cc9bc"
integrity sha512-smOGrsMtxIJZxPP0CN6FMSP+k0NMxYM1dFbvhAxEZ9rAyMBGknVrCGUtZsman9a1ixE30nBv0SCjHB/6vn1bxA==
dependencies:
io-ts "^2.2.20"

Expand Down

0 comments on commit 0cf9f4c

Please sign in to comment.