diff --git a/modules/sdk-coin-algo/src/algo.ts b/modules/sdk-coin-algo/src/algo.ts index 92d50b77396..7b51d89cd9f 100644 --- a/modules/sdk-coin-algo/src/algo.ts +++ b/modules/sdk-coin-algo/src/algo.ts @@ -29,6 +29,7 @@ import { TransactionExplanation, TransactionRecipient, TransactionType, + InvalidTransactionError, UnexpectedAddressError, VerifyAddressOptions, VerifyTransactionOptions, @@ -74,6 +75,7 @@ export interface VerifyAlgoAddressOptions extends VerifyAddressOptions { export interface AlgoTransactionExplanation extends TransactionExplanation { memo?: string; type?: string | number; + reKeyTo?: string; voteKey?: string; selectionKey?: string; voteFirst?: number; @@ -91,6 +93,13 @@ export interface TransactionOperation { export interface SignTransactionOptions extends BaseSignTransactionOptions { txPrebuild: TransactionPrebuild; prv: string; + /** + * Explicit opt-in required to sign a transaction that sets `reKeyTo`: the exact rekey + * address that was shown to and approved by the operator. A rekey hands spending + * authority of the account to another address, so signing throws if the decoded + * transaction rekeys to any other address. + */ + allowReKeyTo?: string; } export interface TransactionPrebuild { @@ -357,7 +366,7 @@ export class Algo extends BaseCoin { * @param params */ async explainTransaction(params: ExplainTransactionOptions): Promise { - const txHex = params.txHex || (params.halfSigned && params.halfSigned.txHex); + const txHex = (params.halfSigned && params.halfSigned.txHex) || params.txHex; if (!txHex || !params.feeInfo) { throw new Error('missing explain tx parameters'); } @@ -412,6 +421,13 @@ export class Algo extends BaseCoin { operations, }; + // A rekey silently reassigns spending authority of the account, so it must never + // be hidden from the explanation shown for approval. + if (txJson.reKeyTo) { + displayOrder.push('reKeyTo'); + explanationResult.reKeyTo = txJson.reKeyTo; + } + if (txJson.tokenId) { explanationResult.tokenId = txJson.tokenId; } @@ -432,7 +448,7 @@ export class Algo extends BaseCoin { 'voteKeyDilution', ]; - return { + const explanationResult: AlgoTransactionExplanation = { displayOrder, id: txJson.id, outputAmount: '0', @@ -448,6 +464,14 @@ export class Algo extends BaseCoin { voteLast: txJson.voteLast, voteKeyDilution: txJson.voteKeyDilution, }; + + // A keyreg transaction can also carry a rekey; surface it like in the send case. + if (txJson.reKeyTo) { + displayOrder.push('reKeyTo'); + explanationResult.reKeyTo = txJson.reKeyTo; + } + + return explanationResult; } } @@ -550,6 +574,20 @@ export class Algo extends BaseCoin { */ async signTransaction(params: SignTransactionOptions): Promise { const { txHex, signers, prv, isHalfSigned, numberSigners } = this.verifySignTransactionParams(params); + // A rekey reassigns spending authority of the account; refuse to sign it unless the + // caller passes the exact rekey address the operator approved. A bare opt-in would + // also approve a different rekey target hidden in the bytes being signed. + // Note: this guards Algo.signTransaction only; other signing entry points (e.g. + // callers driving TransactionBuilder.sign directly) are not covered. + const { txn } = Utils.decodeAlgoTxn(txHex); + if (txn.reKeyTo) { + const reKeyAddress = algosdk.encodeAddress(txn.reKeyTo.publicKey); + if (params.allowReKeyTo !== reKeyAddress) { + throw new InvalidTransactionError( + `transaction rekeys the account to ${reKeyAddress}: refusing to sign unless the rekey is confirmed intentional (pass allowReKeyTo with the approved rekey address)` + ); + } + } const factory = this.getBuilder(); const txBuilder = factory.from(txHex); txBuilder.numberOfRequiredSigners(numberSigners); diff --git a/modules/sdk-coin-algo/src/lib/ifaces.ts b/modules/sdk-coin-algo/src/lib/ifaces.ts index 6d3e15078cf..d1a10edd5c8 100644 --- a/modules/sdk-coin-algo/src/lib/ifaces.ts +++ b/modules/sdk-coin-algo/src/lib/ifaces.ts @@ -20,6 +20,7 @@ export interface TxData { genesisID: string; genesisHash: string | Buffer; closeRemainderTo?: string; + reKeyTo?: string; nonParticipation?: boolean; txType?: string; tokenName?: string; diff --git a/modules/sdk-coin-algo/src/lib/keyRegistrationBuilder.ts b/modules/sdk-coin-algo/src/lib/keyRegistrationBuilder.ts index a8be069a164..39e031ea5f7 100644 --- a/modules/sdk-coin-algo/src/lib/keyRegistrationBuilder.ts +++ b/modules/sdk-coin-algo/src/lib/keyRegistrationBuilder.ts @@ -143,7 +143,8 @@ export class KeyRegistrationBuilder extends TransactionBuilder { undefined, // voteFirst param undefined, // voteLast param undefined, // voteKeyDilution param - this.suggestedParams + this.suggestedParams, + this._reKeyTo ); } @@ -157,7 +158,7 @@ export class KeyRegistrationBuilder extends TransactionBuilder { this._voteLast, this._voteKeyDilution, this.suggestedParams, - undefined, // reKeyTo param + this._reKeyTo, undefined, // nonParticipation param this._stateProofKey ); diff --git a/modules/sdk-coin-algo/src/lib/transaction.ts b/modules/sdk-coin-algo/src/lib/transaction.ts index 376494125a4..a267c10f4d0 100644 --- a/modules/sdk-coin-algo/src/lib/transaction.ts +++ b/modules/sdk-coin-algo/src/lib/transaction.ts @@ -252,6 +252,9 @@ export class Transaction extends BaseTransaction { if (this._algoTransaction.closeRemainderTo) { result.closeRemainderTo = algosdk.encodeAddress(this._algoTransaction.closeRemainderTo.publicKey); } + if (this._algoTransaction.reKeyTo) { + result.reKeyTo = algosdk.encodeAddress(this._algoTransaction.reKeyTo.publicKey); + } if (this.type === TransactionType.Send) { result.to = algosdk.encodeAddress(this._algoTransaction.to.publicKey); result.amount = this._algoTransaction.amount.toString(); diff --git a/modules/sdk-coin-algo/test/unit/algo.ts b/modules/sdk-coin-algo/test/unit/algo.ts index 74f01eca144..672df571d4c 100644 --- a/modules/sdk-coin-algo/test/unit/algo.ts +++ b/modules/sdk-coin-algo/test/unit/algo.ts @@ -233,6 +233,52 @@ describe('ALGO:', function () { explain.outputs[0].amount.should.equal(amount.toString()); explain.outputs[0].address.should.equal(destination); }); + + it('should surface the rekey target when explaining a transfer transaction that rekeys the account', async function () { + const sender = AlgoResources.accounts.account1.address; + const destination = AlgoResources.accounts.account2.address; + const rekeyAddress = AlgoResources.accounts.account3.address; + const txBuilder = buildBaseTransferTransaction({ sender, destination, amount: 10000 }); + txBuilder.reKeyTo({ address: rekeyAddress }); + const unsignedTransaction = await txBuilder.build(); + const unsignedHex = Buffer.from(unsignedTransaction.toBroadcastFormat()).toString('hex'); + const explain = await basecoin.explainTransaction({ + txHex: unsignedHex, + feeInfo: { fee: '1000' }, + }); + explain.reKeyTo.should.equal(rekeyAddress); + explain.displayOrder.should.containEql('reKeyTo'); + explain.outputs[0].address.should.equal(destination); + }); + + it('should explain the halfSigned transaction when both txHex and halfSigned txHex are provided', async function () { + const sender = AlgoResources.accounts.account1.address; + const destination = AlgoResources.accounts.account2.address; + const rekeyAddress = AlgoResources.accounts.account3.address; + + // plain unsigned payment without a rekey, used as the misleading txHex + const plainHex = Buffer.from( + (await buildBaseTransferTransaction({ sender, destination, amount: 10000 }).build()).toBroadcastFormat() + ).toString('hex'); + + // half-signed payment that rekeys the account, used as the halfSigned txHex + const rekeyedBuilder = buildBaseTransferTransaction({ sender, destination, amount: 10000 }); + rekeyedBuilder.reKeyTo({ address: rekeyAddress }); + rekeyedBuilder.numberOfSigners(2); + rekeyedBuilder.setSigners([AlgoResources.accounts.account1.address, AlgoResources.accounts.account3.address]); + rekeyedBuilder.sign({ key: AlgoResources.accounts.account1.prvKey }); + const halfSignedHex = Buffer.from((await rekeyedBuilder.build()).toBroadcastFormat()).toString('hex'); + + // signTransaction signs the halfSigned hex when both are present, so the + // explanation must come from the same bytes, not from txHex + const explain = await basecoin.explainTransaction({ + txHex: plainHex, + halfSigned: { txHex: halfSignedHex }, + feeInfo: { fee: '1000' }, + }); + explain.reKeyTo.should.equal(rekeyAddress); + explain.displayOrder.should.containEql('reKeyTo'); + }); }); describe('Asset Transfer Builder: ', () => { @@ -349,6 +395,23 @@ describe('ALGO:', function () { explain.outputs[0].address.should.equal(destination); explain.tokenId.should.equal(1); }); + + it('should surface the rekey target when explaining an asset transfer transaction that rekeys the account', async function () { + const sender = AlgoResources.accounts.account1.address; + const destination = AlgoResources.accounts.account2.address; + const rekeyAddress = AlgoResources.accounts.account3.address; + const txBuilder = buildBaseAssetTransferTransaction({ sender, destination, amount: 10000, tokenId: 1 }); + txBuilder.reKeyTo({ address: rekeyAddress }); + const unsignedTransaction = await txBuilder.build(); + const unsignedHex = Buffer.from(unsignedTransaction.toBroadcastFormat()).toString('hex'); + const explain = await basecoin.explainTransaction({ + txHex: unsignedHex, + feeInfo: { fee: '1000' }, + }); + explain.reKeyTo.should.equal(rekeyAddress); + explain.displayOrder.should.containEql('reKeyTo'); + explain.outputs[0].address.should.equal(destination); + }); }); describe('Wallet Init Builder: ', () => { @@ -538,6 +601,21 @@ describe('ALGO:', function () { explain.voteLast.should.equal(100); explain.voteKeyDilution.should.equal(9); }); + + it('should surface the rekey target when explaining a keyreg transaction that rekeys the account', async function () { + const sender = AlgoResources.accounts.account1; + const rekeyAddress = AlgoResources.accounts.account3.address; + const txBuilder = buildBaseKeyRegTransaction({ sender }); + txBuilder.reKeyTo({ address: rekeyAddress }); + const unsignedTransaction = await txBuilder.build(); + const unsignedHex = Buffer.from(unsignedTransaction.toBroadcastFormat()).toString('hex'); + const explain = await basecoin.explainTransaction({ + txHex: unsignedHex, + feeInfo: { fee: '1000' }, + }); + explain.reKeyTo.should.equal(rekeyAddress); + explain.displayOrder.should.containEql('reKeyTo'); + }); }); describe('Sign transaction', () => { it('should sign transaction', async function () { @@ -648,6 +726,99 @@ describe('ALGO:', function () { }); signed.txHex.should.equal(AlgoResources.rawTx.transfer.multisig); }); + + it('should refuse to sign a transaction that rekeys the account', async function () { + const factory = new TransactionBuilderFactory(coins.get('algo')); + const txBuilder = factory + .getTransferBuilder() + .sender({ address: AlgoResources.accounts.account1.address }) + .to({ address: AlgoResources.accounts.account2.address }) + .amount(10000) + .isFlatFee(true) + .fee({ fee: '1000' }) + .firstRound(1) + .lastRound(100) + .testnet(); + txBuilder.reKeyTo({ address: AlgoResources.accounts.account3.address }); + const unsignedTx = await txBuilder.build(); + const unsignedHex = Buffer.from(unsignedTx.toBroadcastFormat()).toString('hex'); + + await basecoin + .signTransaction({ + txPrebuild: { + txHex: unsignedHex, + keys: [AlgoResources.accounts.account1.pubKey.toString('hex')], + addressVersion: 1, + }, + prv: AlgoResources.accounts.account1.prvKey, + }) + .should.be.rejectedWith( + new RegExp( + `transaction rekeys the account to ${AlgoResources.accounts.account3.address}: refusing to sign unless the rekey is confirmed intentional` + ) + ); + }); + + it('should sign a transaction that rekeys the account when the rekey is explicitly allowed', async function () { + const factory = new TransactionBuilderFactory(coins.get('algo')); + const txBuilder = factory + .getTransferBuilder() + .sender({ address: AlgoResources.accounts.account1.address }) + .to({ address: AlgoResources.accounts.account2.address }) + .amount(10000) + .isFlatFee(true) + .fee({ fee: '1000' }) + .firstRound(1) + .lastRound(100) + .testnet(); + txBuilder.reKeyTo({ address: AlgoResources.accounts.account3.address }); + const unsignedTx = await txBuilder.build(); + const unsignedHex = Buffer.from(unsignedTx.toBroadcastFormat()).toString('hex'); + + const signed = await basecoin.signTransaction({ + txPrebuild: { + txHex: unsignedHex, + keys: [AlgoResources.accounts.account1.pubKey.toString('hex')], + addressVersion: 1, + }, + prv: AlgoResources.accounts.account1.prvKey, + allowReKeyTo: AlgoResources.accounts.account3.address, + }); + assert.strictEqual(typeof signed.txHex, 'string'); + }); + + it('should refuse to sign a rekeyed transaction when the approved rekey address does not match', async function () { + const factory = new TransactionBuilderFactory(coins.get('algo')); + const txBuilder = factory + .getTransferBuilder() + .sender({ address: AlgoResources.accounts.account1.address }) + .to({ address: AlgoResources.accounts.account2.address }) + .amount(10000) + .isFlatFee(true) + .fee({ fee: '1000' }) + .firstRound(1) + .lastRound(100) + .testnet(); + txBuilder.reKeyTo({ address: AlgoResources.accounts.account3.address }); + const unsignedTx = await txBuilder.build(); + const unsignedHex = Buffer.from(unsignedTx.toBroadcastFormat()).toString('hex'); + + await basecoin + .signTransaction({ + txPrebuild: { + txHex: unsignedHex, + keys: [AlgoResources.accounts.account1.pubKey.toString('hex')], + addressVersion: 1, + }, + prv: AlgoResources.accounts.account1.prvKey, + allowReKeyTo: AlgoResources.accounts.account2.address, + }) + .should.be.rejectedWith( + new RegExp( + `transaction rekeys the account to ${AlgoResources.accounts.account3.address}: refusing to sign unless the rekey is confirmed intentional` + ) + ); + }); }); describe('Sign message', () => {