Skip to content

Commit 43df0e2

Browse files
committed
feat(sdk-coin-hbar): implement hardened token enablement transaction verification
CLOSES TICKET: WP-5746
1 parent c5bf752 commit 43df0e2

2 files changed

Lines changed: 89 additions & 50 deletions

File tree

modules/sdk-coin-hbar/src/hbar.ts

Lines changed: 88 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -267,40 +267,56 @@ export class Hbar extends BaseCoin {
267267
}
268268
}
269269

270-
// Strict validation: exactly 1 output, amount must be 0
270+
// Strict validation: allow 0 outputs or exactly 1 output with amount 0
271271
private validateTxStructureStrict(ex: TransactionExplanation): void {
272-
if (!ex.outputs || ex.outputs.length !== 1) {
273-
throw new Error(`Expected exactly 1 output, got ${ex.outputs?.length ?? 0}`);
272+
if (!ex.outputs || ex.outputs.length === 0) {
273+
return; // acceptable for pure associate in some explainers
274+
}
275+
if (ex.outputs.length !== 1) {
276+
throw new Error(`Expected exactly 1 output, got ${ex.outputs.length}`);
274277
}
275278
const out0 = ex.outputs[0];
276279
if (out0.amount !== '0') {
277280
throw new Error(`Expected output amount '0', got ${out0.amount}`);
278281
}
279282
}
280283

281-
// Ensure no transfers are present (HBAR, token, or NFT)
284+
// Deep recursive scan for any transfers anywhere in the transaction
282285
private validateNoTransfers(raw: any): void {
283-
// Hedera/BitGo raw shapes vary; check all known transfer containers
284-
// Be more lenient since the structure may not always have these fields
285-
const hasHbarTransfers = Array.isArray(raw.transfers) && raw.transfers.length > 0;
286-
const hasTokenTransfers = Array.isArray(raw.tokenTransfers) && raw.tokenTransfers.length > 0;
287-
const hasNftTransfers = Array.isArray(raw.nftTransfers) && raw.nftTransfers.length > 0;
288-
289-
// Only validate if we can clearly identify transfers
290-
if (hasHbarTransfers || hasTokenTransfers || hasNftTransfers) {
286+
if (this.hasAnyTransfers(raw)) {
291287
throw new Error('Transaction contains transfers; not a pure token enablement.');
292288
}
293289
}
294290

295-
// Validate account ID matches in both explained and raw transaction data
291+
// Recursive function to detect any transfers in nested structures
292+
private hasAnyTransfers(obj: any): boolean {
293+
if (!obj || typeof obj !== 'object') return false;
294+
295+
// Check for known transfer containers
296+
if (Array.isArray(obj.accountAmounts) && obj.accountAmounts.length > 0) return true;
297+
if (Array.isArray(obj.tokenTransfers) && obj.tokenTransfers.length > 0) return true;
298+
if (Array.isArray(obj.nftTransfers) && obj.nftTransfers.length > 0) return true;
299+
if (Array.isArray(obj.transfers) && obj.transfers.length > 0) return true;
300+
if (Array.isArray(obj.tokenTransferLists) && obj.tokenTransferLists.some((t: any) => this.hasAnyTransfers(t)))
301+
return true;
302+
303+
// Recursively check all nested objects
304+
return Object.values(obj).some((value: any) => this.hasAnyTransfers(value));
305+
}
306+
307+
// Validate account ID matches in both explained and raw transaction data with normalization
296308
private validateAccountIdMatches(ex: TransactionExplanation, raw: any, expectedAccountId: string): void {
297-
const out0 = ex.outputs[0];
298-
if (out0.address !== expectedAccountId) {
299-
throw new Error(`Expected account ID ${expectedAccountId}, got ${out0.address}`);
309+
// Only validate if outputs exist
310+
if (ex.outputs && ex.outputs.length > 0) {
311+
const out0 = ex.outputs[0];
312+
if (!Utils.isSameBaseAddress(out0.address, expectedAccountId)) {
313+
throw new Error(`Expected account ${expectedAccountId}, got ${out0.address}`);
314+
}
300315
}
316+
301317
const assocAcct = raw.instructionsData?.accountId ?? raw.instructionsData?.owner ?? raw.accountId;
302-
if (assocAcct && assocAcct !== expectedAccountId) {
303-
throw new Error(`Raw associate account ${assocAcct} does not match expected ${expectedAccountId}`);
318+
if (assocAcct && !Utils.isSameBaseAddress(assocAcct, expectedAccountId)) {
319+
throw new Error(`Associate account ${assocAcct} does not match expected ${expectedAccountId}`);
304320
}
305321
}
306322

@@ -310,46 +326,62 @@ export class Hbar extends BaseCoin {
310326
raw: any,
311327
expected: { tokenId?: string; tokenName?: string }
312328
): void {
313-
const out0 = ex.outputs[0];
329+
// Get tokens from raw transaction data
330+
const rawTokens: string[] =
331+
raw.instructionsData?.tokens ??
332+
raw.instructionsData?.tokenIds ??
333+
(raw.instructionsData?.tokenId ? [raw.instructionsData.tokenId] : []);
334+
335+
// If we have raw token data, validate it strictly for security
336+
if (rawTokens.length > 0) {
337+
// Must have exactly 1 token to associate
338+
if (rawTokens.length !== 1) {
339+
throw new Error(`Expected exactly 1 token to associate, got ${rawTokens.length}`);
340+
}
341+
342+
// Prefer tokenId validation over tokenName
343+
if (expected.tokenId) {
344+
if (rawTokens[0] !== expected.tokenId) {
345+
throw new Error(`Raw tokenId ${rawTokens[0]} != expected ${expected.tokenId}`);
346+
}
347+
}
348+
}
314349

315-
// For token enablement, we primarily validate based on the explained transaction
316-
// since the raw transaction structure varies and may not have easily accessible token info
317-
if (expected.tokenName) {
350+
// Primary validation: tokenName from explained transaction
351+
if (expected.tokenName && ex.outputs && ex.outputs.length > 0) {
352+
const out0 = ex.outputs[0];
318353
const explainedName = out0.tokenName;
319354
if (explainedName !== expected.tokenName) {
320355
throw new Error(`Expected token name ${expected.tokenName}, got ${explainedName}`);
321356
}
322-
} else if (expected.tokenId) {
323-
// If tokenId is provided, we can try to validate against raw data if available
324-
// but for now, we'll be more lenient since the structure may vary
325-
const rawTokens: string[] =
326-
raw.instructionsData?.tokens ??
327-
raw.instructionsData?.tokenIds ??
328-
(raw.instructionsData?.tokenId ? [raw.instructionsData.tokenId] : []);
329-
330-
if (rawTokens.length > 0 && rawTokens[0] !== expected.tokenId) {
331-
throw new Error(`Raw tokenId ${rawTokens[0]} != expected ${expected.tokenId}`);
332-
}
333357
}
334358
}
335359

336-
// Validate that this is a pure token associate instruction with no additional operations
360+
// Validate that this is a pure native token associate instruction with no additional operations
337361
private validateAssociateInstructionOnly(raw: any): void {
338-
let t = raw.instructionsData?.type;
339-
if (typeof t === 'string') t = t.toLowerCase();
340-
const ok = t === 'tokenassociate' || t === 'associate' || t === 'associate_token';
341-
if (!ok) {
342-
throw new Error(`Expected token associate instruction, got ${raw.instructionsData?.type ?? 'unknown'}`);
343-
}
344-
345-
// Ensure there are no additional instructions/operations batched
346-
// Be more lenient since the structure may vary
347-
const opsCount =
348-
(Array.isArray(raw.instructions) ? raw.instructions.length : 0) +
349-
(Array.isArray(raw.innerInstructions) ? raw.innerInstructions.length : 0);
350-
if (opsCount > 1) {
362+
const t = String(raw.instructionsData?.type || '').toLowerCase();
363+
364+
// Explicitly reject ContractExecute/precompile routes first
365+
if (t === 'contractexecute' || t === 'contractcall' || t === 'precompile') {
366+
throw new Error(`Contract-based token association not allowed for blind enablement; got ${t}`);
367+
}
368+
369+
// Only allow native TokenAssociate
370+
const isNativeAssociate = t === 'tokenassociate' || t === 'associate' || t === 'associate_token';
371+
if (!isNativeAssociate) {
372+
throw new Error(`Only native TokenAssociate is allowed for blind enablement; got ${t || 'unknown'}`);
373+
}
374+
375+
// Strict batching validation - no additional instructions allowed
376+
if (Array.isArray(raw.instructions) && raw.instructions.length > 0) {
351377
throw new Error('Additional instructions found; transaction is not a pure token enablement.');
352378
}
379+
if (Array.isArray(raw.innerInstructions) && raw.innerInstructions.length > 0) {
380+
throw new Error('Inner instructions found; transaction is not a pure token enablement.');
381+
}
382+
if (raw.scheduledTransactionBody) {
383+
throw new Error('Scheduled transactions are not allowed in blind enablement.');
384+
}
353385
}
354386

355387
async verifyTransaction(params: HbarVerifyTransactionOptions): Promise<boolean> {
@@ -377,8 +409,15 @@ export class Hbar extends BaseCoin {
377409
if (txParams.type === 'enabletoken') {
378410
const r0 = txParams.recipients[0];
379411
const expectedToken: { tokenId?: string; tokenName?: string } = {};
380-
if (r0.tokenName) expectedToken.tokenName = r0.tokenName;
381-
// Note: tokenId is not available on ITransactionRecipient, so we only use tokenName
412+
413+
// Use tokenName from recipient (tokenId not available in current API)
414+
if (r0.tokenName) {
415+
expectedToken.tokenName = r0.tokenName;
416+
// Note: tokenName validation is less secure than tokenId, but tokenId is not available in ITransactionRecipient
417+
} else {
418+
throw new Error('Token enablement requires tokenName in recipient');
419+
}
420+
382421
await this.verifyTokenEnablementTransaction(txPrebuild.txHex, expectedToken, r0.address);
383422
return true; // IMPORTANT: do not fall through to generic transfer verification
384423
}

modules/sdk-coin-hbar/test/unit/hbar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ describe('Hedera Hashgraph:', function () {
551551
it('should fail when account ID does not match', async function () {
552552
await basecoin
553553
.verifyTokenEnablementTransaction(TestData.UNSIGNED_TOKEN_ASSOCIATE, { tokenName: 'thbar:usdc' }, '0.0.99999')
554-
.should.be.rejectedWith(/Expected account ID 0.0.99999, got 0.0.81320/);
554+
.should.be.rejectedWith(/Expected account 0.0.99999, got 0.0.81320/);
555555
});
556556

557557
it('should fail when transaction is not a token enablement (has non-zero amount)', async function () {

0 commit comments

Comments
 (0)