Skip to content

feat(sdk-coin-sol): prevent blind signing sol token enablement #6899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+519 −12
Open

feat(sdk-coin-sol): prevent blind signing sol token enablement #6899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions modules/sdk-coin-sol/src/sol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
* @prettier
*/

import { TOKEN_2022_PROGRAM_ID, TOKEN_PROGRAM_ID } from '@solana/spl-token';
import BigNumber from 'bignumber.js';
import * as base58 from 'bs58';
import * as _ from 'lodash';
import * as request from 'superagent';

import {
AuditDecryptedKeyParams,
BaseBroadcastTransactionOptions,
BaseBroadcastTransactionResult,
BaseCoin,
Expand All @@ -27,9 +31,13 @@ import {
MPCTx,
MPCTxs,
MPCUnsignedTx,
MultisigType,
multisigTypes,
OvcInput,
OvcOutput,
ParsedTransaction,
PopulatedIntent,
PrebuildTransactionWithIntentOptions,
PresignTransactionOptions,
PublicKey,
RecoveryTxRequest,
Expand All @@ -40,16 +48,9 @@ import {
TransactionRecipient,
VerifyAddressOptions,
VerifyTransactionOptions,
MultisigType,
multisigTypes,
AuditDecryptedKeyParams,
PopulatedIntent,
PrebuildTransactionWithIntentOptions,
} from '@bitgo/sdk-core';
import { auditEddsaPrivateKey, getDerivationPath } from '@bitgo/sdk-lib-mpc';
import { BaseNetwork, CoinFamily, coins, BaseCoin as StaticsBaseCoin } from '@bitgo/statics';
import * as _ from 'lodash';
import * as request from 'superagent';
import { KeyPair as SolKeyPair, Transaction, TransactionBuilder, TransactionBuilderFactory } from './lib';
import {
getAssociatedTokenAccountAddress,
Expand All @@ -60,7 +61,6 @@ import {
isValidPublicKey,
validateRawTransaction,
} from './lib/utils';
import { TOKEN_2022_PROGRAM_ID, TOKEN_PROGRAM_ID } from '@solana/spl-token';

export const DEFAULT_SCAN_FACTOR = 20; // default number of receive addresses to scan for funds

Expand Down Expand Up @@ -173,6 +173,8 @@ export interface SolConsolidationRecoveryOptions extends MPCConsolidationRecover
}

const HEX_REGEX = /^[0-9a-fA-F]+$/;
const BLIND_SIGNING_TX_TYPES_TO_CHECK = { enabletoken: 'AssociatedTokenAccountInitialization' };
const BLIND_SIGNING_TX_TYPES_WITH_EMPTY_OUTPUTS = ['enabletoken'];

export class Sol extends BaseCoin {
protected readonly _staticsCoin: Readonly<StaticsBaseCoin>;
Expand Down Expand Up @@ -233,6 +235,32 @@ export class Sol extends BaseCoin {
return Math.pow(10, this._staticsCoin.decimalPlaces);
}

verifyTxType(expectedTypeFromUserParams: string | undefined, actualTypeFromDecoded: string | undefined): void {
// do nothing, let the tx fail way down as always
if (expectedTypeFromUserParams === undefined || actualTypeFromDecoded === undefined) return;

// ex: user param comes with 'enabletoken' included in BLIND SIGNING -> if fails later in the IF check
// ex: user param comes with 'transfer' non included in BLIND SIGNING-> correctPrebuildTxType goes undefined and IF later goes false so it pass
// check the unit tests for this.
const correctPrebuildTxType = BLIND_SIGNING_TX_TYPES_TO_CHECK[expectedTypeFromUserParams];

if (correctPrebuildTxType && correctPrebuildTxType !== actualTypeFromDecoded) {
throw new Error(
`Tx type "${actualTypeFromDecoded}" does not match expected txParams type "${expectedTypeFromUserParams}"`
);
}
}

verifyNoOutputs(expectedTypeFromUserParams: string | undefined, decodedTxHex: TransactionExplanation): void {
if (
expectedTypeFromUserParams &&
BLIND_SIGNING_TX_TYPES_WITH_EMPTY_OUTPUTS.includes(expectedTypeFromUserParams) &&
decodedTxHex.outputs?.length > 0
) {
throw new Error(`Tx outputs were found when none were expected for sol enablement tokens`);
}
}

async verifyTransaction(params: SolVerifyTransactionOptions): Promise<boolean> {
// asset name to transfer amount map
const totalAmount: Record<string, BigNumber> = {};
Expand Down Expand Up @@ -261,6 +289,9 @@ export class Sol extends BaseCoin {
transaction.fromRawTransaction(rawTxBase64);
const explainedTx = transaction.explainTransaction();

this.verifyNoOutputs(txParams.type, explainedTx);
this.verifyTxType(txParams.type, explainedTx.type);
Comment on lines +292 to +293
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's wrap this around a verification option,

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

Line 165 in a514410

export interface VerificationOptions {

which would default to true if unspecified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmm not sure about that tbh, so far I'm starting on XRP and there're things to verify that are not concerns to SOL.

I could try to work the way up in a follow up when we get both XRP and SOL merged to at least extract things in common or see what's an edge case and avoid delaying the delivering of the security features implementation.


// users do not input recipients for consolidation requests as they are generated by the server
if (txParams.recipients !== undefined) {
const filteredRecipients = txParams.recipients?.map((recipient) =>
Expand Down
Loading